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

Feature request: custom SQL in fluent API #7957

Closed
grokky1 opened this issue Mar 22, 2017 · 10 comments
Closed

Feature request: custom SQL in fluent API #7957

grokky1 opened this issue Mar 22, 2017 · 10 comments
Labels
closed-no-further-action The issue is closed and no further action is planned.

Comments

@grokky1
Copy link

grokky1 commented Mar 22, 2017

I hope this is the correct repo for this.

There will always be more you cannot support than can, because every db provider is different.

E.g. I need check constraints, but that can only be done in custom validation (IValidatableObject), or a migration (MigrationBuilder.Sql()). Both are bad options, but reasonable workarounds for now.

It would be better to issue custom SQL from the fluent API. That is where everything is configured, so it makes sense to put it there.

Something like

modelBuilder
  .Property(b => b.Foo)
  .IsRequired()
  .Custom("ADD CONSTRAINT CHECK_SomeTable_SomeColumn CHECK (1 <= Foo);");

Or

modelBuilder
  .Custom("ADD CONSTRAINT CHECK_SomeTable_SomeColumn CHECK (1 <= Foo);");

Or something similar.

They would need to be processed only when the database is generated, not every time. So have for example:

Custom(string sql);                 // processed every time
CustomOnCreating(string sql);       // processed only on db generation

Could something like this be considered?

@roji
Copy link
Member

roji commented Mar 23, 2017

One note: DDL for specifying columns differs across databases, so different clauses may have to be integrate in different places of the statement. In other words, according to the custom behavior you're trying to apply (add constraint or something), the custom fragment may need to be implanted in different places. This makes the idea of custom fragments not very workable - it's better to simply drop down to custom SQL for the entire statement.

@grokky1
Copy link
Author

grokky1 commented Mar 23, 2017

@roji You're right, so:

  • for fragment: should be part of a specific db provider as an extension method, or

  • for entire statement: use modelBuilder.Custom("foo bar"); to apply complete SQL statement (unrelated to a column/table)

Second option is simpler and better (and easier to implement).

@roji
Copy link
Member

roji commented Mar 23, 2017

@grokky1 unfortunately even within a single provider, different fragments may need to be located in different places based one what they're doing.

There's a pretty easy way around all this - you can simply specify raw SQL in your migrations. That is, after having your table created by EF Core in the regular way, you can edit your migration file and simply add an ALTER TABLE statement which does whatever you want. This achieves pretty much what you want in the second option.

@grokky1
Copy link
Author

grokky1 commented Mar 23, 2017

@roji Yeah that's exactly the workaround we're using now.

But specifying some of the config using the ModelBuilder, and the rest using the MigrationBuilder isn't ideal. Since the schema is specified in the fluent API, it makes sense to be able to do it there.

But we must use the "entire statement" approach, like you explained. With something like ModelBuilder.Custom(string sql) or ModelBuilder.CustomOnCreating(string sql).

@roji
Copy link
Member

roji commented Mar 23, 2017

Another issue that comes to mind with specifying raw DDL on the model builder, is statement ordering. Migrations are executed in the order in which they are specified in source code, so they're ideal for inserting some raw DDL somewhere in the middle. The model builder, however, simply builds a model, which later gets translated to (ordered) DDL - how would you possibly specify that some raw DDL has to go after another, or after some non-raw model definition defined via the fluent API?

Although it may seem that currently config is spread out across two different places, I don't think that's the case. It's important to distinguish between different layers: the model is a model, not yet expressed in raw database terms, and indeed may need to get translated to DDL in very different ways, depending on your provider (remember that the same model can be used by different providers). It has nothing to do (yet) with raw DDL. Inserting raw DDL in the model construction skews the line between the different layers (i.e. the model and its translation into DDL by migrations).

At least IMHO this doesn't really make sense...

@grokky1
Copy link
Author

grokky1 commented Mar 23, 2017

The easy way to get around the ordering issue, is to run/translate the custom commands last.

However those are all good points, and I understand what you mean.

(It's just a real pity there's so many places to get the whole story setup, and so many points for failure and confusion. There's the model builder for the basics, if you want extra stuff then stick it into a migration using the migration builder (assuming you're using EF migrations). If you can't then put it into whole-entity validation using something like IValidatableObject, or enforce rules elsewhere in the application rather than the database (our DBA hates that!), or trap the create/edit transaction in the context. And there's repetition in various places. 😆 )

@ajcvickers
Copy link
Member

Triage: we are not planning to add this to the fluent API--see points made by @roji. Ideally, if you wish to flow information from model building through to Migrations this would be done by adding annotations to the model. @bricelam Do you know if flowing annotations through to Migrations is currently doable? Do we have any docs/examples?

@ajcvickers
Copy link
Member

@smitpatel may also have some info on this, such as tests that currently flow annotations from the model to Migrations.

@bricelam
Copy link
Contributor

Do you know if flowing annotations through to Migrations is currently doable? Do we have any docs/examples?

It's possible by overriding the provider-specific IMigrationsAnnotationProvider service. Here's a StackOverflow answer about it.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Mar 30, 2017
@grokky1
Copy link
Author

grokky1 commented Mar 31, 2017

I've posted an SO question related to this. To be honest, I don't understand your advice, or how it helps to solve the issue, because I'm not familiar with EF's internals at all, I only know how to consume it. 😆 I hope someone will be kind enough to look at that question. Thanks for your time and advice.

(PS I still think @roji answer was good in general, but don't agree. Too many configurations happen in too many places. Setting up a big context in EF is messy from a config point of view, but it becomes very elegant once done. I hope with time we can make the config more elegant too. I addressed his concerns with the simple fix. Maybe someone will relook at this issue in the future.)

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned.
Projects
None yet
Development

No branches or pull requests

4 participants