opt: incorporate operator volatility #49923
Conversation
|
This change is |
RaduBerinde
force-pushed
the
opt-volatility-ops
branch
2 times, most recently
from
c6d2562
to
3968765
Compare
There was a problem hiding this comment.
Reviewed 75 of 75 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @RaduBerinde)
pkg/sql/opt/memo/logical_props_builder.go, line 1376 at r1 (raw file):
// non-zero constant. // // TODO(radu): clean up this special case.
Maybe expand on this TODO a bit to note that this case should eventually be removed so all division expressions will have the same logic as the IsBinaryOp logic below.
pkg/sql/opt/memo/logical_props_builder.go, line 1422 at r1 (raw file):
o, ok := FindUnaryOverload(e.Op(), inputType) if !ok { panic(errors.AssertionFailedf("unary overload found (%s, %s)", e.Op(), inputType))
found -> not found
There was a problem hiding this comment.
Reviewable status:
a discussion (no related file):
Incorporate volatility of operators (unary, binary, comparison) into the VolatilitySet property. Unfortunately, this modifies most plans as pretty much everything is "immutable". Perhaps once this work is behind us we will want to hide this information from plans in most cases. Release note: None
RaduBerinde
force-pushed
the
opt-volatility-ops
branch
from
3968765
to
1c97090
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/memo/logical_props_builder.go, line 1376 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
Maybe expand on this TODO a bit to note that this case should eventually be removed so all division expressions will have the same logic as the
IsBinaryOplogic below.
Done.
pkg/sql/opt/memo/logical_props_builder.go, line 1422 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
found -> not found
Done.
There was a problem hiding this comment.
Reviewed 9 of 9 files at r2.
Reviewable status:
|
bors r+ |
Build succeeded |
Incorporate volatility of operators (unary, binary, comparison) into the
VolatilitySet property.
Unfortunately, this modifies most plans as pretty much everything is
"immutable". Perhaps once this work is behind us we will want to hide this
information from plans in most cases.
Release note: None