Merge #65375 #65524

65375: sql: add support for expression-based indexes with CREATE INDEX r=mgartner a=mgartner

#### sql: add experimental_enable_expression_based_indexes session setting

This commit adds a session setting that will eventually enable users to
create expression-based indexes. The setting will be removed when
expression-based indexes are fully supported.

Release note: None

#### sql: add support for expression-based indexes with CREATE INDEX

This commit adds basic support for creating expression-based indexes
with a `CREATE INDEX` statement. An expression-based index is syntactic
sugar for an index on a virtual computed column. Creating an
expression-based index will automatically created a hidden virtual
column with the given expression. If a virtual column with the given
expression already exists, that column is used rather than creating a
new one.

Future work includes supporting expression-based indexes in
`CREATE TABLE` and making error messages related to these indexes more
user-friendly.

There is no release note because expression-based indexes are not
enabled by default. They require the
`experimental_enable_expression_based_indexes` session setting until
they are fully supported.

Release note: None


65524: sql: clear right rows correctly during apply-join r=mgartner a=mgartner

This commit fixes a bug introduced in #63900 which causes execution of
semi and anti apply-joins to panic. For a each row on the left side of
the apply-join, rows are fetched for the right side of the join and
added to an iterator. For semi and anti apply-joins, the right rows are
only consumed until a match is found. These right rows were not being
cleared for the next successive left row. This caused a panic when the
apply-join predicate would be applied on a `nil` left row during the
next call to `applyJoinNode.Next`; the next left row is only fetched if
the right row iterator has been cleared.

Fixes #65040

There is no release note because this bug is only present in
21.2.0-alpha, which has not yet been released.

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>

docs/generated/settings/settings-for-tenants.txt

@@ -109,4 +109,4 @@ trace.datadog.project	string	CockroachDB	the project under which traces will be
 trace.debug.enable	boolean	false	if set, traces for recent requests can be seen at https://<ui>/debug/requests
 trace.lightstep.token	string		if set, traces go to Lightstep using this token
 trace.zipkin.collector	string		if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'). Only one tracer can be configured at a time.
-version	version	21.1-8	set the active cluster version in the format '<major>.<minor>'
+version	version	21.1-10	set the active cluster version in the format '<major>.<minor>'

docs/generated/settings/settings.html

@@ -111,6 +111,6 @@
 <tr><td><code>trace.debug.enable</code></td><td>boolean</td><td><code>false</code></td><td>if set, traces for recent requests can be seen at https://<ui>/debug/requests</td></tr>
 <tr><td><code>trace.lightstep.token</code></td><td>string</td><td><code></code></td><td>if set, traces go to Lightstep using this token</td></tr>
 <tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'). Only one tracer can be configured at a time.</td></tr>
-<tr><td><code>version</code></td><td>version</td><td><code>21.1-8</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
+<tr><td><code>version</code></td><td>version</td><td><code>21.1-10</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
 </tbody>
 </table>

pkg/clusterversion/cockroach_versions.go

@@ -334,6 +334,8 @@ const (
 	// SerializeViewUDTs serializes user defined types used in views to allow
 	// for renaming of the referenced types.
 	SerializeViewUDTs
+	// ExpressionBasedIndexes is when expression-based indexes are supported.
+	ExpressionBasedIndexes
 
 	// Step (1): Add new versions here.
 )
@@ -559,6 +561,10 @@ var versionsSingleton = keyedVersions([]keyedVersion{
 		Key:     SerializeViewUDTs,
 		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 8},
 	},
+	{
+		Key:     ExpressionBasedIndexes,
+		Version: roachpb.Version{Major: 21, Minor: 1, Internal: 10},
+	},
 
 	// Step (2): Add new versions here.
 })

pkg/sql/add_column.go

@@ -18,7 +18,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
-	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
 	"github.com/cockroachdb/errors"
 	"github.com/lib/pq/oid"
@@ -31,7 +30,6 @@ func (p *planner) addColumnImpl(
 	tn *tree.TableName,
 	desc *tabledesc.Mutable,
 	t *tree.AlterTableAddColumn,
-	sessionData *sessiondata.SessionData,
 ) error {
 	d := t.ColumnDef
 	version := params.ExecCfg().Settings.Version.ActiveVersionOrEmpty(params.ctx)

pkg/sql/alter_column_type.go

@@ -259,7 +259,7 @@ func alterColumnTypeGeneral(
 		return err == nil
 	}
 
-	shadowColName := tabledesc.GenerateUniqueConstraintName(col.GetName(), nameExists)
+	shadowColName := tabledesc.GenerateUniqueName(col.GetName(), nameExists)
 
 	var newColComputeExpr *string
 	// oldCol still needs to have values written to it in case nodes read it from
@@ -273,7 +273,7 @@ func alterColumnTypeGeneral(
 	var inverseExpr string
 	if using != nil {
 		// Validate the provided using expr and ensure it has the correct type.
-		expr, _, err := schemaexpr.DequalifyAndValidateExpr(
+		expr, _, _, err := schemaexpr.DequalifyAndValidateExpr(
 			ctx,
 			tableDesc,
 			using,

pkg/sql/alter_primary_key.go

@@ -186,7 +186,7 @@ func (p *planner) AlterPrimaryKey(
 	}
 
 	// Make a new index that is suitable to be a primary index.
-	name := tabledesc.GenerateUniqueConstraintName(
+	name := tabledesc.GenerateUniqueName(
 		"new_primary_key",
 		nameExists,
 	)
@@ -484,7 +484,7 @@ func (p *planner) AlterPrimaryKey(
 			tabledesc.UpdateIndexPartitioning(&newIndex, newImplicitCols, newPartitioning)
 		}
 
-		newIndex.Name = tabledesc.GenerateUniqueConstraintName(basename, nameExists)
+		newIndex.Name = tabledesc.GenerateUniqueName(basename, nameExists)
 		if err := addIndexMutationWithSpecificPrimaryKey(ctx, tableDesc, &newIndex, newPrimaryIndexDesc); err != nil {
 			return err
 		}

pkg/sql/alter_table.go

@@ -175,7 +175,7 @@ func (n *alterTableNode) startExec(params runParams) error {
 			}
 			var err error
 			params.p.runWithOptions(resolveFlags{contextDatabaseID: n.tableDesc.ParentID}, func() {
-				err = params.p.addColumnImpl(params, n, tn, n.tableDesc, t, params.SessionData())
+				err = params.p.addColumnImpl(params, n, tn, n.tableDesc, t)
 			})
 			if err != nil {
 				return err

pkg/sql/alter_table_locality.go

@@ -335,7 +335,6 @@ func (n *alterTableSetLocalityNode) alterTableLocalityToRegionalByRow(
 			tn,
 			n.tableDesc,
 			defaultColDef,
-			params.SessionData(),
 		); err != nil {
 			return err
 		}

pkg/sql/apply_join.go

@@ -151,6 +151,14 @@ func (a *applyJoinNode) Next(params runParams) (bool, error) {
 				a.pred.prepareRow(a.run.out, a.run.leftRow, rrow)
 				return true, nil
 			}
+
+			// We're either out of right side rows or we broke out of the loop
+			// before consuming all right rows because we found a match for an
+			// anti or semi join. Clear the right rows to prepare them for the
+			// next left row.
+			if err := a.clearRightRows(params); err != nil {
+				return false, err
+			}
 		}
 		// We're out of right side rows. Reset the match state for next time.
 		foundAMatch := a.run.leftRowFoundAMatch
@@ -217,20 +225,23 @@ func (a *applyJoinNode) Next(params runParams) (bool, error) {
 	}
 }
 
+// clearRightRows clears rightRows and resets rightRowsIterator. This function
+// must be called before reusing rightRows and rightRowIterator.
+func (a *applyJoinNode) clearRightRows(params runParams) error {
+	if err := a.run.rightRows.clear(params.ctx); err != nil {
+		return err
+	}
+	a.run.rightRowsIterator.close()
+	a.run.rightRowsIterator = nil
+	return nil
+}
+
 // runRightSidePlan runs a planTop that's been generated based on the
 // re-optimized right hand side of the apply join, stashing the result in
 // a.run.rightRows, ready for retrieval. An error indicates that something went
 // wrong during execution of the right hand side of the join, and that we should
 // completely give up on the outer join.
 func (a *applyJoinNode) runRightSidePlan(params runParams, plan *planComponents) error {
-	// Prepare rightRows state for reuse.
-	if err := a.run.rightRows.clear(params.ctx); err != nil {
-		return err
-	}
-	if a.run.rightRowsIterator != nil {
-		a.run.rightRowsIterator.close()
-		a.run.rightRowsIterator = nil
-	}
 	if err := runPlanInsidePlan(params, plan, &a.run.rightRows); err != nil {
 		return err
 	}

pkg/sql/catalog/schemaexpr/check_constraint.go

@@ -89,7 +89,7 @@ func (b *CheckConstraintBuilder) Build(
 
 	// Verify that the expression results in a boolean and does not use
 	// invalid functions.
-	expr, colIDs, err := DequalifyAndValidateExpr(
+	expr, _, colIDs, err := DequalifyAndValidateExpr(
 		b.ctx,
 		b.desc,
 		c.Expr,

pkg/sql/catalog/schemaexpr/computed_column.go

@@ -101,7 +101,7 @@ func ValidateComputedColumnExpression(
 	// are no variable expressions (besides dummyColumnItems) and no impure
 	// functions. In order to safely serialize user defined types and their
 	// members, we need to serialize the typed expression here.
-	expr, _, err := DequalifyAndValidateExpr(
+	expr, _, _, err := DequalifyAndValidateExpr(
 		ctx,
 		desc,
 		d.Computed.Expr,

pkg/sql/catalog/schemaexpr/expr.go

@@ -25,10 +25,10 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-// DequalifyAndValidateExpr validates that an expression has the given type
-// and contains no functions with a volatility greater than maxVolatility. The
-// type-checked and constant-folded expression and the set of column IDs within
-// the expression are returned, if valid.
+// DequalifyAndValidateExpr validates that an expression has the given type and
+// contains no functions with a volatility greater than maxVolatility. The
+// type-checked and constant-folded expression, the type of the expression, and
+// the set of column IDs within the expression are returned, if valid.
 //
 // The serialized expression is returned because returning the created
 // tree.TypedExpr would be dangerous. It contains dummyColumns which do not
@@ -43,22 +43,22 @@ func DequalifyAndValidateExpr(
 	semaCtx *tree.SemaContext,
 	maxVolatility tree.Volatility,
 	tn *tree.TableName,
-) (string, catalog.TableColSet, error) {
+) (string, *types.T, catalog.TableColSet, error) {
 	var colIDs catalog.TableColSet
 	nonDropColumns := desc.NonDropColumns()
 	sourceInfo := colinfo.NewSourceInfoForSingleTable(
 		*tn, colinfo.ResultColumnsFromColumns(desc.GetID(), nonDropColumns),
 	)
 	expr, err := dequalifyColumnRefs(ctx, sourceInfo, expr)
 	if err != nil {
-		return "", colIDs, err
+		return "", nil, colIDs, err
 	}
 
 	// Replace the column variables with dummyColumns so that they can be
 	// type-checked.
 	replacedExpr, colIDs, err := replaceColumnVars(desc, expr)
 	if err != nil {
-		return "", colIDs, err
+		return "", nil, colIDs, err
 	}
 
 	typedExpr, err := SanitizeVarFreeExpr(
@@ -71,10 +71,10 @@ func DequalifyAndValidateExpr(
 	)
 
 	if err != nil {
-		return "", colIDs, err
+		return "", nil, colIDs, err
 	}
 
-	return tree.Serialize(typedExpr), colIDs, nil
+	return tree.Serialize(typedExpr), typedExpr.ResolvedType(), colIDs, nil
 }
 
 // ExtractColumnIDs returns the set of column IDs within the given expression.

pkg/sql/catalog/schemaexpr/expr_test.go

@@ -77,7 +77,7 @@ func TestValidateExpr(t *testing.T) {
 				t.Fatalf("%s: unexpected error: %s", d.expr, err)
 			}
 
-			deqExpr, _, err := schemaexpr.DequalifyAndValidateExpr(
+			deqExpr, _, _, err := schemaexpr.DequalifyAndValidateExpr(
 				ctx,
 				desc,
 				expr,

pkg/sql/catalog/schemaexpr/partial_index.go

@@ -40,7 +40,7 @@ func ValidatePartialIndexPredicate(
 	tn *tree.TableName,
 	semaCtx *tree.SemaContext,
 ) (string, error) {
-	expr, _, err := DequalifyAndValidateExpr(
+	expr, _, _, err := DequalifyAndValidateExpr(
 		ctx,
 		desc,
 		e,

pkg/sql/catalog/schemaexpr/unique_contraint.go

@@ -37,7 +37,7 @@ func ValidateUniqueWithoutIndexPredicate(
 	pred tree.Expr,
 	semaCtx *tree.SemaContext,
 ) (string, error) {
-	expr, _, err := DequalifyAndValidateExpr(
+	expr, _, _, err := DequalifyAndValidateExpr(
 		ctx,
 		desc,
 		pred,

pkg/sql/catalog/tabledesc/structured.go

@@ -608,7 +608,7 @@ func (desc *Mutable) ensurePrimaryKey() error {
 		}
 		s := "unique_rowid()"
 		col := &descpb.ColumnDescriptor{
-			Name:        GenerateUniqueConstraintName("rowid", nameExists),
+			Name:        GenerateUniqueName("rowid", nameExists),
 			Type:        types.Int,
 			DefaultExpr: &s,
 			Hidden:      true,
@@ -1719,7 +1719,7 @@ func (desc *Mutable) performComputedColumnSwap(swap *descpb.ComputedColumnSwap)
 		return err == nil
 	}
 
-	uniqueName := GenerateUniqueConstraintName(newCol.GetName(), nameExists)
+	uniqueName := GenerateUniqueName(newCol.GetName(), nameExists)
 
 	// Remember the name of oldCol, because newCol will take it.
 	oldColName := oldCol.GetName()
@@ -2264,11 +2264,10 @@ func (desc *wrapper) TableDesc() *descpb.TableDescriptor {
 	return &desc.TableDescriptor
 }
 
-// GenerateUniqueConstraintName attempts to generate a unique constraint name
-// with the given prefix.
-// It will first try prefix by itself, then it will subsequently try
-// adding numeric digits at the end, starting from 1.
-func GenerateUniqueConstraintName(prefix string, nameExistsFunc func(name string) bool) string {
+// GenerateUniqueName attempts to generate a unique name with the given prefix.
+// It will first try prefix by itself, then it will subsequently try adding
+// numeric digits at the end, starting from 1.
+func GenerateUniqueName(prefix string, nameExistsFunc func(name string) bool) string {
 	name := prefix
 	for i := 1; nameExistsFunc(name); i++ {
 		name = fmt.Sprintf("%s_%d", prefix, i)

pkg/sql/catalog/tabledesc/table_desc.go

@@ -402,7 +402,7 @@ func (desc *wrapper) FindColumnWithID(id descpb.ColumnID) (catalog.Column, error
 }
 
 // FindColumnWithName returns the first column found whose name matches the
-// provided target ID, in the canonical order.
+// provided target name, in the canonical order.
 // If no column is found then an error is also returned.
 func (desc *wrapper) FindColumnWithName(name tree.Name) (catalog.Column, error) {
 	for _, col := range desc.AllColumns() {
@@ -413,6 +413,18 @@ func (desc *wrapper) FindColumnWithName(name tree.Name) (catalog.Column, error)
 	return nil, colinfo.NewUndefinedColumnError(string(name))
 }
 
+// FindVirtualColumnWithExpr returns the first virtual computed column whose
+// expression matches the provided target expression, in the canonical order. If
+// no column is found then ok=false is returned.
+func (desc *wrapper) FindVirtualColumnWithExpr(expr string) (_ catalog.Column, ok bool) {
+	for _, col := range desc.AllColumns() {
+		if col.IsVirtual() && col.GetComputeExpr() == expr {
+			return col, true
+		}
+	}
+	return nil, false
+}
+
 // getExistingOrNewMutationCache should be the only place where the
 // mutationCache field in wrapper is ever read.
 func (desc *wrapper) getExistingOrNewMutationCache() *mutationCache {