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

database EnsureDeleted & EnsureCreated #4028

Closed
DickBaker opened this issue Sep 13, 2022 · 11 comments · Fixed by #4066
Closed

database EnsureDeleted & EnsureCreated #4028

DickBaker opened this issue Sep 13, 2022 · 11 comments · Fixed by #4066

Comments

@DickBaker
Copy link
Contributor

docs at https://docs.microsoft.com/en-us/ef/core/testing/testing-with-the-database#efficient-database-creation debate when EnsureX is called (worrying about outdated schema). Surely this 1-off db drop+create could be best implemented in a static ctor since this must be executed [once] before any instance ctor is invoked ?


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@roji
Copy link
Member

roji commented Sep 13, 2022

@DickBaker the drop+create in the samples on that page are already placed in an xunit class fixture, which means that these operations execute once, before the tests are executed.

However, database drop and create still are relatively heavy operations, and when quickly iterating over code changes and re-running tests over and over, it may be worth temporarily commenting those lines out. Hope that makes sense.

@DickBaker
Copy link
Contributor Author

@roji, yes the fixture means 1-off, but propose guideline to devs to employ static ctor anyway in their own code. I agree heavy impact, but EnsureCreated should be quick if db does exist (and if it doesn't it is obviously worth its o/h).
Anyway I wasn't a great fan of the "DELETE FROM [Blogs] WHERE 1=1;" either as WHERE clause seems unnecessary (also assuming cascade delete of any Posts, Ratings etc child rows).
Happy to accept your decision if any changes merited, so please close this #4028 when done.

@roji
Copy link
Member

roji commented Sep 20, 2022

yes the fixture means 1-off, but propose guideline to devs to employ static ctor anyway in their own code.

What's the advantage of using a static constructor rather than the class fixture? The point of doing it in a class fixture is that the same fixture type can be used multiple times to manage different databases; this isn't possible with a static constructor, which runs only once.

I agree heavy impact, but EnsureCreated should be quick if db does exist (and if it doesn't it is obviously worth its o/h).

In any normal development process, the database always already exists from the last time you ran the tests.

Anyway I wasn't a great fan of the "DELETE FROM [Blogs] WHERE 1=1;" either as WHERE clause seems unnecessary (also assuming cascade delete of any Posts, Ratings etc child rows).

The WHERE clause is required on SQL Server (which is what the sample is for).

@ajcvickers
Copy link
Member

Note from triage: this is not something we plan to do.

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2022
@DickBaker
Copy link
Contributor Author

The "DELETE FROM [Blogs] WHERE 1=1;" clause is required on SQL Server (which is what the sample is for)
please explain why this is necessary. Are you sure MSSQL doesn't require a "DELETE FROM [Blogs] WHERE 1=1 and 2=2;" ?

@roji
Copy link
Member

roji commented Sep 27, 2022

@DickBaker just try it out on SQL Server. Doing DELETE FROM table errors with 'Delete' statement without 'where' clears all data in the table. It's a "safety mechanism" to prevent unintentional deletion of all rows in the table; specifying the (always true) WHERE clause silences it.

@DickBaker
Copy link
Contributor Author

DickBaker commented Sep 28, 2022

weird! Trying to repro your experience I have just tried my SSMS apps
v17.9.1 [aka 14..0.17289.0]
v18.12.1 [aka 15.0.18424.0]
v19.0 Preview 3 [aka 16.0.19061.0
and [outdated] ADS v1.32.0 (user setup) dated 2021-08-16
on a small db on my .\MSSQLLocalDb ("Blogging"), and my local PROD ("Relations") with no errors as per attached

I am aware that SSMS has a Tools, Options, Designers, Table and Database Designers : "Prevent saving changes that require table re-creation" to prevent meltdowns, but I could find no other config settings server/client-side to explain your UX

Please run the @@Version, sp_configure stuff, SSMS/ADS options and any QP (& qry opts) so we can see what is causing that.

Roji, I have to admit shock that there is [somewhere] a "safety mechanism" imposed by some nanny-state buffoon, and that this is so easily defeated by adding a spurious WHERE clause.

Ditto that I had never hit that speed-bump, so I apologise for reacting with horror/disbelief to your "necessary" comment!
Dick4Roji.zip

@DickBaker
Copy link
Contributor Author

update ..
I tried SO that mentioned AWS Redshift but no help to us here

I have now found these gems that suggests this is a JetBrains client-side "help"
https://intellij-support.jetbrains.com/hc/en-us/community/posts/4410218655122-delete-without-where-not-possible
https://www.jetbrains.com/help/phpstorm/sql-delete-or-update-statement-without-where-clauses.html

so maybe not a server-side (MSSQL on-prem or Azure MI/etc cloud) problem afterr all.
Please tell me the EF team is not writing code destined for back-end MSSQL as knee-jerk reaction to the dummy-nanny of a J-B client-side tool !

@roji
Copy link
Member

roji commented Sep 28, 2022

Could be that I was writing my docs SQL samples on Jetbrains... Please feel free to submit a quick PR removing that WHERE clause.

@DickBaker
Copy link
Contributor Author

thanks for admission; best larf I've had today.

I have raised Issue #4060 and will leave you to clear up the doc +/- code mess
sorry I can't muster the strength to raise a formal PR with all the MD for this trivial change

actually I am relieved that the MSSQL wizards haven't been infiltrated by some woke nanny/PHB enforcing global equality & diversity (or in UK the Health & Safety nannies)

As you & Arfur have dismissed my #4028 I feel like we have at least reached a 15-all deuce!

@roji
Copy link
Member

roji commented Sep 28, 2022

sorry I can't muster the strength to raise a formal PR with all the MD for this trivial change

Is it really that hard? It's literally just clicking this button:

image

You don't need to do any markdown, just delete that clause...

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

Successfully merging a pull request may close this issue.

3 participants