Merge #113106

113106: sql: add regression test for previously fixed JSON bug r=mgartner a=mgartner

#### opt: remove unnecessary argument in memo.CanBeCompositeSensitive

Release note: None

#### sql: add regression test for previously fixed JSON bug

This commit adds a regression test for a bug that was unknowingly fixed
in #99275. See #113103 for more details.

Epic: None

Release note: None


Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>

pkg/sql/logictest/testdata/logic_test/json

@@ -973,3 +973,32 @@ SELECT * FROM test_49144 WHERE ("test_49144"."value" -> 'c') > '2.33' ORDER BY 1
 ----
 {"c": 2.5}
 {"c": 3}
+
+# Regression test for #113103. Expressions with JSON values are
+# composite-sensitive. The optimizer should not assume otherwise and perform
+# optimizations that produce incorrect results.
+subtest regression_113103
+
+# Order three text values that are cast from logically equivalent JSON values.
+query T
+SELECT j::TEXT
+FROM (VALUES ('1.0'::JSON), ('1'::JSON), ('1.00'::JSON)) v(j)
+ORDER BY 1
+----
+1
+1.0
+1.00
+
+# Add a filter that holds the JSON values constant to 1. The output should be
+# the same as above.
+query T
+SELECT j::TEXT
+FROM (VALUES ('1.0'::JSON), ('1'::JSON), ('1.00'::JSON)) v(j)
+WHERE j = '1'::JSON
+ORDER BY 1
+----
+1
+1.0
+1.00
+
+subtest end

pkg/sql/opt/memo/expr.go

@@ -1083,7 +1083,7 @@ func (prj *ProjectExpr) initUnexportedFields(mem *Memo) {
 			// This does not necessarily hold for "composite" types like decimals or
 			// collated strings. For example if d is a decimal, d::TEXT can have
 			// different values for equal values of d, like 1 and 1.0.
-			if !CanBeCompositeSensitive(mem.Metadata(), item.Element) {
+			if !CanBeCompositeSensitive(item.Element) {
 				prj.internalFuncDeps.AddSynthesizedCol(from, item.Col)
 			}
 		}

pkg/sql/opt/memo/logical_props_builder.go

@@ -1586,7 +1586,7 @@ func (b *logicalPropsBuilder) buildFiltersItemProps(item *FiltersItem, scalar *p
 				// then x is functionally determined by the columns that appear in the
 				// expression.
 				if !scalar.VolatilitySet.HasVolatile() &&
-					!CanBeCompositeSensitive(b.mem.Metadata(), eq.Right) {
+					!CanBeCompositeSensitive(eq.Right) {
 					outerCols := getOuterCols(eq.Right)
 					if !outerCols.Contains(leftVar.Col) {
 						scalar.FuncDeps.AddSynthesizedCol(getOuterCols(eq.Right), leftVar.Col)
@@ -1939,7 +1939,7 @@ func MakeTableFuncDep(md *opt.Metadata, tabID opt.TableID) *props.FuncDepSet {
 			// This does not necessarily hold for "composite" types like decimals or
 			// collated strings. For example if d is a decimal, d::TEXT can have
 			// different values for equal values of d, like 1 and 1.0.
-			if !CanBeCompositeSensitive(md, expr) {
+			if !CanBeCompositeSensitive(expr) {
 				fd.AddSynthesizedCol(from, colID)
 			}
 		}
@@ -2832,7 +2832,7 @@ func deriveWithUses(r opt.Expr) props.WithUsesMap {
 // This property is used to determine when a scalar expression can be copied,
 // with outer column variable references changed to refer to other columns that
 // are known to be equal to the original columns.
-func CanBeCompositeSensitive(md *opt.Metadata, e opt.Expr) bool {
+func CanBeCompositeSensitive(e opt.Expr) bool {
 	// check is a recursive function which returns the following:
 	//  - isCompositeInsensitive as defined above.
 	//  - isCompositeIdentical is a stronger property, which says that for equal

pkg/sql/opt/memo/memo_test.go

@@ -103,7 +103,7 @@ func TestCompositeSensitive(t *testing.T) {
 		if err != nil {
 			d.Fatalf(t, "error building: %v", err)
 		}
-		return fmt.Sprintf("%v", memo.CanBeCompositeSensitive(md, scalar))
+		return fmt.Sprintf("%v", memo.CanBeCompositeSensitive(scalar))
 	})
 }
 

pkg/sql/opt/norm/general_funcs.go

@@ -1022,7 +1022,7 @@ func (c *CustomFuncs) tryFoldComputedCol(
 	}
 
 	computedCol := ComputedCols[computedColID]
-	if memo.CanBeCompositeSensitive(c.f.mem.Metadata(), computedCol) {
+	if memo.CanBeCompositeSensitive(computedCol) {
 		// The computed column expression can return different values for logically
 		// equal outer columns (e.g. d::STRING where d is a DECIMAL).
 		return false

pkg/sql/opt/norm/join_funcs.go

@@ -295,7 +295,7 @@ func (c *CustomFuncs) CanMapJoinOpFilter(
 		return false
 	}
 
-	allowCompositeEncoding := !memo.CanBeCompositeSensitive(c.mem.Metadata(), src)
+	allowCompositeEncoding := !memo.CanBeCompositeSensitive(src)
 
 	// For CanMapJoinOpFilter to be true, each column in src must map to at
 	// least one column in dst.
@@ -339,7 +339,7 @@ func (c *CustomFuncs) MapJoinOpFilter(
 		return src.Condition
 	}
 
-	allowCompositeEncoding := !memo.CanBeCompositeSensitive(c.mem.Metadata(), src)
+	allowCompositeEncoding := !memo.CanBeCompositeSensitive(src)
 
 	// Map each column in src to one column in dst. We choose an arbitrary column
 	// (the one with the smallest ColumnID) if there are multiple choices.

pkg/sql/opt/norm/select_funcs.go

@@ -21,7 +21,7 @@ import (
 // CanMapOnSetOp determines whether the filter can be mapped to either
 // side of a set operator.
 func (c *CustomFuncs) CanMapOnSetOp(filter *memo.FiltersItem) bool {
-	if memo.CanBeCompositeSensitive(c.mem.Metadata(), filter) {
+	if memo.CanBeCompositeSensitive(filter) {
 		// In general, it is not safe to remap a composite-sensitive filter.
 		// For example:
 		//  - the set operation is Except

pkg/sql/opt/optbuilder/fk_cascade.go

@@ -189,7 +189,7 @@ func tryNewOnDeleteFastCascadeBuilder(
 		if !p.OuterCols.SubsetOf(fkCols.ToSet()) {
 			return nil, false
 		}
-		if memo.CanBeCompositeSensitive(md, &sel.Filters) {
+		if memo.CanBeCompositeSensitive(&sel.Filters) {
 			return nil, false
 		}
 		if sel.Relational().HasSubquery {