efcore.pg

efcore.pg copied to clipboard

Could go along well with partial index support (#116). See open issue on possible support in EFCore itself: https://github.com/aspnet/EntityFramework/issues/3986.

Note previous issue when scaffolding a database that contained a partial index: #12

It turns out PostgreSQL supports indices with multiple expressions, and even the mixing of columns and expressions on the same index...

This makes it pretty impractical to model expression indices in EF Core: a built-in Index contains properties, and while we could use annotations to augment it, this becomes very messy with the mixing of both columns and annotations.

Doesn't seem to be worth the effort, when it's easy to add raw SQL in migrations for creating these indices.

I'm trying to add an index on the lowercase of a text column, so that if I need to do case insensitive lookup on the column (which is not always), I can use .ToLower() == .ToLower() and have the query use the index.

My first attempt was to do something like: builder.Entity<Something>(o => { o.HasIndex(s => s.SomeStringColumn.ToLower()); }

But obviously this doesn't work. Is this use case totally off the table?

I'm currently using migrationBuilder.Sql("CREATE UNIQUE INDEX \"IX_Something_SomeStringColumn_lower\" ON \"Something\" ((lower(\"SomeStringColumn\")));");

@crozone you definitely can't do anything like that with HasIndex() - this is what this issue was originally about but I ended up dropping the idea because it didn't make much sense.

Your second (current) approach is the right one - using raw SQL to define the index - is it not working for you?

@roji - If I am understanding correctly, at present there is no way to create index on arbitrary expressions other than using migrationBuilder.Sql() (in PostgreSQL)?

@smitpatel that's right, I looked at this a while back but as I wrote above, PostgreSQL support for expression indices is very powerful/flexible, and so the effort of modeling everything seemed a bit too much considering you can just use raw sql.

Are you guys considering some support for expression indices or something? I can revisit this if relevant.

We have tracking issue on EF side without a note from 2015 (I need to find out what we are going to do with that). I was looking at the issue from RevEng side mainly. Since EFCore can translate annotations while scaffolding now, PostgreSQL can map such indices to their fluent API if any through annotations.

@smitpatel OK, thanks for the update. Scaffolding wasn't really the issue here, but rather the complexity of the fluent API that would be needed to express indices over any number of both columns and arbitrary expressions (and mixing them together). Not that it's not possible - it's just that the value of providing such an API didn't seem clear when the same can be done with raw SQL.

Would definitely be interested in your input on this (I also have no idea regarding expression index capabilities in other databases etc.)

@smitpatel, I'm revisiting this after a while, and considering adding support for a limited scenario, but which probably is the 90% case (index with a single expression and no columns). You mentioned an issue in EF Core about supporting expression index, can you fill me in on what you decided to do? If you guys have plans for some sort of expression index support, it's better for me to do it in a way that aligns with yours etc.

The referenced issue: https://github.com/aspnet/EntityFrameworkCore/issues/3986 We decided not to fix it in EF Core since it may not make sense for all providers. Since there is difficulty in modeling it in existing Index objects which are bound to properties (aka columns), they are not the best object to represent this. Further it is purely database side optimization and does not affect working of EF Core runtime. The best way to support this would be an API to configure index which would store the custom SQL as annotation over the EntityType which migration pipeline could light up and create database index. RevEng could work it backwards to create ET annotation and generate methodcall appropriately.

Thanks @smitpatel, an annotation on the index was the approach I was going to use - and if you guys have no plans to support this in any way, I'll go ahead and do it as a totally PostgreSQL-specific feature.

/// <summary>
/// This is currently unsupported and will be ignored.
/// </summary>
public const string IndexExpression = Prefix + "IndexExpression";

will it be done to .NET 5?

@iPilot this issue is in the backlog milestone, so it's unlikely. Index expressions are very flexible and therefore difficult to model, and you can also just set them up yourself - so the value of having EF Core do this for you is rather low.

It would be nice to at least have something we could put with the rest of the Model Build steps or as Annotation so it's documented with the other parts of the schema. This might be similar to the HasColumnName() or HasColumnType() that let's us put a string but in this case let's us specify the raw sql that would go into the Migration.

i.e. builder.HasSqlIndex("upper(columnname)"); // with name generated as it is now but by by replacing non-alphanum characters with "_" or let us specify the name. builder.HasSqlIndex("IX_table_upper_column", "upper(columnname)");

@JChaneyGuardian yeah, a modelling API for expression indexes (à la HasColumnName) is what this issue tracks. In the meantime, you can use raw SQL in migrations, so you shouldn't be blocked from doing this.

I looked into this again, and it does seem like there's broad cross-database support for this. Above, I mentioned the possibility of mixing column and expression components to be a problem; even if we don't come up with a good way to model this, letting users specify single-expression indexes would probably cover most cases anyway.

Opened https://github.com/dotnet/efcore/issues/28360 to consider doing this at the EF Core level instead of as a PG-only feature.

How would you like this support to be added (i.e. naming of the entity type builder extension method, parameters, requirements)?

I'm considering developing the fix and proposing a PR myself, since I need it a lot to have indexes on unaccent'ed text fields for ILIKE-search purposes. I cannot easily maintain the migrations by manually adding the SQL to have the indices created, and this would lead to very cumbersome repetitive code; I'd rather have an attribute on the entity fields, and reflection to have the proposed "HasSqlIndex()" method be called on the entity type builder.

In desperation, I also tried forcing an Annotation on the model entity, and tried having it passed to the migrationBuilder, so that a custom SQL generator could get the annotation converted to the appropriate CREATE INDEX instruction. Unfortunately, a lot of the migration annotation stuff is internal, and I fail at that workaround - yes I tried ignoring the EF1001 warning, but still the result feels dirty and convoluted, so I'd prefer a native way of expressing that requirement.

@alex-mazzariol None of the stuff needed to flow an annotation to the migrations sql generator should be internal, but this is indeed something that a database provider typically does, rather than an end user. If you're looking to modify Npgsql.EntityFrameworkCore.PostgreSQL to support this, then that should definitely be possible without touching anything internal (look at one of the other PG-specific features as an example).

Unfortunately, I don't have the bandwidth at this particular moment to think about the proper design here or to provide implementation help...

@alex-mazzariol

I'd rather have an attribute on the entity fields, and reflection to have the proposed "HasSqlIndex()" method be called on the entity type builder.

You got the right approach already. You could do it like this:

• Define a HasExpressionIdnex on EntityType, pass it the index name, the raw SQL expression part after the "ON " keyword (for example USING btree (..) would be the string needed to be passed), your code would generate the rest, and a list of "dependent columns" on which the expression depends. Set this info on annotations.
• Define a IRelationalAnnotationProvider that transfers the annotations from the EntityType metadata to the ITable metadata.
• Modify / wrap / decorate the IMigrationsModelDiffer.HasDifferences method so that it calls the base and then searches for annotations on old and new relational models in GetDifferences, if there's a change, generate a "drop" sql migration and generate a "create new" sql migration. Using a record type to fill in from the annotations for comparison and processing will make it easier to detect changes.
• Finally in the GetDifferences method, you'll need to reorder statements so that your index creates are last and your index drops are first.
• If the user renames or drops columns used by the expression index, they would get a PostgreSQL error when they run their migration, so then they will change the expression index call as well to harmonize with the changes, and having your drops first and creates last will ensure the change meshes with the new migrations.
• You could try to be clever in the GetDifferences method and detect if a column you need has been dropped or renamed and give a more useful contextful error message.

One more important note on this: since there's cross-database support, the right approach here would be to implement this in EF's relational layer, rather than as a PG-specific feature in this provider (that's what #28360 tracks) - I'd specifically prefer to not merge a PR here that does this as a PG-specific thing.

Another thing which I think I remember is that PostgreSQL indices can actually mix multiple columns and expressions, which makes it slightly harder to model. Of course, it's probably reasonable to just model the simple case of a single expression, which is likely to be the portable thing across databases in any case.

Yeah I was pondering the IMigrationsModelDiffer route last night too, it is nice to find confirmations on this. Also, thank you all for chiming back on this old issue.

@roji while I agree that this implementation should be split among the EFCore base and provider libraries, this means that this feature will not see the light of day anytime soon (i.e. next 2 years). While I understand and agree with the situation as it is, it would still be nice to have some sort of "reasonable" fallback (i.e. the Npgsql:IndexExpression annotation?) to cover most of the cases in a provider-dependent manner.

@alex-mazzariol FWIW I think that implementing the upstream support in EF itself wouldn't be very different compared to doing it here - the same overall principle apply: metadata and builder APIs which add/manipulate an annotation representing the expression index, flowing that to the provider's migrations SQL generator, and then implementing the actual DDL SQL generation for that. So if you're willing to work on this, I'd highly recommend taking a look there - I know it may sound hard/complicated, but in principle it really is the same kind of work.

Unfortunately we're very near the end of the EF 8.0 release (in fact, no more new features can go in), so this would have to be released for 9.0. At the same time, I doubt I'll have time to review a PG-specific PR either (there's simply too much to do in the coming couple of months), and as I said above, it wouldn't be a good idea to do that only to later replace it with a general EF mechanism (possibly even introducing a breaking change). So I'd suggest looking at doing things properly at the EF level, and if you urgently need something now, patch the relevant services in your specific app.

One more note: IMigrationsModelDiffer is not generally meant to be modified by providers (note that MigrationsModelDiffer is internal), so overriding/wrapoing.HasDifferences isn't the right way to proceed here (/cc @rwasef1830). However, the differ should already handle annotation differences between the source and target model, so you shouldn't need to touch anything there.

I did not find an easy or obvious way of having an annotation be "automatically" moved from the entity model to the table model; as far as I understand it, it should be handled by some custom annotation provider otherwise it is only available in the entity model (i.e. in the generated ".Designer.cs" migration file) but not in the actual migration Up()/Down(). And the Npgsql Annotation provider is internal and should not be messed with, nor can it be instructed to support "more" custom annotations...

I resorted to a completely different, albeit more imprecise, approach - a custom MigrationService.

After all pending migrations have been applied, I run this code:

foreach (var entityType in context.Model.GetEntityTypes())
{
    var entityClrType = entityType.ClrType;
    
    foreach (var property in entityType.GetProperties())
    {
        var propertyInfo = entityClrType.GetProperty(property.Name);

        if (propertyInfo?.IsDefined(typeof(FuzzySearchableAttribute), false) ?? false)
        {
          var indexName = $"IX_{entityType.GetTableName()}_{property.Name}_rgsrc";

          await context.Database.ExecuteSqlRawAsync($"""
            CREATE INDEX IF NOT EXISTS "{indexName}"
              ON "{entityType.GetSchema()}"."{entityType.GetTableName()}"
              USING GIN (immutable_unaccent("{property.Name}") gin_trgm_ops)
          """);
        }
    }
}

And hope that no such column is dropped, or I will have to manually edit the migration to have it cascade the drop (or remove the index before the column drop). I'll leave this here to help for people coming to this issue with similar goals.

Good luck for the EF 8.0 release!

@roji

One more note: IMigrationsModelDiffer is not generally meant to be modified by providers (note that MigrationsModelDiffer is internal), so overriding/wrapoing.HasDifferences isn't the right way to proceed here (/cc @rwasef1830). However, the differ should already handle annotation differences between the source and target model, so you shouldn't need to touch anything there.

If you introduce a new kind of annotation that the existing differ does not know how to compare and generate the right migration operations for, I don't see how to accomplish the stated goal without wrapping the differ. I've had to do that in my own apps to generate some rudimentary trigger maintenance support. Especially if the required migration operation is a SqlOperation with PG-specific SQL that EF simply doesn't know how to generate.

And the Npgsql Annotation provider is internal and should not be messed with, nor can it be instructed to support "more" custom annotations...

Adding support in the provider - which is what this would be about - involves modifying Npgsql's annotation provider. If you're looking to do this only in your own application (rather than submitting a PR to the provider), then you can override this internal service, as you're messing with provider internals at that point. That's OK as long as you understand you may be broken in later releases.

@rwasef1830 the differ already knows how to generally compare annotations in the source and target models, and doesn't need specific knowledge about each and every annotation; annotations are nothing more than key value pairs that the differ compares. You can look at any of the migration-related annotations that already work in Npgsql, none involve changes in the differ.

For totally new migration operations not covered by the existing ones, you don't need to actually create a new MigrationOperation type (which would indeed require modifying the differ to produce it). Instead, when the differ detects any discrepancy between the models, it generates an AlterDatabase, AlterTable, AlterColumn operation (and so forth) containing the old and new annotations. As an example, see how the Npgsql provider does enum management: these are totally PG-specific migration operations which are fully managed via annotations, without any differ support.

Bottom line, providers (and applications) are not supposed to touch the differ - which is why the MigrationsModelDiffer is internal. Annotations can simply flow all the way to the provider's MigrationSqlGenerator, at which point the generator can do whatever it wants with them.