opt,sql: don't fold "- 0" for JSON #35617
Conversation
|
This change is |
|
this may want a backport. |
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @justinj)
pkg/sql/opt/norm/custom_funcs.go, line 1295 at r1 (raw file):
// ---------------------------------------------------------------------- // IsAdditive returned true if the type of the expression supports addition and
returned -> returns
pkg/sql/opt/norm/custom_funcs.go, line 1298 at r1 (raw file):
// subtraction in the natural way. This differs from "has a +/- Numeric // implementation" because JSON has an implementation for "- INT" which doesn't // obey x - 0 = x. This includes not just numeric types, but also DATE.
[nit] this last sentence is a bit confusing. I'd say something like "additive types include all numeric types as well as timestamps and dates"
pkg/sql/opt/norm/rules/numeric.opt, line 23 at r1 (raw file):
# FoldMinusZero folds $left - 0 for numeric types. This rule requires a check # that $left is numeric because JSON - INT is valid and is not a no-op with a # zero value.
Is subtraction the only operation that JSON performs differently?
Fixes cockroachdb#35612. Prior to this commit, we would unconditionally fold x - 0 to x. This was invalid in the case where x was JSON, since `- 0` in that case means "delete the 0th entry in the array". This bug existed in both the HP and CBO. This fix slightly abuses the existing "types.IsAdditiveType" function to determine whether or not the identity applies. Release note (bug fix): Subtracting 0 from a JSON array now correctly removes its first element.
There was a problem hiding this comment.
Agreed, will backport. Thanks all!
bors r+
Reviewable status:
pkg/sql/opt/norm/custom_funcs.go, line 1295 at r1 (raw file):
Previously, rytaft wrote…
returned -> returns
Done.
pkg/sql/opt/norm/custom_funcs.go, line 1298 at r1 (raw file):
Previously, rytaft wrote…
[nit] this last sentence is a bit confusing. I'd say something like "additive types include all numeric types as well as timestamps and dates"
Done.
pkg/sql/opt/norm/rules/numeric.opt, line 23 at r1 (raw file):
Previously, rytaft wrote…
Is subtraction the only operation that JSON performs differently?
It's the only one it has, there's no implementation for the other "arithmetic" operators.
35617: opt,sql: don't fold "- 0" for JSON r=justinj a=justinj Fixes #35612. Prior to this commit, we would unconditionally fold x - 0 to x. This was invalid in the case where x was JSON, since `- 0` in that case means "delete the 0th entry in the array". This bug existed in both the HP and CBO. This fix slightly abuses the existing "types.IsAdditiveType" function to determine whether or not the identity applies. Release note (bug fix): Subtracting 0 from a JSON array now correctly removes its first element. Co-authored-by: Justin Jaffray <[email protected]>
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:
Build succeeded |
Fixes #35612.
Prior to this commit, we would unconditionally fold x - 0 to x. This was
invalid in the case where x was JSON, since
- 0in that case means"delete the 0th entry in the array".
This bug existed in both the HP and CBO.
This fix slightly abuses the existing "types.IsAdditiveType" function to
determine whether or not the identity applies.
Release note (bug fix): Subtracting 0 from a JSON array now correctly
removes its first element.