opt: build ANY expressions as regular subqueries within UDFs #98375
Conversation
|
This change is |
|
I haven't looked into the specifics yet, but a quick comment based on the PR message - agg filters can prevent decorrelation, so I think we're better off using something like |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)
pkg/sql/opt/optbuilder/builder.go line 133 at r2 (raw file):
// udfDepth is greater than zero, then the builder is currently building // expressions within one or more UDFs. udfDepth int
It seems like we only need to know if we're inside a UDF, so would a bool be sufficient to follow YAGNI principles?
pkg/sql/opt/optbuilder/subquery.go line 376 at r2 (raw file):
if b.udfDepth > 0 { // Any expressions are cannot be built by the optimizer within a UDF, so
nit: Any expressions cannot
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 4 of 5 files at r2, all commit messages.
Reviewable status:
There was a problem hiding this comment.
I haven't looked into the specifics yet, but a quick comment based on the PR message - agg filters can prevent decorrelation, so I think we're better off using something like bool_or(col IS NULL) AND NULL
Done.
It seems like we only need to know if we're inside a UDF, so would a bool be sufficient to follow YAGNI principles?
Ok, switched to a boolean for now. We'll have to use an integer once we support UDFs that call other UDFs.
nit: Any expressions cannot
Fixed.
Reviewable status:
|
Just found |
Heh, no. Looks like we reinvented the wheel here. And a worse version - |
|
This was a great find @DrewKimball! It should be easy to reuse the same code as |
|
Ok this is ready for another look. The first commit is from #98700. |
There was a problem hiding this comment.
Reviewed 9 of 9 files at r5, 6 of 6 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status:
pkg/sql/opt/optbuilder/subquery.go line 377 at r6 (raw file):
if b.insideUDF { // Any expressions cannot be built by the optimizer within a UDF, so // build them as subqueries with GroupBy expressions instead.
[nit] GroupBy->ScalarGroupBy
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 5 of 5 files at r2.
Reviewable status:
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/subquery.go line 377 at r6 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
[nit]
GroupBy->ScalarGroupBy
Done.
To support execution of ANY expressions within UDFs, the optimizer builds them as subqueries with GroupBy expressions instead. The transformation simulates the peculiar behavior of ANY expressions. See `CustomFuncs.ConstructGroupByAny` for more details on this transformation. By constructing ANY expressions within UDFs as subqueries, we avoid adding complexity to the execution engine, which is not currently capable of evaluating ANY expressions within the context of a UDF. Fixes cockroachdb#87291 Release note: None
Release note: None
|
TFTRs! bors r+ |
|
Build succeeded: |
opt: build ANY expressions as regular subqueries within UDFs
To support execution of ANY expressions within UDFs, the optimizer
builds them as subqueries with GroupBy expressions instead. The
transformation simulates the peculiar behavior of ANY expressions. See
CustomFuncs.ConstructGroupByAnyfor more details on thistransformation.
By constructing ANY expressions within UDFs as subqueries, we avoid
adding complexity to the execution engine, which is not currently
capable of evaluating ANY expressions within the context of a UDF.
Fixes #87291
Release note: None
opt: fix GenerateIndexScans comment
Release note: None