opt: Prune Delete operator input columns

[andy-kimball]

Prune input columns that are not needed by a Delete operator. Needed
columns include returned columns and index key columns. All other columns
can be pruned.

Pruning Delete columns causes new interactions with the Delete "fast path"
that uses range delete. When multiple column families are in use, the
range delete needs to cover all column families. The "forDelete" flag
is used to construct spans that cover all columns, rather than just
"needed" columns. A new ConstructDeleteRange exec factory function sets
this flag correctly.

After this change, optimizer deletes no longer need to stay behind a
feature flag, as all fast paths should now work at least as well as they
do with the heuristic planner.

Release note: None

[cockroach-teamcity]

This change is 

[andy-kimball]

@jordanlewis, I've added a ConstructDeleteRange method that I think should hook into your new operator easily.

[jordanlewis]

Nice. Should this come sequenced after the execution PR?

[andy-kimball]

I assume so, if you get yours in soon.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @justinj, @RaduBerinde, and @rytaft)

[jordanlewis]

Mine is merged now.

[RaduBerinde]

(modulo using the new delete range operator)

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andy-kimball, @justinj, @RaduBerinde, and @rytaft)

---

pkg/sql/opt/metadata.go, line 220 at r3 (raw file):

// is passed separately so that its original formatting is preserved for error
// messages, pretty-printing, etc.
func (md *Metadata) AddTableWithAlias(tab cat.Table, alias tree.TableName) TableID {

[nit] most functions pass *TableName. We'd just make a copy so it shouldn't escape

---

pkg/sql/opt/cat/table.go, line 29 at r1 (raw file):

// Table is an interface to a database table, exposing only the information
// needed by the query optimizer.
type Table interface {

[nit] This comment could be extended with some general info on public, writable, deletable columns.

---

pkg/sql/opt/cat/table.go, line 43 at r1 (raw file):

	ColumnCount() int

	// WritableColumnCount returns the number of public and writable columns in

In these descriptions, "writable" and "deletable" each have two meanings (they either include the other columns or not). I would switch to using "write-only" and "delete-only" when referring to just those columns (consistent with the ImmutableTableDescriptor terminology). E.g. WritableColumnCount returns the number of public and write-only columns.

---

pkg/sql/opt/cat/table.go, line 73 at r2 (raw file):

	IndexCount() int

	// WritableIndexCount returns the number of public and writable indexes

Same suggestion, "write-only indexes".

[rytaft]

Reviewed 25 of 25 files at r1, 5 of 5 files at r2, 15 of 15 files at r3, 20 of 20 files at r4.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @andy-kimball and @justinj)

---

pkg/sql/logictest/testdata/logic_test/delete, line 267 at r4 (raw file):

BEGIN; ALTER TABLE family ADD COLUMN z INT CREATE FAMILY

# Check that the new column is not usable in RETURNING

I think this comment is stale

---

pkg/sql/opt/cat/column.go, line 75 at r1 (raw file):

// the given ordinal position is a mutation column.
func IsMutationColumn(table Table, ord int) bool {
	return ord >= table.ColumnCount()

Do you also want to check that it's less than table.DeletableColumnCount()?

---

pkg/sql/opt/norm/testdata/rules/prune_cols, line 1653 at r4 (raw file):

# Prune when computed ordering column is present.
opt

do you want to add an expectation here?

---

pkg/sql/opt/norm/testdata/rules/prune_cols, line 1706 at r4 (raw file):

# Prune when mutation columns/indexes exist.
opt

and here?

---

pkg/sql/opt/optbuilder/insert.go, line 408 at r1 (raw file):

// addTargetTableColsForInsert adds up to maxCols columns to the list of columns
// that will be set by an INSERT operation. Non-mutation olumns are added from

Non-mutation olumns -> Non-mutation columns

---

pkg/sql/opt/optbuilder/insert.go, line 423 at r1 (raw file):

	// Only consider non-mutation columns, since mutation columns are hidden from
	// the SQL user.

Is this a change from before?

[andy-kimball]

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @justinj and @rytaft)

---

pkg/sql/logictest/testdata/logic_test/delete, line 267 at r4 (raw file):

Previously, rytaft wrote…

I think this comment is stale

Done.

---

pkg/sql/opt/cat/column.go, line 75 at r1 (raw file):

Previously, rytaft wrote…

Do you also want to check that it's less than table.DeletableColumnCount()?

That'd be more of an assert, since only mutation columns are >= ColumnCount. I want this to be inlined, and don't want to add an assert panic in this code path, since it's used in contexts where we should be returning errors rather than panicking. I'm also not too worried about that case. So I'll leave as-is.

---

pkg/sql/opt/norm/testdata/rules/prune_cols, line 1653 at r4 (raw file):

Previously, rytaft wrote…

do you want to add an expectation here?

Done.

---

pkg/sql/opt/norm/testdata/rules/prune_cols, line 1706 at r4 (raw file):

Previously, rytaft wrote…

and here?

Done.

---

pkg/sql/opt/optbuilder/insert.go, line 408 at r1 (raw file):

Previously, rytaft wrote…

Non-mutation olumns -> Non-mutation columns

Done.

---

pkg/sql/opt/optbuilder/insert.go, line 423 at r1 (raw file):

Previously, rytaft wrote…

Is this a change from before?

No, just additional comment for clarification.

[rytaft]

Reviewed 2 of 47 files at r5, 13 of 25 files at r6, 1 of 5 files at r7, 9 of 15 files at r8, 23 of 23 files at r9.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @justinj)

[danhhz]

changefeed changes lgtm

[andy-kimball]

bors r+

[craig]

Build failed

• GitHub CI (Cockroach)

[andy-kimball]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)