Merge #71226 #71231

71226: rowexec: fix zigzag joiner with ON expression in some cases r=yuzefovich a=yuzefovich

Zigzag joiner needs to tell the row fetcher which columns are needed.
Previously, we forgot to include the columns that are needed by the ON
expression but are not needed in the output, so when evaluating such an
ON expression, we would hit an internal error. This commit fixes the
problem by including all columns referenced by the ON expression into
the set of columns to be fetched.

Fixes: #71093

Release note (bug fix): Previously, CockroachDB could encounter an
internal error when executing a zigzag join in some cases (when there
are multiple filters present and at least one filter refers to the
column that is part of STORING clause of the secondary index that is
used by the zigzag join), and this has been fixed.

71231: Added SHOW CREATE SCHEDULES diagram r=ericharmeling a=ericharmeling

Related to cockroachdb/docs#11008

@kathancox 

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Eric Harmeling <[email protected]>

docs/generated/sql/bnf/BUILD.bazel

@@ -183,6 +183,7 @@ genrule(
         "show_constraints.bnf",
         "show_constraints_stmt.bnf",
         "show_create_stmt.bnf",
+        "show_create_schedules_stmt.bnf",
         "show_databases_stmt.bnf",
         "show_default_privileges_stmt.bnf",
         "show_enums.bnf",

docs/generated/sql/bnf/show_create_schedules_stmt.bnf

@@ -0,0 +1,3 @@
+show_create_schedules_stmt ::=
+	'SHOW' 'CREATE' 'ALL' 'SCHEDULES'
+	| 'SHOW' 'CREATE' 'SCHEDULE' a_expr

docs/generated/sql/bnf/show_var.bnf

@@ -3,6 +3,7 @@ show_stmt ::=
 	| show_columns_stmt
 	| show_constraints_stmt
 	| show_create_stmt
+	| show_create_schedules_stmt
 	| show_csettings_stmt
 	| show_databases_stmt
 	| show_enums_stmt

docs/generated/sql/bnf/stmt_block.bnf

@@ -215,6 +215,7 @@ show_stmt ::=
 	| show_columns_stmt
 	| show_constraints_stmt
 	| show_create_stmt
+	| show_create_schedules_stmt
 	| show_csettings_stmt
 	| show_databases_stmt
 	| show_enums_stmt
@@ -647,6 +648,10 @@ show_create_stmt ::=
 	'SHOW' 'CREATE' table_name
 	| 'SHOW' 'CREATE' 'ALL' 'TABLES'
 
+show_create_schedules_stmt ::=
+	'SHOW' 'CREATE' 'ALL' 'SCHEDULES'
+	| 'SHOW' 'CREATE' 'SCHEDULE' a_expr
+
 show_csettings_stmt ::=
 	'SHOW' 'CLUSTER' 'SETTING' var_name
 	| 'SHOW' 'CLUSTER' 'SETTING' 'ALL'

pkg/cmd/docgen/diagrams.go

@@ -1362,6 +1362,9 @@ var specs = []stmtSpec{
 		stmt:   "show_schedules_stmt",
 		inline: []string{"schedule_state", "opt_schedule_executor_type"},
 	},
+	{
+		name: "show_create_schedules_stmt",
+	},
 	{
 		name: "show_schemas",
 		stmt: "show_schemas_stmt",

pkg/sql/logictest/testdata/logic_test/zigzag_join

@@ -250,3 +250,27 @@ SELECT * FROM a@{FORCE_ZIGZAG=[6],FORCE_ZIGZAG=a_idx} WHERE a = 3 AND b = 7
 
 statement error FORCE_ZIGZAG cannot be specified in conjunction with NO_ZIGZAG_JOIN
 SELECT * FROM a@{FORCE_ZIGZAG,NO_ZIGZAG_JOIN} WHERE a = 3 AND b = 7
+
+# Regression tests for not fetching columns that are only needed by the ON
+# expression (#71093).
+statement ok
+CREATE TABLE t71093 (a INT, b INT, c INT, d INT, INDEX a_idx(a) STORING (b), INDEX c_idx(c) STORING (d));
+INSERT INTO t71093 VALUES (0, 1, 2, 3)
+
+# ON expr needs the stored column from the left side.
+query I
+SELECT count(*) FROM t71093 WHERE a = 0 AND b = 1 AND c = 2
+----
+1
+
+# ON expr needs the stored column from the right side.
+query I
+SELECT count(*) FROM t71093 WHERE a = 0 AND c = 2 AND d = 3
+----
+1
+
+# ON expr needs the stored columns from both sides.
+query I
+SELECT count(*) FROM t71093 WHERE a = 0 AND b = 1 AND c = 2 AND d = 3
+----
+1

pkg/sql/opt/exec/execbuilder/testdata/zigzag_join

@@ -0,0 +1,57 @@
+# LogicTest: local
+
+# Make sure that the zigzag join is used in the regression tests for #71093.
+statement ok
+CREATE TABLE t71093 (a INT, b INT, c INT, d INT, INDEX a_idx(a) STORING (b), INDEX c_idx(c) STORING (d));
+INSERT INTO t71093 VALUES (0, 1, 2, 3)
+
+query T
+EXPLAIN SELECT count(*) FROM t71093 WHERE a = 0 AND b = 1 AND c = 2
+----
+distribution: local
+vectorized: true
+·
+• group (scalar)
+│
+└── • zigzag join
+      pred: ((a = 0) AND (b = 1)) AND (c = 2)
+      left table: t71093@a_idx
+      left columns: (a, b)
+      left fixed values: 1 column
+      right table: t71093@c_idx
+      right columns: (c)
+      right fixed values: 1 column
+
+query T
+EXPLAIN SELECT count(*) FROM t71093 WHERE a = 0 AND c = 2 AND d = 3
+----
+distribution: local
+vectorized: true
+·
+• group (scalar)
+│
+└── • zigzag join
+      pred: ((a = 0) AND (c = 2)) AND (d = 3)
+      left table: t71093@a_idx
+      left columns: (a)
+      left fixed values: 1 column
+      right table: t71093@c_idx
+      right columns: (c, d)
+      right fixed values: 1 column
+
+query T
+EXPLAIN SELECT count(*) FROM t71093 WHERE a = 0 AND b = 1 AND c = 2 AND d = 3
+----
+distribution: local
+vectorized: true
+·
+• group (scalar)
+│
+└── • zigzag join
+      pred: (((a = 0) AND (b = 1)) AND (c = 2)) AND (d = 3)
+      left table: t71093@a_idx
+      left columns: (a, b)
+      left fixed values: 1 column
+      right table: t71093@c_idx
+      right columns: (c, d)
+      right fixed values: 1 column

pkg/sql/parser/sql.y

@@ -5667,13 +5667,11 @@ show_create_stmt:
 show_create_schedules_stmt:
   SHOW CREATE ALL SCHEDULES
   {
-    /* SKIP DOC */
     $$.val = &tree.ShowCreateSchedules{}
   }
 | SHOW CREATE ALL SCHEDULES error // SHOW HELP: SHOW CREATE SCHEDULES
 | SHOW CREATE SCHEDULE a_expr
   {
-    /* SKIP DOC */
     $$.val = &tree.ShowCreateSchedules{ScheduleID: $4.expr()}
   }
 | SHOW CREATE SCHEDULE error // SHOW HELP: SHOW CREATE SCHEDULES

pkg/sql/rowexec/zigzagjoiner.go

@@ -466,6 +466,15 @@ func (z *zigzagJoiner) setupInfo(
 		neededCols.Add(int(col))
 	}
 
+	// Add columns needed by OnExpr.
+	for _, v := range z.onCond.Vars.GetIndexedVars() {
+		// We only include the columns that come from this side (all such
+		// columns have the ordinals in [colOffset, maxCol) range).
+		if v.Idx >= colOffset && v.Idx < maxCol {
+			neededCols.Add(v.Idx - colOffset)
+		}
+	}
+
 	// Setup the RowContainers.
 	info.container.Reset()