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

EF should probe compatibility_level (for MS SQL) and translate appropriately #32528

Closed
Montago opened this issue Dec 6, 2023 · 12 comments
Closed

Comments

@Montago
Copy link

Montago commented Dec 6, 2023

EF8 introduces OPENJSON for Contains operations which isn't available in older versions of MSSQL.

Having to probe the MSSQL server for compatibility_level when constructing DBContext ads complexity and increase learning curve for beginners.

EF could probe the MSSQL server once for each program lifecycle and store the information in static variables for later instantiations.
eg. have a Dictionary of [String, Int] : ConnectionString -> compatibility_level

this would most likely also solve other issues where EF translates to incompatible SQL.

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 6, 2023

@Montago Do you have a percentage of how many of your customers still run a compat level below 130? (Current is 160)

@Montago
Copy link
Author

Montago commented Dec 6, 2023

I have no such statistics.

In my case our company has some old SQL servers still running Compat level 100 (SQL 2008) for no apparant reason.

However, automatic probing would make it easier to work with EF8

It would be nice if EF threw a warning message somewhere that the server is running in some low level. or if level is lower than what the server supports.

Some companies might choose to run on older SQL servers because they are cheaper.

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 6, 2023

SQL Server 2008 is not supported by EF Core - only SQL Server 2014 and later is currrently supported.

@Montago
Copy link
Author

Montago commented Dec 6, 2023

The use of OpenJson seems to be a regression / mistake:

#32394
#32374

@Montago
Copy link
Author

Montago commented Dec 6, 2023

SQL Server 2008 is not supported by EF Core - only SQL Server 2014 and later is currrently supported.

We have a SQL 2016 server that runs level 100 and EF8 connects to it fine.

This is mainly our problem.

the OpenJson issue is an EF problem :)

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 6, 2023

@Montago The second issue your refer to is an issue that has already been fixed in the latest daily build, and will most likely be patched in EF8, the first issue is still pending a repro for further investigation.

@Montago
Copy link
Author

Montago commented Dec 6, 2023

regardless ..

Wouldn't it always be beneficial to translate to the proper SQL compat level ?

@ajcvickers
Copy link
Member

@Montago This has been discussed many times, and in fact EF6 used to do this. It's a nightmare because theoretically the compat level can change with every different connection, and yet checking on every single connection is prohibitive from a performance perspective. There's a lot more involved in this that I could go into, like permissions problems, when to do the check to avoid ever connecting to the database when we would not normally, etc.

@Montago
Copy link
Author

Montago commented Dec 6, 2023

It's a nightmare because theoretically the compat level can change with every different connection

do you mean against the same database or different databases ?

I would suspect that each connectionstring / dbcontext use the same compatibility_level each time ?

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 6, 2023

@Montago The compatibility level can be changed at any time...

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2024
@vgallegob
Copy link

I have the same problem, now my query takes minutes...

We had to spend a hole day debuging. Haven't fixed it yet.

This feels wrong.

So many developers will fail to create a efficient query!

@roji
Copy link
Member

roji commented Jan 12, 2024

@vgallegob we've identified several cases where the switch to OPENJSON for Contains causes a regression; at least one such serious case is being fixed for 8.0.2 (#32574, Contains when the item is nullable). Another issue is being investigated #32394; it would be useful if you could open a new issue with your query details and a minimal repro, so that we could understand your case more.

Otherwise, performance regressions here have nothing to do with detecting the SQL Server compatibility level; the compatibility level is only for managing whether OPENJSON is supported at all - on old database it causes a SQL error rather than slower queries.

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

No branches or pull requests

5 participants