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

Change Translate methods on QueryableMethodTranslatingEV to accept untranslated source #30711

Open
Tracked by #30731
roji opened this issue Apr 18, 2023 · 2 comments
Open
Tracked by #30731

Comments

@roji
Copy link
Member

roji commented Apr 18, 2023

In QueryableMethodTranslatingExpressionVisitor, our operator dispatching logic first visits the source, and only if that visitation succeeds, proceeds to dispatch into one of the Translate methods. For example, the TranslateAny method looks like this:

protected abstract ShapedQueryExpression? TranslateAny(ShapedQueryExpression source, LambdaExpression? predicate);

Note that it accepts an already-translated source (ShapedQueryExpression), but an untranslated predicate (LambdaExpression).

This can be a problem in cases where we want to perform higher-level pattern matching involving the original, untranslated source expression, either because it's untranslatable on its own, or because the original expression is a better fit for pattern matching. A specific example is pattern matching Contains (TranslateAny) over a ParameterQueryRootExpression; the translation of that is provider-specific (e.g. OPENJSON for SQL Server), and so we can't identify it in the relational layer (#30712).

The proposed change here is simply to pass the original, untranslated expression to all the Translate* methods; each method would Visit that internally, and perform any matching if it needs to. This would be a provider-facing breaking change.

@roji
Copy link
Member Author

roji commented Apr 27, 2023

Another important use-case: identifying Count() over byte[] (e.g. test Byte_array_filter_by_length_parameter_compiled). Rather than a JSON array with a COUNT(*) subquery, this needs to be simplified to DATALENGTH(); but since after translation we have an OpenJson TableValuedFunctionExpression, the simplification can't be done at the relational level. If we had access to the pre-translated expression, we'd just check for ParameterQueryRootExpression.

@roji
Copy link
Member Author

roji commented May 9, 2023

Design decision: we're OK with making this change.

@roji roji removed the needs-design label May 9, 2023
@roji roji added this to the Backlog milestone May 9, 2023
@roji roji self-assigned this May 9, 2023
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

2 participants