Merge #35321 #35587 #35630 #35644 #35651

35321: opt: propagate set operation output types to input columns r=rytaft a=rytaft

This commit updates the `optbuilder` logic for set operations in which
the types of the input columns do not match the types of the output
columns. This can happen if a column on one side has type Unknown,
but the corresponding column on the other side has a known type such
as Int. The known type must be propagated to the side with the unknown
type to prevent errors in the execution engine related to decoding
types.

If there are any column types on either side that don't match the output,
then the `optbuilder` propagates the output types of the set operation down
to the input columns by wrapping the side with mismatched types in a
Project operation. The Project operation passes through columns that
already have the correct type, and creates cast expressions for those
that don't.

Fixes #34524

Release note (bug fix): Fixed an error that happened when executing
some set operations containing only nulls in one of the input columns.

35587: opt: add more cost for lookup joins with more ON conditions r=RaduBerinde a=RaduBerinde

This is a very limited fix for #34810.

The core problem is that we don't take into account that if we have an
ON condition, not only there's a cost to evaluate it on each row, but
we are generating more internal rows to get a given number of output
rows.

I attempted to do a more general fix (for all join types), where I
tried to estimate the "internal" number of rows using
`unknownFilterSelectivity` for each ON condition. There were two
problems:
 - in some cases (especially with lookup joins) we have an extra ON
   condition that doesn't actually do anything:
     `ab JOIN xy ON a=x AND a=10` becomes
     `ab JOIN xy ON a=x AND a=10 AND x=10` becomes
   and `a=10` could remain as an ON condition. This results in bad
   query plans in important cases (e.g. TPCC) where it prefers to do
   an extra lookup join (due to a non-covering index) just because of
   that condition.

 - we don't have the equality columns readily available for hash join
   (and didn't want to extract them each time we cost). In the future
   we may split the planning into a logical and physical stage, and we
   should then separate the logical joins from hash join.

For 19.1, we simply simply add a cost for lookup joins that is
proportional to the number of remaining ON conditions. This is the
least disruptive method that still fixes the case observed in #34810.
I will leave the issue open to address this properly in the next
release.

Note that although hash joins and merge joins have the same issue in
principle, in practice we always generate these expressions with
equality on all possible columns.

Release note: None

35630: storage/tscache: Pick up andy-kimball/arenaskl fix r=nvanbenschoten a=nvanbenschoten

Fixes #31624.
Fixes #35557.

This commit picks up andy-kimball/arenaskl#4.

I strongly suspect that the uint32 overflow fixed in that PR was the
cause of the two index out of bounds panics. See that commit for more
details.

The PR also fixes a bug in memory recylcling within the tscache. I confirmed
on adriatic that over 900 64MB arenas had been allocated since it was last
wiped.

35644: opt: use correct ordering for insert input in execbuilder r=RaduBerinde a=RaduBerinde

We were setting up a projection on the Insert's input but we were
accidentally using the parent Insert's ordering instead of that of the
input.

Fixes #35564.

Release note (bug fix): Fixed a "column not in input" crash when
`INSERT ... RETURNING` is used inside a clause that requires an
ordering.

35651: jobs, sql, ui: Create `AutoCreateStats` job type r=celiala a=celiala

With #34279, enabling the cluster setting
`sql.stats.experimental_automatic_collection.enabled` has the potential
to create many CreateStats jobs, which can cause the Jobs view on the
AdminUI to become cluttered.

This commit creates a new `AutoCreateStats` job type for these auto-created
CreateStats jobs, so that users are able to still see their own manual runs
of CREATE STATISTICS, via the pre-existing `CreateStats` type.

cc @danhhz, @piyush-singh, @rolandcrosby 

![jobs-auto-create-stats](https://user-images.githubusercontent.com/3051672/54212467-5cea2c80-44b9-11e9-9c11-db749814f019.gif)

Release note (admin ui change): AutoCreateStats type added to
Jobs page to filter automatic statistics jobs.

Fixes #34377.

Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Celia La <[email protected]>

pkg/jobs/jobspb/jobs.proto

@@ -281,4 +281,5 @@ enum Type {
   IMPORT = 4 [(gogoproto.enumvalue_customname) = "TypeImport"];
   CHANGEFEED = 5 [(gogoproto.enumvalue_customname) = "TypeChangefeed"];
   CREATE_STATS = 6 [(gogoproto.enumvalue_customname) = "TypeCreateStats"];
+  AUTO_CREATE_STATS = 7 [(gogoproto.enumvalue_customname) = "TypeAutoCreateStats"];
 }

pkg/jobs/jobspb/wrap.go

@@ -19,6 +19,7 @@ import (
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/stats"
 )
 
 // Details is a marker interface for job details proto structs.
@@ -58,6 +59,10 @@ func DetailsType(d isPayload_Details) Type {
 	case *Payload_Changefeed:
 		return TypeChangefeed
 	case *Payload_CreateStats:
+		createStatsName := d.(*Payload_CreateStats).CreateStats.Name
+		if createStatsName == stats.AutoStatsName {
+			return TypeAutoCreateStats
+		}
 		return TypeCreateStats
 	default:
 		panic(fmt.Sprintf("Payload.Type called on a payload with an unknown details type: %T", d))

pkg/sql/create_stats.go

@@ -383,7 +383,7 @@ func checkRunningJobs(ctx context.Context, job *jobs.Job, p *planner) error {
 			return err
 		}
 
-		if payload.Type() == jobspb.TypeCreateStats {
+		if payload.Type() == jobspb.TypeCreateStats || payload.Type() == jobspb.TypeAutoCreateStats {
 			id := (*int64)(row[0].(*tree.DInt))
 			if *id == jobID {
 				break
@@ -443,7 +443,7 @@ func (r *createStatsResumer) OnTerminal(
 
 func init() {
 	jobs.AddResumeHook(func(typ jobspb.Type, settings *cluster.Settings) jobs.Resumer {
-		if typ != jobspb.TypeCreateStats {
+		if typ != jobspb.TypeCreateStats && typ != jobspb.TypeAutoCreateStats {
 			return nil
 		}
 		return &createStatsResumer{}

pkg/sql/opt/exec/execbuilder/relational_builder.go

@@ -1297,7 +1297,7 @@ func (b *Builder) buildInsert(ins *memo.InsertExpr) (execPlan, error) {
 	colList := make(opt.ColList, 0, len(ins.InsertCols)+len(ins.CheckCols))
 	colList = appendColsWhenPresent(colList, ins.InsertCols)
 	colList = appendColsWhenPresent(colList, ins.CheckCols)
-	input, err = b.ensureColumns(input, colList, nil, ins.ProvidedPhysical().Ordering)
+	input, err = b.ensureColumns(input, colList, nil, ins.Input.ProvidedPhysical().Ordering)
 	if err != nil {
 		return execPlan{}, err
 	}
@@ -1348,7 +1348,7 @@ func (b *Builder) buildUpdate(upd *memo.UpdateExpr) (execPlan, error) {
 	colList = appendColsWhenPresent(colList, upd.FetchCols)
 	colList = appendColsWhenPresent(colList, upd.UpdateCols)
 	colList = appendColsWhenPresent(colList, upd.CheckCols)
-	input, err = b.ensureColumns(input, colList, nil, upd.ProvidedPhysical().Ordering)
+	input, err = b.ensureColumns(input, colList, nil, upd.Input.ProvidedPhysical().Ordering)
 	if err != nil {
 		return execPlan{}, err
 	}
@@ -1414,7 +1414,7 @@ func (b *Builder) buildUpsert(ups *memo.UpsertExpr) (execPlan, error) {
 		colList = append(colList, ups.CanaryCol)
 	}
 	colList = appendColsWhenPresent(colList, ups.CheckCols)
-	input, err = b.ensureColumns(input, colList, nil, ups.ProvidedPhysical().Ordering)
+	input, err = b.ensureColumns(input, colList, nil, ups.Input.ProvidedPhysical().Ordering)
 	if err != nil {
 		return execPlan{}, err
 	}
@@ -1473,7 +1473,7 @@ func (b *Builder) buildDelete(del *memo.DeleteExpr) (execPlan, error) {
 	// Upgrade execution engine to not require this.
 	colList := make(opt.ColList, 0, len(del.FetchCols))
 	colList = appendColsWhenPresent(colList, del.FetchCols)
-	input, err = b.ensureColumns(input, colList, nil, del.ProvidedPhysical().Ordering)
+	input, err = b.ensureColumns(input, colList, nil, del.Input.ProvidedPhysical().Ordering)
 	if err != nil {
 		return execPlan{}, err
 	}

pkg/sql/opt/exec/execbuilder/testdata/insert

@@ -467,3 +467,30 @@ count             ·              ·                      ()
 
 statement ok
 ROLLBACK
+
+# Regression test for #35564: make sure we use the Insert's input required
+# ordering for the internal projection.
+
+statement ok
+CREATE TABLE abc (a INT, b INT, c INT, INDEX(c) STORING(a,b))
+
+statement ok
+CREATE TABLE xyz (x INT, y INT, z INT)
+
+query TTTTT
+EXPLAIN (VERBOSE) SELECT * FROM [INSERT INTO xyz SELECT a, b, c FROM abc RETURNING z] ORDER BY z
+----
+render                    ·         ·                    (z)                       +z
+ │                        render 0  z                    ·                         ·
+ └── run                  ·         ·                    (x, y, z, rowid[hidden])  ·
+      └── insert          ·         ·                    (x, y, z, rowid[hidden])  ·
+           │              into      xyz(x, y, z, rowid)  ·                         ·
+           │              strategy  inserter             ·                         ·
+           └── render     ·         ·                    (a, b, c, column9)        +c
+                │         render 0  a                    ·                         ·
+                │         render 1  b                    ·                         ·
+                │         render 2  c                    ·                         ·
+                │         render 3  unique_rowid()       ·                         ·
+                └── scan  ·         ·                    (a, b, c)                 +c
+·                         table     abc@abc_c_idx        ·                         ·
+·                         spans     ALL                  ·                         ·

pkg/sql/opt/exec/execbuilder/testdata/union

@@ -1,4 +1,4 @@
-# LogicTest: local-opt
+# LogicTest: local-opt fakedist-opt
 
 statement ok
 CREATE TABLE uniontest (
@@ -101,3 +101,12 @@ render                 ·              ·                 ("?column?", "?column?
 ·                      row 0, expr 0  ''                ·                                     ·
 ·                      row 0, expr 1  ''                ·                                     ·
 ·                      row 0, expr 2  'x'               ·                                     ·
+
+statement ok
+CREATE TABLE a (a INT PRIMARY KEY)
+
+# Regression test for #34524. This test is here because the issue still exists
+# in the heuristic planner.
+query I
+(SELECT NULL FROM a) EXCEPT (VALUES((SELECT 1 FROM a LIMIT 1)), (1))
+----

pkg/sql/opt/exec/execbuilder/testdata/update

@@ -296,3 +296,29 @@ Del /Table/57/1/1/1/1
 Del /Table/57/1/1/2/1
 fast path completed
 rows affected: 1
+
+# Regression test for #35564: make sure we use the Update's input required
+# ordering for the internal projection.
+
+statement ok
+CREATE TABLE abc (a INT, b INT, c INT, INDEX(c) STORING(a,b))
+
+query TTTTT
+EXPLAIN (VERBOSE) SELECT * FROM [ UPDATE abc SET a=c RETURNING a ] ORDER BY a
+----
+render                    ·         ·              (a)                       +a
+ │                        render 0  a              ·                         ·
+ └── run                  ·         ·              (a, b, c, rowid[hidden])  ·
+      └── update          ·         ·              (a, b, c, rowid[hidden])  ·
+           │              table     abc            ·                         ·
+           │              set       a              ·                         ·
+           │              strategy  updater        ·                         ·
+           └── render     ·         ·              (a, b, c, rowid, c)       +c
+                │         render 0  a              ·                         ·
+                │         render 1  b              ·                         ·
+                │         render 2  c              ·                         ·
+                │         render 3  rowid          ·                         ·
+                │         render 4  c              ·                         ·
+                └── scan  ·         ·              (a, b, c, rowid[hidden])  +c
+·                         table     abc@abc_c_idx  ·                         ·
+·                         spans     ALL            ·                         ·

pkg/sql/opt/exec/execbuilder/testdata/upsert

@@ -307,3 +307,38 @@ INSERT INTO t5 VALUES (1, 10, 9) ON CONFLICT (k) DO NOTHING
 
 statement ok
 INSERT INTO t5 VALUES (1, 10, 20) ON CONFLICT (k) DO NOTHING
+
+# Regression test for #35564: make sure we use the Upsert's input required
+# ordering for the internal projection.
+
+statement ok
+CREATE TABLE abc (a INT, b INT, c INT, INDEX(c) STORING(a,b))
+
+statement ok
+CREATE TABLE xyz (x INT, y INT, z INT)
+
+query TTTTT
+EXPLAIN (VERBOSE) SELECT * FROM [UPSERT INTO xyz SELECT a, b, c FROM abc RETURNING z] ORDER BY z
+----
+render                         ·         ·                    (z)                          +z
+ │                             render 0  z                    ·                            ·
+ └── run                       ·         ·                    (x, y, z, rowid[hidden])     ·
+      └── upsert               ·         ·                    (x, y, z, rowid[hidden])     ·
+           │                   into      xyz(x, y, z, rowid)  ·                            ·
+           │                   strategy  opt upserter         ·                            ·
+           └── render          ·         ·                    (a, b, c, column9, a, b, c)  +c
+                │              render 0  a                    ·                            ·
+                │              render 1  b                    ·                            ·
+                │              render 2  c                    ·                            ·
+                │              render 3  column9              ·                            ·
+                │              render 4  a                    ·                            ·
+                │              render 5  b                    ·                            ·
+                │              render 6  c                    ·                            ·
+                └── render     ·         ·                    (column9, a, b, c)           +c
+                     │         render 0  unique_rowid()       ·                            ·
+                     │         render 1  a                    ·                            ·
+                     │         render 2  b                    ·                            ·
+                     │         render 3  c                    ·                            ·
+                     └── scan  ·         ·                    (a, b, c)                    +c
+·                              table     abc@abc_c_idx        ·                            ·
+·                              spans     ALL                  ·                            ·

pkg/sql/opt/norm/custom_funcs.go

@@ -658,32 +658,6 @@ func (c *CustomFuncs) AreProjectionsCorrelated(
 	return false
 }
 
-// ProjectColMapLeft returns a Projections operator that maps the left side
-// columns in a SetPrivate to the output columns in it. Useful for replacing set
-// operations with simpler constructs.
-func (c *CustomFuncs) ProjectColMapLeft(set *memo.SetPrivate) memo.ProjectionsExpr {
-	return c.projectColMapSide(set.OutCols, set.LeftCols)
-}
-
-// ProjectColMapRight returns a Project operator that maps the right side
-// columns in a SetPrivate to the output columns in it. Useful for replacing set
-// operations with simpler constructs.
-func (c *CustomFuncs) ProjectColMapRight(set *memo.SetPrivate) memo.ProjectionsExpr {
-	return c.projectColMapSide(set.OutCols, set.RightCols)
-}
-
-// projectColMapSide implements the side-agnostic logic from ProjectColMapLeft
-// and ProjectColMapRight.
-func (c *CustomFuncs) projectColMapSide(toList, fromList opt.ColList) memo.ProjectionsExpr {
-	items := make(memo.ProjectionsExpr, len(toList))
-	for idx, fromCol := range fromList {
-		toCol := toList[idx]
-		items[idx].Element = c.f.ConstructVariable(fromCol)
-		items[idx].Col = toCol
-	}
-	return items
-}
-
 // MakeEmptyColSet returns a column set with no columns in it.
 func (c *CustomFuncs) MakeEmptyColSet() opt.ColSet {
 	return opt.ColSet{}
@@ -885,6 +859,39 @@ func (c *CustomFuncs) ZipOuterCols(zip memo.ZipExpr) opt.ColSet {
 	return colSet
 }
 
+// ----------------------------------------------------------------------
+//
+// Set Rules
+//   Custom match and replace functions used with set.opt rules.
+//
+// ----------------------------------------------------------------------
+
+// ProjectColMapLeft returns a Projections operator that maps the left side
+// columns in a SetPrivate to the output columns in it. Useful for replacing set
+// operations with simpler constructs.
+func (c *CustomFuncs) ProjectColMapLeft(set *memo.SetPrivate) memo.ProjectionsExpr {
+	return c.projectColMapSide(set.OutCols, set.LeftCols)
+}
+
+// ProjectColMapRight returns a Project operator that maps the right side
+// columns in a SetPrivate to the output columns in it. Useful for replacing set
+// operations with simpler constructs.
+func (c *CustomFuncs) ProjectColMapRight(set *memo.SetPrivate) memo.ProjectionsExpr {
+	return c.projectColMapSide(set.OutCols, set.RightCols)
+}
+
+// projectColMapSide implements the side-agnostic logic from ProjectColMapLeft
+// and ProjectColMapRight.
+func (c *CustomFuncs) projectColMapSide(toList, fromList opt.ColList) memo.ProjectionsExpr {
+	items := make(memo.ProjectionsExpr, len(toList))
+	for idx, fromCol := range fromList {
+		toCol := toList[idx]
+		items[idx].Element = c.f.ConstructVariable(fromCol)
+		items[idx].Col = toCol
+	}
+	return items
+}
+
 // ----------------------------------------------------------------------
 //
 // Boolean Rules

pkg/sql/opt/norm/rules/select.opt

@@ -344,33 +344,3 @@
     $input
     (RemoveFiltersItem $filters $item)
 )
-
-# EliminateUnionAllLeft replaces a union all with a right side having a
-# cardinality of zero, with just the left side operand.
-[EliminateUnionAllLeft, Normalize]
-(UnionAll
-    $left:*
-    $right:* & (HasZeroRows $right)
-    $colmap:*
-)
-=>
-(Project
-    $left
-    (ProjectColMapLeft $colmap)
-    (MakeEmptyColSet)
-)
-
-# EliminateUnionAllRight replaces a union all with a left side having a
-# cardinality of zero, with just the right side operand.
-[EliminateUnionAllRight, Normalize]
-(UnionAll
-    $left:* & (HasZeroRows $left)
-    $right:*
-    $colmap:*
-)
-=>
-(Project
-    $right
-    (ProjectColMapRight $colmap)
-    (MakeEmptyColSet)
-)

pkg/sql/opt/norm/rules/set.opt

@@ -0,0 +1,33 @@
+# =============================================================================
+# set.opt contains normalization rules for set operators.
+# =============================================================================
+
+# EliminateUnionAllLeft replaces a union all with a right side having a
+# cardinality of zero, with just the left side operand.
+[EliminateUnionAllLeft, Normalize]
+(UnionAll
+    $left:*
+    $right:* & (HasZeroRows $right)
+    $colmap:*
+)
+=>
+(Project
+    $left
+    (ProjectColMapLeft $colmap)
+    (MakeEmptyColSet)
+)
+
+# EliminateUnionAllRight replaces a union all with a left side having a
+# cardinality of zero, with just the right side operand.
+[EliminateUnionAllRight, Normalize]
+(UnionAll
+    $left:* & (HasZeroRows $left)
+    $right:*
+    $colmap:*
+)
+=>
+(Project
+    $right
+    (ProjectColMapRight $colmap)
+    (MakeEmptyColSet)
+)