Merge #35617

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]>

pkg/sql/logictest/testdata/logic_test/json

@@ -498,6 +498,11 @@ SELECT '{"a": 1}'::JSONB - 'b'
 {"a": 1}
 
 # `-` is one of the very few cases that PG errors in a JSON type mismatch with operators.
+query T
+SELECT '[1,2,3]'::JSONB - 0
+----
+[2, 3]
+
 query T
 SELECT '[1,2,3]'::JSONB - 1
 ----

pkg/sql/opt/norm/custom_funcs.go

@@ -1292,6 +1292,15 @@ func (c *CustomFuncs) MakeLimited(sub *memo.SubqueryPrivate) *memo.SubqueryPriva
 //
 // ----------------------------------------------------------------------
 
+// IsAdditive returns true if the type of the expression supports addition and
+// 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. Additive types include all numeric types as well as
+// timestamps and dates.
+func (c *CustomFuncs) IsAdditive(e opt.ScalarExpr) bool {
+	return types.IsAdditiveType(e.DataType())
+}
+
 // EqualsNumber returns true if the given numeric value (decimal, float, or
 // integer) is equal to the given integer value.
 func (c *CustomFuncs) EqualsNumber(datum tree.Datum, value int64) bool {

pkg/sql/opt/norm/rules/numeric.opt

@@ -18,9 +18,11 @@
 [FoldZeroPlus, Normalize]
 (Plus $left:(Const 0) $right:*) => (Cast $right (BinaryColType Plus $left $right))
 
-# FoldMinusZero folds $left - 0 for numeric types.
+# 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.
 [FoldMinusZero, Normalize]
-(Minus $left:* $right:(Const 0)) => (Cast $left (BinaryColType Minus $left $right))
+(Minus $left:(IsAdditive $left) $right:(Const 0)) => (Cast $left (BinaryColType Minus $left $right))
 
 # FoldMultOne folds $left * 1 for numeric types.
 [FoldMultOne, Normalize]

pkg/sql/opt/norm/testdata/rules/numeric

@@ -88,6 +88,17 @@ values
  ├── fd: ()-->(1)
  └── (0,) [type=tuple{decimal}]
 
+# Regression test for #35612.
+opt expect-not=FoldMinusZero
+SELECT '[123]'::jsonb - 0
+----
+values
+ ├── columns: "?column?":1(jsonb)
+ ├── cardinality: [1 - 1]
+ ├── key: ()
+ ├── fd: ()-->(1)
+ └── ('[]',) [type=tuple{jsonb}]
+
 # --------------------------------------------------
 # FoldMultOne, FoldOneMult
 # --------------------------------------------------

pkg/sql/sem/tree/normalize.go

@@ -140,7 +140,7 @@ func (expr *BinaryExpr) normalize(v *NormalizeVisitor) TypedExpr {
 			break
 		}
 	case Minus:
-		if v.isNumericZero(right) {
+		if types.IsAdditiveType(left.ResolvedType()) && v.isNumericZero(right) {
 			final, v.err = ReType(left, expectedType)
 			break
 		}