opt: inline UDFs as subqueries #92955
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @rharding6373)
pkg/sql/opt/norm/inline_funcs.go line 440 at r1 (raw file):
// argForParam returns the argument that can be substituted for the given // column, if the column is a parameter of the UDF. It returns ok=false if // the column is not a UDF parameter.
Nice, your work to disambiguate parameters and arguments is paying off here.
pkg/sql/opt/norm/inline_funcs.go line 481 at r1 (raw file):
&memo.SubqueryPrivate{ OriginalExpr: nil, Ordering: ordering,
[nit] Is it necessary to propagate an ordering here, since the input is guaranteed to return at most one row? Any required ordering should already be preserved by the limit, right?
[also nit] why explicitly set OriginalExpr here?
pkg/sql/opt/norm/testdata/rules/inline line 1149 at r1 (raw file):
│ └── "?column?":13 = 100 [outer=(13), constraints=(/13: [/100 - /100]; tight), fd=()-->(13)] └── projections └── (i:2 + 20) + 10 [as=add:20, outer=(2), immutable]
Why isn't this folded into i + 30? Just don't have a rule for it I guess?
There was a problem hiding this comment.
Reviewable status:
mgartner
force-pushed
the
inline-udf-1
branch
from
6d20af6 to
ebad5b8
Compare
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/sql/opt/norm/inline_funcs.go line 440 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
Nice, your work to disambiguate parameters and arguments is paying off here.
🎉
pkg/sql/opt/norm/inline_funcs.go line 481 at r1 (raw file):
You're right, the Limit maintains the ordering. This might change when we support set-returning UDFs, so I've left some TODOs for now so we don't forget.
[also nit] why explicitly set OriginalExpr here?
Good catch. That was just leftover; I've removed it.
pkg/sql/opt/norm/testdata/rules/inline line 1149 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
Why isn't this folded into
i + 30? Just don't have a rule for it I guess?
Ya, there's not a rule for it:
defaultdb> EXPLAIN (OPT) SELECT i+10+20 FROM generate_series(1, 5) g(i);
info
-----------------------------------------
project
├── project-set
│ ├── values
│ │ └── ()
│ └── zip
│ └── generate_series(1, 5)
└── projections
└── (generate_series + 10) + 20
(8 rows)
add(10, add(i, 200)) = 100 would be normalized (in a project or filter) though because we have NormalizeCmpPlusConstwhich would fire twice.
|
Build failed (retrying...): |
|
I think this PR is introducing a flake in bors r- |
|
Canceled. |
|
@yuzefovich where do you see that? |
|
The bors batch failed and then Bazel Essential CI on this PR is red for the same reason on |
UDFs are now inlined as subqueries by a normalization rule when possible, speeding up their evaluation. Release note (performance improvement): Some types of user-defined functions are now inlined in query plans as relation expressions, which speeds up their evaluation. UDFs must be non-volatile and have a single statement in the function body to be inlined.
|
Fixed the flake. bors r+ |
|
Build succeeded: |
UDFs are now inlined as subqueries by a normalization rule when possible, speeding up their evaluation.
Epic: CRDB-20370
Release note (performance improvement): Some types of user-defined functions are now inlined in query plans as relation expressions, which speeds up their evaluation. UDFs must be non-volatile and have a single statement in the function body to be inlined.