opt: fix hoist of ANY comparison with tuples

Prior to this commit, when hoisting Any expressions like `<left> = ANY (SELECT <right> ...)`, we constructed `(IsNot <left|right> Null)` expressions which are equivalent to `<left|right> IS DISTINCT FROM NULL`. As discovered in cockroachdb#46675, these expressions have different behavior than `<left> IS NOT NULL` when `<left>` is a tuple. As a result, the hoisting transformations could construct invalid plans that cause incorrect results. This commit fixes the issue by using `IsTupleNotNull` expressions when `<left>` and `<right> are tupleq. Fixes cockroachdb#98691 Release note (bug fix): A bug has been fixes that caused incorrect results of ANY comparisons of tuples. For example, an expression like `(x, y) = ANY (SELECT a, b FROM t WHERE ...)` could return `true` instead of the correct result of `NULL` when `x` and `y` were `NULL`, or `a` and `b` were `NULL`. This could only occur if the subquery was correlated, i.e., it references columns from the outer part of the query. This bug was present since the cost-based optimizer was introduced in version 2.1.
mgartner · Mar 15, 2023 · a48ec29 · a48ec29
1 parent 125aa4a
commit a48ec29
Show file tree

Hide file tree

Showing 4 changed files with 268 additions and 62 deletions.
diff --git a/pkg/sql/logictest/testdata/logic_test/subquery_correlated b/pkg/sql/logictest/testdata/logic_test/subquery_correlated
@@ -1303,3 +1303,32 @@ WHERE key IN (
 3  2
 4  1
 4  3
+
+# Regression test for #98691.
+statement ok
+CREATE TABLE t98691 (
+  a INT,
+  b INT
+)
+
+statement ok
+INSERT INTO t98691 VALUES (1, 10)
+
+query B
+SELECT (NULL, NULL) = ANY (
+  SELECT a, b FROM t98691 WHERE a > i
+) FROM (VALUES (0), (0)) v(i)
+----
+NULL
+NULL
+
+statement ok
+INSERT INTO t98691 VALUES (NULL, NULL)
+
+query B
+SELECT (2, 20) = ANY (
+  SELECT a, b FROM t98691 WHERE a > i OR a IS NULL
+) FROM (VALUES (0), (0)) v(i)
+----
+NULL
+NULL
diff --git a/pkg/sql/opt/norm/decorrelate_funcs.go b/pkg/sql/opt/norm/decorrelate_funcs.go
@@ -1009,6 +1009,20 @@ func (r *subqueryHoister) constructGroupByAny(
 	aggVar := r.f.ConstructVariable(aggColID)
 	caseColID := r.f.Metadata().AddColumn("case", types.Bool)
 
+	var scalarNotNull opt.ScalarExpr
+	if scalar.DataType().Family() == types.TupleFamily {
+		scalarNotNull = r.f.ConstructIsTupleNotNull(scalar)
+	} else {
+		scalarNotNull = r.f.ConstructIsNot(scalar, memo.NullSingleton)
+	}
+
+	var inputNotNull opt.ScalarExpr
+	if inputVar.DataType().Family() == types.TupleFamily {
+		inputNotNull = r.f.ConstructIsTupleNotNull(inputVar)
+	} else {
+		inputNotNull = r.f.ConstructIsNot(inputVar, memo.NullSingleton)
+	}
+
 	return r.f.ConstructProject(
 		r.f.ConstructScalarGroupBy(
 			r.f.ConstructProject(
@@ -1022,7 +1036,7 @@ func (r *subqueryHoister) constructGroupByAny(
 					)},
 				),
 				memo.ProjectionsExpr{r.f.ConstructProjectionsItem(
-					r.f.ConstructIsNot(inputVar, memo.NullSingleton),
+					inputNotNull,
 					notNullColID,
 				)},
 				opt.ColSet{},
@@ -1042,7 +1056,7 @@ func (r *subqueryHoister) constructGroupByAny(
 					r.f.ConstructWhen(
 						r.f.ConstructAnd(
 							aggVar,
-							r.f.ConstructIsNot(scalar, memo.NullSingleton),
+							scalarNotNull,
 						),
 						r.f.ConstructTrue(),
 					),

diff --git a/pkg/sql/opt/norm/testdata/rules/combo b/pkg/sql/opt/norm/testdata/rules/combo
@@ -1252,6 +1252,74 @@ HoistProjectSubquery
   -           └── 5
   +      └── case:15 [as=r:12, outer=(15)]
 ================================================================================
+FoldNonNullIsNotNull
+  Cost: 2209.75
+================================================================================
+   project
+    ├── columns: r:12
+    ├── inner-join-apply
+    │    ├── columns: x:1!null case:15
+    │    ├── key: (1)
+    │    ├── fd: (1)-->(15)
+    │    ├── scan xy
+    │    │    ├── columns: x:1!null
+    │    │    └── key: (1)
+    │    ├── project
+    │    │    ├── columns: case:15
+    │    │    ├── outer: (1)
+    │    │    ├── cardinality: [1 - 1]
+    │    │    ├── key: ()
+    │    │    ├── fd: ()-->(15)
+    │    │    ├── scalar-group-by
+    │    │    │    ├── columns: bool_or:14
+    │    │    │    ├── outer: (1)
+    │    │    │    ├── cardinality: [1 - 1]
+    │    │    │    ├── key: ()
+    │    │    │    ├── fd: ()-->(14)
+    │    │    │    ├── project
+    │    │    │    │    ├── columns: notnull:13!null
+    │    │    │    │    ├── outer: (1)
+    │    │    │    │    ├── cardinality: [0 - 1]
+    │    │    │    │    ├── key: ()
+    │    │    │    │    ├── fd: ()-->(13)
+    │    │    │    │    ├── select
+    │    │    │    │    │    ├── columns: i:6
+    │    │    │    │    │    ├── outer: (1)
+    │    │    │    │    │    ├── cardinality: [0 - 1]
+    │    │    │    │    │    ├── key: ()
+    │    │    │    │    │    ├── fd: ()-->(6)
+    │    │    │    │    │    ├── project
+    │    │    │    │    │    │    ├── columns: i:6
+    │    │    │    │    │    │    ├── outer: (1)
+    │    │    │    │    │    │    ├── cardinality: [0 - 1]
+    │    │    │    │    │    │    ├── key: ()
+    │    │    │    │    │    │    ├── fd: ()-->(6)
+    │    │    │    │    │    │    └── select
+    │    │    │    │    │    │         ├── columns: k:5!null i:6
+    │    │    │    │    │    │         ├── outer: (1)
+    │    │    │    │    │    │         ├── cardinality: [0 - 1]
+    │    │    │    │    │    │         ├── key: ()
+    │    │    │    │    │    │         ├── fd: ()-->(5,6)
+    │    │    │    │    │    │         ├── scan a
+    │    │    │    │    │    │         │    ├── columns: k:5!null i:6
+    │    │    │    │    │    │         │    ├── key: (5)
+    │    │    │    │    │    │         │    └── fd: (5)-->(6)
+    │    │    │    │    │    │         └── filters
+    │    │    │    │    │    │              └── k:5 = x:1 [outer=(1,5), constraints=(/1: (/NULL - ]; /5: (/NULL - ]), fd=(1)==(5), (5)==(1)]
+    │    │    │    │    │    └── filters
+    │    │    │    │    │         └── (5 = i:6) IS NOT false [outer=(6)]
+    │    │    │    │    └── projections
+    │    │    │    │         └── i:6 IS NOT NULL [as=notnull:13, outer=(6)]
+    │    │    │    └── aggregations
+    │    │    │         └── bool-or [as=bool_or:14, outer=(13)]
+    │    │    │              └── notnull:13
+    │    │    └── projections
+  - │    │         └── CASE WHEN bool_or:14 AND (5 IS NOT NULL) THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
+  + │    │         └── CASE WHEN bool_or:14 AND true THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
+    │    └── filters (true)
+    └── projections
+         └── case:15 [as=r:12, outer=(15)]
+================================================================================
 CommuteVar
   Cost: 2209.75
 ================================================================================
@@ -1315,7 +1383,7 @@ CommuteVar
     │    │    │         └── bool-or [as=bool_or:14, outer=(13)]
     │    │    │              └── notnull:13
     │    │    └── projections
-    │    │         └── CASE WHEN bool_or:14 AND (5 IS NOT NULL) THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
+    │    │         └── CASE WHEN bool_or:14 AND true THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
     │    └── filters (true)
     └── projections
          └── case:15 [as=r:12, outer=(15)]
@@ -1395,7 +1463,7 @@ PushSelectIntoProject
     │    │    │         └── bool-or [as=bool_or:14, outer=(13)]
     │    │    │              └── notnull:13
     │    │    └── projections
-    │    │         └── CASE WHEN bool_or:14 AND (5 IS NOT NULL) THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
+    │    │         └── CASE WHEN bool_or:14 AND true THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
     │    └── filters (true)
     └── projections
          └── case:15 [as=r:12, outer=(15)]
@@ -1473,7 +1541,7 @@ MergeSelects
     │    │    │         └── bool-or [as=bool_or:14, outer=(13)]
     │    │    │              └── notnull:13
     │    │    └── projections
-    │    │         └── CASE WHEN bool_or:14 AND (5 IS NOT NULL) THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
+    │    │         └── CASE WHEN bool_or:14 AND true THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
     │    └── filters (true)
     └── projections
          └── case:15 [as=r:12, outer=(15)]
@@ -1554,7 +1622,7 @@ EliminateSelect
     │    │    │         └── bool-or [as=bool_or:14, outer=(13)]
     │    │    │              └── notnull:13
     │    │    └── projections
-    │    │         └── CASE WHEN bool_or:14 AND (5 IS NOT NULL) THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
+    │    │         └── CASE WHEN bool_or:14 AND true THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
     │    └── filters (true)
     └── projections
          └── case:15 [as=r:12, outer=(15)]
@@ -1624,62 +1692,7 @@ MergeProjects
     │    │    │         └── bool-or [as=bool_or:14, outer=(13)]
     │    │    │              └── notnull:13
     │    │    └── projections
-    │    │         └── CASE WHEN bool_or:14 AND (5 IS NOT NULL) THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
-    │    └── filters (true)
-    └── projections
-         └── case:15 [as=r:12, outer=(15)]
-================================================================================
-FoldNonNullIsNotNull
-  Cost: 2209.73
-================================================================================
-   project
-    ├── columns: r:12
-    ├── inner-join-apply
-    │    ├── columns: x:1!null case:15
-    │    ├── key: (1)
-    │    ├── fd: (1)-->(15)
-    │    ├── scan xy
-    │    │    ├── columns: x:1!null
-    │    │    └── key: (1)
-    │    ├── project
-    │    │    ├── columns: case:15
-    │    │    ├── outer: (1)
-    │    │    ├── cardinality: [1 - 1]
-    │    │    ├── key: ()
-    │    │    ├── fd: ()-->(15)
-    │    │    ├── scalar-group-by
-    │    │    │    ├── columns: bool_or:14
-    │    │    │    ├── outer: (1)
-    │    │    │    ├── cardinality: [1 - 1]
-    │    │    │    ├── key: ()
-    │    │    │    ├── fd: ()-->(14)
-    │    │    │    ├── project
-    │    │    │    │    ├── columns: notnull:13!null
-    │    │    │    │    ├── outer: (1)
-    │    │    │    │    ├── cardinality: [0 - 1]
-    │    │    │    │    ├── key: ()
-    │    │    │    │    ├── fd: ()-->(13)
-    │    │    │    │    ├── select
-    │    │    │    │    │    ├── columns: k:5!null i:6
-    │    │    │    │    │    ├── outer: (1)
-    │    │    │    │    │    ├── cardinality: [0 - 1]
-    │    │    │    │    │    ├── key: ()
-    │    │    │    │    │    ├── fd: ()-->(5,6)
-    │    │    │    │    │    ├── scan a
-    │    │    │    │    │    │    ├── columns: k:5!null i:6
-    │    │    │    │    │    │    ├── key: (5)
-    │    │    │    │    │    │    └── fd: (5)-->(6)
-    │    │    │    │    │    └── filters
-    │    │    │    │    │         ├── k:5 = x:1 [outer=(1,5), constraints=(/1: (/NULL - ]; /5: (/NULL - ]), fd=(1)==(5), (5)==(1)]
-    │    │    │    │    │         └── (i:6 = 5) IS NOT false [outer=(6)]
-    │    │    │    │    └── projections
-    │    │    │    │         └── i:6 IS NOT NULL [as=notnull:13, outer=(6)]
-    │    │    │    └── aggregations
-    │    │    │         └── bool-or [as=bool_or:14, outer=(13)]
-    │    │    │              └── notnull:13
-    │    │    └── projections
-  - │    │         └── CASE WHEN bool_or:14 AND (5 IS NOT NULL) THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
-  + │    │         └── CASE WHEN bool_or:14 AND true THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
+    │    │         └── CASE WHEN bool_or:14 AND true THEN true WHEN bool_or:14 IS NULL THEN false ELSE CAST(NULL AS BOOL) END [as=case:15, outer=(14)]
     │    └── filters (true)
     └── projections
          └── case:15 [as=r:12, outer=(15)]