opt: incorporate operator volatility#49923
Conversation
|
This change is |
c6d2562 to
3968765
Compare
rytaft
left a comment
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
mgartner
left a comment
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
3968765 to
1c97090
Compare
RaduBerinde
left a comment
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.
rytaft
left a comment
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