-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Consider removing unnecessary CASTs around JSON_VALUE #28881
Comments
FYI, if the user doesn't include the cast in an index computed column, this causes SQL Server to do an index scan instead of a seek, which causes a very significant perf degradation. We should really prioritize this (and possibly even consider patching, if it's really low-risk). See https://github.com/roji/JsonColumnsPerf for more details. |
And you cannot create indexes on nvarchar(MAX) columns... |
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
Re-open to consider for current? |
I may have drunk a glass or two of wine, but fuck it, let's patch this! |
That will make a lot of DBAs happy and Reduce carbon footprint 😅 |
…und JSON_VALUE We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
…und JSON_VALUE We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
…und JSON_VALUE We can remove cast for string scalars, other types maybe cause conversion issues in the generated sql. Fixes #28881
Rejected for servicing: "It's only perf on a new feature (not a regression) and no customers have reported an impact on their business." |
Too bad. Assume it is in 8.0 today? |
Yes. |
We currently generate JSON queries such as the following:
We seem to systematically wrap JSON_VALUE with a CAST to the specific type - we should consider whether this is actually necessary and when.
Aside from the general verbosness/ugliness factor and unknown perf implications, I expect users will just follow the SQL Server documentation for JSON indexes, which instruct users to create a virtual computed column with the JSON_VALUE expression, and create an index over that. I expect lots of users won't know to include the CAST inside their compuetd column, leading to the index not getting used.
The text was updated successfully, but these errors were encountered: