opt: Inline constant values#33417
Conversation
|
This change is |
1124b3d to
4e55f39
Compare
justinj
left a comment
There was a problem hiding this comment.
Reviewed 9 of 9 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained
pkg/sql/opt/norm/inline.go, line 25 at r1 (raw file):
// constant value expressions: ConstOp, TrueOp, FalseOp, or NullOp. Constant // value expressions can often be inlined into referencing expressions. Only // Project and Values operators synthesize constant value expressions.
What about things like Select operators that have an constant equality condition in their filters? Not sure if that check is also valuable. I guess more generally this is sort of like an FD constant column, except with the restriction that we know the constant value statically. Probably not worth getting super exhaustive on these except in the common cases.
pkg/sql/opt/norm/rules/inline.opt, line 31 at r1 (raw file):
=> (Project $input
nit: you've got some tabs here instead of spaces (and also below)
Inline constants in expressions like: SELECT x, x+1 FROM (VALUES (1)) AS t(x) ; with the new inlining rules, this becomes: VALUES (1, 2) The new inlining rules are useful for mutation expressions (e.g. UPDATE), which can nest multiple Project and Values expressions that often use constant values. For example: CREATE TABLE ab (a INT PRIMARY KEY, b INT AS (a + 1) STORED); UPDATE ab SET a=1 This now gets mapped by the optimizer to this internal equivalent: UPDATE ab SET a=1, b=2 Release note: None
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 9 of 9 files at r1, 2 of 2 files at r2.
Reviewable status:
pkg/sql/opt/norm/testdata/rules/inline, line 154 at r1 (raw file):
│ ├── scan xy │ └── filters │ └── 1 > 0 [type=bool]
why is this not getting folded to true?
andy-kimball
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/norm/inline.go, line 25 at r1 (raw file):
Previously, justinj (Justin Jaffray) wrote…
What about things like
Selectoperators that have an constant equality condition in their filters? Not sure if that check is also valuable. I guess more generally this is sort of like an FD constant column, except with the restriction that we know the constant value statically. Probably not worth getting super exhaustive on these except in the common cases.
I think it's potentially interesting to look for constant equality conditions. I'd do it if/when we find cases where it's important. The cases I'm adding here are useful for mutation cases, since we often "stack" multiple Project/Values operators, and I was noticing we weren't able to reduce them effectively.
pkg/sql/opt/norm/testdata/rules/inline, line 154 at r1 (raw file):
Previously, rytaft wrote…
why is this not getting folded to true?
It appears that we're not normalizing GT to LT (see tree.foldComparisonExpr) when doing folding. I can file another issue on this, as it's orthogonal to this PR.
rytaft
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/norm/testdata/rules/inline, line 154 at r1 (raw file):
Previously, andy-kimball (Andy Kimball) wrote…
It appears that we're not normalizing
GTtoLT(seetree.foldComparisonExpr) when doing folding. I can file another issue on this, as it's orthogonal to this PR.
Sounds good.
|
bors r+ |
33312: sql/opt: avoid swallowing TransactionAbortedErrors r=benesch a=benesch An optimizer code path could, in rare cases, fail to propagate a transaction aborted error. This proved disastrous as, due to a footgun in our transaction API (#22615), swallowing a transaction aborted error results in proceeding with a brand new transaction that has no knowledge of the earlier operations performed on the original transaction. This presented as a rare and confusing bug in splits, as the split transaction uses an internal executor. The internal executor would occasionally silently return a new transaction that only had half of the necessary operations performed on it, and committing that partial transaction would result in a "range does not match splits" error. Fixes #32784. Release note: None /cc @tbg 33417: opt: Inline constant values r=andy-kimball a=andy-kimball Inline constants in expressions like: SELECT x, x+1 FROM (VALUES (1)) AS t(x) ; with the new inlining rules, this becomes: VALUES (1, 2) The new inlining rules are useful for mutation expressions (e.g. UPDATE), which can nest multiple Project and Values expressions that often use constant values. For example: CREATE TABLE ab (a INT PRIMARY KEY, b INT AS (a + 1) STORED); UPDATE ab SET a=1 This now gets mapped by the optimizer to this internal equivalent: UPDATE ab SET a=1, b=2 Release note: None 33421: opt: Tighten up InlineProjectInProject rule r=andy-kimball a=andy-kimball Allow inlining nested Project in case where there are duplicate refs to same inner passthrough column. Previously, this case prevented inlining. However, only duplicate references to inner *synthesized* columns need to be detected. Release note: None Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com> Co-authored-by: Andrew Kimball <andyk@cockroachlabs.com>
Build succeeded |
Inline constants in expressions like:
SELECT x, x+1 FROM (VALUES (1)) AS t(x) ;
with the new inlining rules, this becomes:
VALUES (1, 2)
The new inlining rules are useful for mutation expressions (e.g. UPDATE),
which can nest multiple Project and Values expressions that often use
constant values. For example:
CREATE TABLE ab (a INT PRIMARY KEY, b INT AS (a + 1) STORED);
UPDATE ab SET a=1
This now gets mapped by the optimizer to this internal equivalent:
UPDATE ab SET a=1, b=2
Release note: None