opt: Prune Delete operator input columns

pkg/ccl/changefeedccl/helpers_test.go

@@ -553,7 +553,7 @@ func (c *tableFeed) Next(
 		// which seems limiting.
 		var err error
 		c.rows, err = c.db.Query(
-			`DELETE FROM sqlsink ORDER BY PRIMARY KEY sqlsink RETURNING *`)
+			`SELECT * FROM [DELETE FROM sqlsink RETURNING *] ORDER BY topic, partition, message_id`)
 		if err != nil {
 			t.Fatal(err)
 		}

pkg/sql/delete_range.go

@@ -57,6 +57,9 @@ var _ batchedPlanNode = &deleteRangeNode{}
 // canDeleteFast determines if the deletion of `rows` can be done
 // without actually scanning them.
 // This should be called after plan simplification for optimal results.
+//
+// This logic should be kept in sync with exec.Builder.canUseDeleteRange.
+// TODO(andyk): Remove when the heuristic planner code is removed.
 func maybeCreateDeleteFastNode(
 	ctx context.Context,
 	source planNode,

pkg/sql/logictest/testdata/logic_test/delete

@@ -124,16 +124,25 @@ k v
 5 6
 7 8
 
+statement count 4
+DELETE FROM unindexed
+
+# Delete of range with limit.
+statement count 4
+INSERT INTO unindexed VALUES (1, 2), (3, 4), (5, 6), (7, 8)
+
+statement count 1
+DELETE FROM unindexed WHERE k >= 4 ORDER BY k LIMIT 1
+
 query II colnames,rowsort
 SELECT k, v FROM unindexed
 ----
 k v
 1 2
 3 4
-5 6
 7 8
 
-statement count 4
+statement count 3
 DELETE FROM unindexed
 
 query II colnames
@@ -241,3 +250,28 @@ query II
 DELETE FROM t33361 RETURNING *; COMMIT
 ----
 1 3
+
+# Test that delete works with column families (no indexes, so fast path).
+statement ok
+CREATE TABLE family (
+    x INT PRIMARY KEY,
+    y INT,
+    FAMILY (x),
+    FAMILY (y)
+);
+INSERT INTO family VALUES (1, 1), (2, 2), (3, 3)
+
+statement ok
+BEGIN; ALTER TABLE family ADD COLUMN z INT CREATE FAMILY
+
+statement ok
+DELETE FROM family WHERE x=2
+
+statement ok
+COMMIT
+
+query III rowsort
+SELECT x, y, z FROM family
+----
+1  1  NULL
+3  3  NULL

pkg/sql/logictest/testdata/logic_test/optimizer

@@ -123,14 +123,8 @@ SET experimental_optimizer_mutations = false
 statement error pq: no data source matches prefix: t
 UPDATE t SET v=(SELECT v+1 FROM t AS t2 WHERE t2.k=t.k)
 
-statement error pq: no data source matches prefix: t
-DELETE FROM t WHERE EXISTS(SELECT * FROM t AS t2 WHERE t2.k=t.k)
-
 statement ok
 SET experimental_optimizer_mutations = true
 
 statement ok
 UPDATE t SET v=(SELECT v+1 FROM t AS t2 WHERE t2.k=t.k)
-
-statement ok
-DELETE FROM t WHERE EXISTS(SELECT * FROM t AS t2 WHERE t2.k=t.k)

pkg/sql/logictest/testdata/logic_test/update

@@ -460,8 +460,7 @@ UPDATE t29494 SET x = 124 WHERE x = 12 RETURNING *
 ----
 124
 
-# The HP and CBO show different errors for this case.
-statement error [column "y" does not exist | column "y" is being backfilled]
+statement error column "y" does not exist
 UPDATE t29494 SET y = 123
 
 # Check the new column is not usable in assignments. We need to

pkg/sql/opt/bench/stub_factory.go

@@ -214,12 +214,6 @@ func (f *stubFactory) ConstructUpdate(
 	return struct{}{}, nil
 }
 
-func (f *stubFactory) ConstructCreateTable(
-	input exec.Node, schema cat.Schema, ct *tree.CreateTable,
-) (exec.Node, error) {
-	return struct{}{}, nil
-}
-
 func (f *stubFactory) ConstructUpsert(
 	input exec.Node,
 	table cat.Table,
@@ -239,6 +233,18 @@ func (f *stubFactory) ConstructDelete(
 	return struct{}{}, nil
 }
 
+func (f *stubFactory) ConstructDeleteRange(
+	table cat.Table, needed exec.ColumnOrdinalSet, indexConstraint *constraint.Constraint,
+) (exec.Node, error) {
+	return struct{}{}, nil
+}
+
+func (f *stubFactory) ConstructCreateTable(
+	input exec.Node, schema cat.Schema, ct *tree.CreateTable,
+) (exec.Node, error) {
+	return struct{}{}, nil
+}
+
 func (f *stubFactory) ConstructSequenceSelect(seq cat.Sequence) (exec.Node, error) {
 	return struct{}{}, nil
 }

pkg/sql/opt/cat/column.go

@@ -69,22 +69,8 @@ type Column interface {
 	ComputedExprStr() string
 }
 
-// MutationColumn describes a single column that is being added to a table or
-// dropped from a table. Mutation columns require special handling by mutation
-// operators like Insert, Update, and Delete.
-type MutationColumn struct {
-	// Column is the Table.Column that is being added or dropped.
-	Column
-
-	// IsDeleteOnly is true if the column only needs special handling by Delete
-	// operations; Update and Insert operations can ignore the column. See
-	// sqlbase.DescriptorMutation_DELETE_ONLY for more information.
-	IsDeleteOnly bool
-}
-
 // IsMutationColumn is a convenience function that returns true if the column at
 // the given ordinal position is a mutation column.
-func IsMutationColumn(table Table, i int) bool {
-	_, ok := table.Column(i).(*MutationColumn)
-	return ok
+func IsMutationColumn(table Table, ord int) bool {
+	return ord >= table.ColumnCount()
 }

pkg/sql/opt/cat/index.go

@@ -133,3 +133,9 @@ type IndexColumn struct {
 	// this column, rather than least to greatest.
 	Descending bool
 }
+
+// IsMutationIndex is a convenience function that returns true if the index at
+// the given ordinal position is a mutation index.
+func IsMutationIndex(table Table, ord int) bool {
+	return ord >= table.IndexCount()
+}

pkg/sql/opt/cat/table.go

@@ -26,6 +26,21 @@ import (
 
 // Table is an interface to a database table, exposing only the information
 // needed by the query optimizer.
+//
+// Both columns and indexes are grouped into three sets: public, write-only, and
+// delete-only. When a column or index is added or dropped, it proceeds through
+// each of the three states as that schema change is incrementally rolled out to
+// the cluster without blocking ongoing queries. In the public state, reads,
+// writes, and deletes are allowed. In the write-only state, only writes and
+// deletes are allowed. Finally, in the delete-only state, only deletes are
+// allowed. Further details about "online schema change" can be found in:
+//
+//   docs/RFCS/20151014_online_schema_change.md
+//
+// Calling code must take care to use the right collection of columns or
+// indexes. Usually this should be the public collections, since most usages are
+// read-only, but mutation operators generally need to consider non-public
+// columns and indexes.
 type Table interface {
 	DataSource
 
@@ -34,9 +49,30 @@ type Table interface {
 	// information_schema tables.
 	IsVirtualTable() bool
 
-	// ColumnCount returns the number of columns in the table.
+	// IsInterleaved returns true if any of this table's indexes are interleaved
+	// with index(es) from other table(s).
+	IsInterleaved() bool
+
+	// IsReferenced returns true if this table is referenced by at least one
+	// foreign key defined on another table (or this one if self-referential).
+	IsReferenced() bool
+
+	// ColumnCount returns the number of public columns in the table. Public
+	// columns are not currently being added or dropped from the table. This
+	// method should be used when mutation columns can be ignored (the common
+	// case).
 	ColumnCount() int
 
+	// WritableColumnCount returns the number of public and write-only columns in
+	// the table. Although write-only columns are not visible, any inserts and
+	// updates must still set them. WritableColumnCount is always >= ColumnCount.
+	WritableColumnCount() int
+
+	// DeletableColumnCount returns the number of public, write-only, and
+	// delete- only columns in the table. DeletableColumnCount is always >=
+	// WritableColumnCount.
+	DeletableColumnCount() int
+
 	// Column returns a Column interface to the column at the ith ordinal
 	// position within the table, where i < ColumnCount. Note that the Columns
 	// collection includes mutation columns, if present. Mutation columns are in
@@ -46,14 +82,28 @@ type Table interface {
 	//
 	//   cockroachdb/cockroach/docs/RFCS/20151014_online_schema_change.md
 	//
-	// To determine if the column is a mutation column, try to cast it to
-	// *MutationColumn.
+	// Writable columns are always situated after public columns, and are followed
+	// by deletable columns.
 	Column(i int) Column
 
-	// IndexCount returns the number of indexes defined on this table. This
-	// includes the primary index, so the count is always >= 1.
+	// IndexCount returns the number of public indexes defined on this table.
+	// Public indexes are not currently being added or dropped from the table.
+	// This method should be used when mutation columns can be ignored (the common
+	// case). The returned indexes include the primary index, so the count is
+	// always >= 1.
 	IndexCount() int
 
+	// WritableIndexCount returns the number of public and write-only indexes
+	// defined on this table. Although write-only indexes are not visible, any
+	// table mutation operations must still be applied to them. WritableIndexCount
+	// is always >= IndexCount.
+	WritableIndexCount() int
+
+	// DeletableIndexCount returns the number of public, write-only, and
+	// delete-onlyindexes defined on this table. DeletableIndexCount is always
+	// >= WritableIndexCount.
+	DeletableIndexCount() int
+
 	// Index returns the ith index, where i < IndexCount. The table's primary
 	// index is always the 0th index, and is always present (use cat.PrimaryIndex
 	// to select it). The primary index corresponds to the table's primary key.
@@ -130,8 +180,8 @@ type ForeignKeyReference struct {
 	Match tree.CompositeKeyMatchMethod
 }
 
-// FindTableColumnByName returns the ordinal of the column having the given
-// name, if one exists in the given table. Otherwise, it returns -1.
+// FindTableColumnByName returns the ordinal of the non-mutation column having
+// the given name, if one exists in the given table. Otherwise, it returns -1.
 func FindTableColumnByName(tab Table, name tree.Name) int {
 	for ord, n := 0, tab.ColumnCount(); ord < n; ord++ {
 		if tab.Column(ord).ColName() == name {
@@ -147,21 +197,21 @@ func FormatCatalogTable(cat Catalog, tab Table, tp treeprinter.Node) {
 	child := tp.Childf("TABLE %s", tab.Name().TableName)
 
 	var buf bytes.Buffer
-	for i := 0; i < tab.ColumnCount(); i++ {
+	for i := 0; i < tab.DeletableColumnCount(); i++ {
 		buf.Reset()
-		formatColumn(tab.Column(i), &buf)
+		formatColumn(tab.Column(i), IsMutationColumn(tab, i), &buf)
 		child.Child(buf.String())
 	}
 
-	for i := 0; i < tab.IndexCount(); i++ {
-		formatCatalogIndex(tab.Index(i), i == PrimaryIndex, child)
+	for i := 0; i < tab.DeletableIndexCount(); i++ {
+		formatCatalogIndex(tab, i, child)
 	}
 
 	for i := 0; i < tab.IndexCount(); i++ {
 		fkRef, ok := tab.Index(i).ForeignKey()
 
 		if ok {
-			formatCatalogFKRef(cat, tab, tab.Index(i), fkRef, child)
+			formatCatalogFKRef(cat, tab.Index(i), fkRef, child)
 		}
 	}
 
@@ -172,16 +222,21 @@ func FormatCatalogTable(cat Catalog, tab Table, tp treeprinter.Node) {
 
 // formatCatalogIndex nicely formats a catalog index using a treeprinter for
 // debugging and testing.
-func formatCatalogIndex(idx Index, isPrimary bool, tp treeprinter.Node) {
+func formatCatalogIndex(tab Table, ord int, tp treeprinter.Node) {
+	idx := tab.Index(ord)
 	inverted := ""
 	if idx.IsInverted() {
 		inverted = "INVERTED "
 	}
-	child := tp.Childf("%sINDEX %s", inverted, idx.Name())
+	mutation := ""
+	if IsMutationIndex(tab, ord) {
+		mutation = " (mutation)"
+	}
+	child := tp.Childf("%sINDEX %s%s", inverted, idx.Name(), mutation)
 
 	var buf bytes.Buffer
 	colCount := idx.ColumnCount()
-	if isPrimary {
+	if ord == PrimaryIndex {
 		// Omit the "stored" columns from the primary index.
 		colCount = idx.KeyColumnCount()
 	}
@@ -190,7 +245,7 @@ func formatCatalogIndex(idx Index, isPrimary bool, tp treeprinter.Node) {
 		buf.Reset()
 
 		idxCol := idx.Column(i)
-		formatColumn(idxCol.Column, &buf)
+		formatColumn(idxCol.Column, false /* isMutationCol */, &buf)
 		if idxCol.Descending {
 			fmt.Fprintf(&buf, " desc")
 		}
@@ -221,9 +276,7 @@ func formatColPrefix(idx Index, prefixLen int) string {
 
 // formatCatalogFKRef nicely formats a catalog foreign key reference using a
 // treeprinter for debugging and testing.
-func formatCatalogFKRef(
-	cat Catalog, tab Table, idx Index, fkRef ForeignKeyReference, tp treeprinter.Node,
-) {
+func formatCatalogFKRef(cat Catalog, idx Index, fkRef ForeignKeyReference, tp treeprinter.Node) {
 	ds, err := cat.ResolveDataSourceByID(context.TODO(), fkRef.TableID)
 	if err != nil {
 		panic(err)
@@ -247,12 +300,15 @@ func formatCatalogFKRef(
 	)
 }
 
-func formatColumn(col Column, buf *bytes.Buffer) {
+func formatColumn(col Column, isMutationCol bool, buf *bytes.Buffer) {
 	fmt.Fprintf(buf, "%s %s", col.ColName(), col.DatumType())
 	if !col.IsNullable() {
 		fmt.Fprintf(buf, " not null")
 	}
 	if col.IsHidden() {
 		fmt.Fprintf(buf, " (hidden)")
 	}
+	if isMutationCol {
+		fmt.Fprintf(buf, " (mutation)")
+	}
 }