Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Marten keeps recreating indexes #3596

Closed
nkosi23 opened this issue Dec 14, 2024 · 13 comments · Fixed by JasperFx/weasel#165
Closed

Marten keeps recreating indexes #3596

nkosi23 opened this issue Dec 14, 2024 · 13 comments · Fixed by JasperFx/weasel#165
Assignees
Labels

Comments

@nkosi23
Copy link

nkosi23 commented Dec 14, 2024

I have about 10 indexes for which Marten keeps creating a migration script looking like the below even though the indexes have been successfully created in the DB:

drop index if exists abccorp_online_operations.bw_idx_entityid_someprop;
CREATE INDEX bw_idx_entityid_someprop ON abccorp_online_operations.mt_doc_abccorp_people_documents USING btree ((CAST(data ->> 'EntityId' as varchar)), (data -> 'SomeProp'));
storeOptions.Schema
        .For<ABCCorpPeopleDocument>()
        .DocumentAlias("abccorp_people_documents")
        .Index((fun x -> (x.EntityId, x.SomeProp) :> obj), (fun x -> x.Name <- "bw_idx_entityid_someprop"))

While I have changed the names for privacy reason, their length is pretty much intact as I suspect that this problem may be somehow related to the one where Marten didn't detect that objects with truncated names (because the name was too long) have been created in the DB. Some observations:

  • The documents in question both define a DocumentAlias and a name for their index . However, this is the case for other document types that do not have this issue.
  • Moreover, not all indexes of the document types affected have this issue: for example for one of the types having 4 indexes, Marten tries to recreate 3 of the indexes (and the one correctly detected has a name longer than the one of some of the indexes being recreated).
  • I initially suspected that the problem could be with case detection: using uppercase characters for the index name set in marten while postgresql automatically lowercases everything. Marten-side, I was using camel-cases index names such as: "bw_idx_entityId_someProp". However, lower casing everything did not solve the issue.

Since the objects are correctly created in postgresql, this looks like a Marten-side change detection issue.

@mysticmind
Copy link
Member

mysticmind commented Dec 14, 2024

Based on your comments, I am assuming the names are not truncated in the Postgres index definition added in db. I will take a look and revert, most likely a delta detection which is seeing a difference causing it to recreate the indexes.

Edit: A minimal repro would help.

@nkosi23
Copy link
Author

nkosi23 commented Dec 14, 2024

Thanks a lot for your swift feedback, on my side i'll look into making the repro

@nkosi23
Copy link
Author

nkosi23 commented Dec 14, 2024

Okay, I have been able to create a repro
MartenRepro.zip

if you run dotnet run -- db-patch migration3.sql you should see that one of the indexes keeps being recreated:

drop index if exists marten_repro_schema.bwsp_idx_goho705dg4q_poho705d;
CREATE INDEX bwsp_idx_Goho705dg4q_poho705d ON marten_repro_schema.mt_doc_abccorp_people_documents USING btree ((data ->> 'Goho705dg4q33r68'), (data ->> 'Poho705dg4q33r68dfgr'));

@mysticmind
Copy link
Member

@nkosi23 Thanks for the repro, will take it from here

@mysticmind
Copy link
Member

@nkosi23 Quick note, I had troubleshooted and spent time, yet to zero in on it. I will keep you posted.

@mysticmind mysticmind self-assigned this Dec 18, 2024
@nkosi23
Copy link
Author

nkosi23 commented Dec 18, 2024

@mysticmind Thanks a lot for your efforts!

@WilvanBil
Copy link

I encounter the same issue not only with indexes, but also with sequences.

CREATE SEQUENCE IF NOT EXISTS
public.mt_foosequence MAXVALUE 99999999
CYCLE;
CREATE SEQUENCE IF NOT EXISTS
public.mt_barsequence MAXVALUE 9999
CYCLE;

is what the validation returns.

@mysticmind
Copy link
Member

mysticmind commented Jan 1, 2025

@WilvanBil I am still troubleshooting to figure out a fix for the original issue raised, will also look at the issue what you have outlined with sequences. Can you please provide a repro?

@nkosi23
Copy link
Author

nkosi23 commented Jan 7, 2025

@mysticmind have you been able to find any clue about a possible root cause?

@mysticmind
Copy link
Member

@nkosi23 I spent a lot of time troubleshooting, unable to zero in as yet. I will take a look again with a fresh pair of eyes this week, will revert.

@jakobt
Copy link
Contributor

jakobt commented Jan 12, 2025

Hi @nkosi23,

I looked a bit into this and based on what i found your initial hunch that casing was the problem seems pretty spot on, i have reduced your reproduction to this:

`using Marten;
using Shouldly;
using Weasel.Core.Migrations;

public class Tests{

public class SomeEntity
{
    public long Id { get; set; }
    public string Foo { get; set; }
    public string Bar { get; set; }
}

[Fact]
public async Task No_OutStanding_Migrations_After_Initial_With_UpperCase_Characters_In_IndexName(){
    var store = DocumentStore.For(opts => {
        opts.Connection("Host=127.0.0.1;Port=5432;Database=marten_testing;Username=postgres;Password=postgres");
            opts.AutoCreateSchemaObjects = Weasel.Core.AutoCreate.None;
            opts.DatabaseSchemaName = "marten_repro_schema";
            opts.Schema
                .For<SomeEntity>()
                .Index(x => x.Foo, x => x.Name = "Foobar");
    });
    await store.Storage.Database.CompletelyRemoveAllAsync();
    await store.Storage.ApplyAllConfiguredChangesToDatabaseAsync();
    await store.Storage.Database.WriteMigrationFileAsync("second.sql");
    File.ReadAllText("second.sql").ShouldBeEmpty();
}

[Fact]
public async Task No_OutStanding_Migrations_After_Initial_With_LowerCased_IndexName(){
    var store = DocumentStore.For(opts => {
        opts.Connection("Host=127.0.0.1;Port=5432;Database=marten_testing;Username=postgres;Password=postgres");
            opts.AutoCreateSchemaObjects = Weasel.Core.AutoCreate.None;
            opts.DatabaseSchemaName = "marten_repro_schema";
            opts.Schema
                .For<SomeEntity>()
                .Index(x => x.Foo, x => x.Name = "foobar");
    });
    await store.Storage.Database.CompletelyRemoveAllAsync();
    await store.Storage.ApplyAllConfiguredChangesToDatabaseAsync();
    await store.Storage.Database.WriteMigrationFileAsync("second.sql");
    File.ReadAllText("second.sql").ShouldBeEmpty();
}

}`

The lowercase example suceeds whereas the mixed casing in the indexname fails (It keeps creating the index). This is a weasel problem, there is a test case already here https://github.com/JasperFx/weasel/blob/master/src/Weasel.Postgresql.Tests/Tables/detecting_table_deltas.cs#L729

Which fails if you change the indexname to mixed case like below:
image

I would think that its in the lookup in the below line that we are failing to get the correct index and add it as a difference:
https://github.com/JasperFx/weasel/blob/master/src/Weasel.SqlServer/Tables/ItemDelta.cs#L19

A bit unsure if there is a risk of breaking things by making this case insensitive comparison and how that fits with the strategy of weasel and the comment in the readme that states:

By default Postgres tests are run with case insensitive names. To run tests against case sensitive, set environment variable: USE_CASE_SENSITIVE_QUALIFIED_NAMES=true

@mysticmind
Copy link
Member

mysticmind commented Jan 12, 2025

@jakobt thanks for the details. What you are suggesting will break things for others. I will see how to remediate this in terms of delta checks.

@mysticmind
Copy link
Member

mysticmind commented Jan 13, 2025

I have a fix for this - JasperFx/weasel#165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants