-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fallback to OPENJSON with many parameters #37234
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
base: main
Are you sure you want to change the base?
Conversation
| { | ||
| switch (node) | ||
| { | ||
| case ValuesExpression valuesExpression when valuesExpression.ValuesParameter is { } valuesParameter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
| case ValuesExpression valuesExpression when valuesExpression.ValuesParameter is { } valuesParameter: | |
| case ValuesExpression { ValuesParameter: { } valuesParameter }: |
(and below)
| } | ||
| else | ||
| { | ||
| ProcessParameter(sqlParameterExpression); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we also need to take Constants mode into account no? In that case the parameter shouldn't be counted at all (since its contents are fully inlined)
| private readonly ISqlServerSingletonOptions _sqlServerSingletonOptions; | ||
|
|
||
| private int _openJsonAliasCounter; | ||
| private int _parametersCount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private int _parametersCount; | |
| private int _totalParameterCount; |
| Count += count; | ||
| if (bucketization) | ||
| { | ||
| var elementTypeMapping = (RelationalTypeMapping)sqlParameterExpression.TypeMapping!.ElementTypeMapping!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remind me - we need the type mapping just in case the padding/bucketization needs to work differently depending on the parameter type (e.g. bucketize on int but not on big varbinary)? If so, we don't actually currently do that, do we?
Fixes #37185.