sql: add NotVisible to index descriptor

This PR adds a new field `NotVisible` to the struct `IndexDescriptor`. Since primary indexes cannot be not visible, it also includes another check in `pkg/sql/catalog/tabledesc/validate.go`. Since the invisible index feature has not been introduced yet, all indexes created should be visible. See also: cockroachdb#84776 Assists: cockroachdb#72576 Release note: none
wenyihu6 · Jul 21, 2022 · 9c18b23 · 9c18b23
1 parent 4a8f1be
commit 9c18b23
Show file tree

Hide file tree

Showing 25 changed files with 129 additions and 16 deletions.
diff --git a/pkg/ccl/schemachangerccl/testdata/decomp/multiregion b/pkg/ccl/schemachangerccl/testdata/decomp/multiregion
@@ -121,6 +121,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -368,6 +369,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -627,6 +629,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0

diff --git a/pkg/ccl/schemachangerccl/testdata/decomp/partitioning b/pkg/ccl/schemachangerccl/testdata/decomp/partitioning
@@ -94,6 +94,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -106,6 +107,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: true
+    isNotVisible: false
     isUnique: false
     sharding: null
     sourceIndexId: 0
@@ -459,6 +461,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -471,6 +474,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0

diff --git a/pkg/cli/testdata/doctor/test_recreate_zipdir b/pkg/cli/testdata/doctor/test_recreate_zipdir
diff --git a/pkg/sql/catalog/catformat/index.go b/pkg/sql/catalog/catformat/index.go
@@ -175,6 +175,11 @@ func indexForDisplay(
 		}
 	}
 
+	// TODO(wenyihu6): add test cases for SHOW CREATE TABLE once not visible indexes can be added.
+	if index.NotVisible {
+		f.WriteString(" NOT VISIBLE")
+	}
+
 	return f.CloseAndGetString(), nil
 }
 

diff --git a/pkg/sql/catalog/descpb/structured.proto b/pkg/sql/catalog/descpb/structured.proto
@@ -491,7 +491,13 @@ message IndexDescriptor {
   optional uint32 constraint_id = 26 [(gogoproto.customname) = "ConstraintID",
     (gogoproto.casttype) = "ConstraintID", (gogoproto.nullable) = false];
 
-  // Next ID: 28
+  // NotVisible specifies whether the index is not visible to the optimizer.
+  // A not visible index is ignored by the optimizer unless it is used for
+  // constraint check or is explicitly selected with index hinting (force
+  // index). By default, an index should be visible.
+  optional bool not_visible = 28 [(gogoproto.nullable) = false];
+
+  // Next ID: 29
 }
 
 // ConstraintToUpdate represents a constraint to be added to the table and

diff --git a/pkg/sql/catalog/table_elements.go b/pkg/sql/catalog/table_elements.go
@@ -139,6 +139,7 @@ type Index interface {
 	IsUnique() bool
 	IsDisabled() bool
 	IsSharded() bool
+	IsNotVisible() bool
 	IsCreatedExplicitly() bool
 	GetPredicate() string
 	GetType() descpb.IndexDescriptor_Type

diff --git a/pkg/sql/catalog/tabledesc/index.go b/pkg/sql/catalog/tabledesc/index.go
@@ -102,6 +102,11 @@ func (w index) IsSharded() bool {
 	return w.desc.IsSharded()
 }
 
+// IsNotVisible returns true iff the index is not visible.
+func (w index) IsNotVisible() bool {
+	return w.desc.NotVisible
+}
+
 // IsCreatedExplicitly returns true iff this index was created explicitly, i.e.
 // via 'CREATE INDEX' statement.
 func (w index) IsCreatedExplicitly() bool {

diff --git a/pkg/sql/catalog/tabledesc/validate.go b/pkg/sql/catalog/tabledesc/validate.go
@@ -1291,6 +1291,9 @@ func (desc *wrapper) validateTableIndexes(
 				return errors.AssertionFailedf("primary index %q has invalid encoding type %d in proto, expected %d",
 					idx.GetName(), idx.IndexDesc().EncodingType, descpb.PrimaryIndexEncoding)
 			}
+			if idx.IsNotVisible() {
+				return errors.Newf("primary index %q cannot be not visible", idx.GetName())
+			}
 		}
 		// Ensure that index column ID subsets are well formed.
 		if idx.GetVersion() < descpb.StrictIndexColumnIDGuaranteesVersion {

diff --git a/pkg/sql/catalog/tabledesc/validate_test.go b/pkg/sql/catalog/tabledesc/validate_test.go
@@ -139,6 +139,7 @@ var validationMap = []struct {
 			"Name":                {status: thisFieldReferencesNoObjects},
 			"ID":                  {status: thisFieldReferencesNoObjects},
 			"Unique":              {status: thisFieldReferencesNoObjects},
+			"NotVisible":          {status: thisFieldReferencesNoObjects},
 			"Version":             {status: thisFieldReferencesNoObjects},
 			"KeyColumnNames":      {status: iSolemnlySwearThisFieldIsValidated},
 			"KeyColumnDirections": {status: iSolemnlySwearThisFieldIsValidated},
@@ -651,6 +652,36 @@ func TestValidateTableDesc(t *testing.T) {
 				NextColumnID: 2,
 				NextFamilyID: 1,
 			}},
+		{`primary index "p_idx" cannot be not visible`,
+			descpb.TableDescriptor{
+				ID:            2,
+				ParentID:      1,
+				Name:          "foo",
+				FormatVersion: descpb.InterleavedFormatVersion,
+				Columns: []descpb.ColumnDescriptor{
+					{ID: 1, Name: "bar", Type: types.Int},
+				},
+				PrimaryIndex: descpb.IndexDescriptor{
+					ID:                  1,
+					Name:                "p_idx",
+					KeyColumnIDs:        []descpb.ColumnID{1},
+					KeyColumnNames:      []string{"bar"},
+					KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC},
+					Version:             descpb.PrimaryIndexWithStoredColumnsVersion,
+					EncodingType:        descpb.PrimaryIndexEncoding,
+					ConstraintID:        1,
+					NotVisible:          true,
+				},
+				Families: []descpb.ColumnFamilyDescriptor{
+					{ID: 0, Name: "primary",
+						ColumnIDs:   []descpb.ColumnID{1},
+						ColumnNames: []string{"bar"},
+					},
+				},
+				NextColumnID: 2,
+				NextFamilyID: 1,
+				NextIndexID:  2,
+			}},
 		{`invalid index ID 0`,
 			descpb.TableDescriptor{
 				ID:            2,

diff --git a/pkg/sql/opt/cat/index.go b/pkg/sql/opt/cat/index.go
@@ -56,6 +56,9 @@ type Index interface {
 	// IsInverted returns true if this is an inverted index.
 	IsInverted() bool
 
+	// IsNotVisible returns true if this index is not visible.
+	IsNotVisible() bool
+
 	// ColumnCount returns the number of columns in the index. This includes
 	// columns that were part of the index definition (including the STORING
 	// clause), as well as implicitly added primary key columns. It also contains

diff --git a/pkg/sql/opt/cat/utils.go b/pkg/sql/opt/cat/utils.go
@@ -129,7 +129,13 @@ func formatCatalogIndex(tab Table, ord int, tp treeprinter.Node) {
 	if IsMutationIndex(tab, ord) {
 		mutation = " (mutation)"
 	}
-	child := tp.Childf("%sINDEX %s%s", idxType, idx.Name(), mutation)
+
+	isNotVisible := ""
+	if idx.IsNotVisible() {
+		isNotVisible = " NOT VISIBLE"
+	}
+
+	child := tp.Childf("%sINDEX %s%s%s", idxType, idx.Name(), mutation, isNotVisible)
 
 	var buf bytes.Buffer
 	colCount := idx.ColumnCount()

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/show_trace b/pkg/sql/opt/exec/execbuilder/testdata/show_trace
diff --git a/pkg/sql/opt/exec/explain/plan_gist_factory.go b/pkg/sql/opt/exec/explain/plan_gist_factory.go
@@ -587,6 +587,10 @@ func (u *unknownIndex) IsInverted() bool {
 	return false
 }
 
+func (u *unknownIndex) IsNotVisible() bool {
+	return false
+}
+
 func (u *unknownIndex) ColumnCount() int {
 	return 0
 }

diff --git a/pkg/sql/opt/indexrec/hypothetical_index.go b/pkg/sql/opt/indexrec/hypothetical_index.go
@@ -111,6 +111,13 @@ func (hi *hypotheticalIndex) IsInverted() bool {
 	return hi.inverted
 }
 
+// IsNotVisible is part of the cat.Index interface.
+func (hi *hypotheticalIndex) IsNotVisible() bool {
+	// A hypotheticalIndex should not be invisible because there is no motivation
+	// to recommend an invisible index.
+	return false
+}
+
 // ColumnCount is part of the cat.Index interface.
 func (hi *hypotheticalIndex) ColumnCount() int {
 	return len(hi.cols) + len(hi.suffixKeyColsOrdList) + hi.storedColsOrdSet.Len()

diff --git a/pkg/sql/opt/indexrec/hypothetical_table.go b/pkg/sql/opt/indexrec/hypothetical_table.go
@@ -60,6 +60,8 @@ func BuildOptAndHypTableMaps(
 			// index with the same key. Inverted indexes do not have stored columns,
 			// so we should not make a recommendation if the same index already
 			// exists.
+			// TODO(wenyihu6): We should still consider invisible indexes and make a
+			// recommendation to mark the index as visible if it is chosen.
 			if !inverted || hypTable.existingRedundantIndex(&hypIndex) == nil {
 				hypIndexes = append(hypIndexes, hypIndex)
 			}

diff --git a/pkg/sql/opt/testutils/testcat/test_catalog.go b/pkg/sql/opt/testutils/testcat/test_catalog.go
@@ -935,6 +935,9 @@ type Index struct {
 	// Inverted is true when this index is an inverted index.
 	Inverted bool
 
+	// NotVisible is true when this index is a not visible index.
+	NotVisible bool
+
 	Columns []cat.IndexColumn
 
 	// IdxZone is the zone associated with the index. This may be inherited from
@@ -996,6 +999,11 @@ func (ti *Index) IsInverted() bool {
 	return ti.Inverted
 }
 
+// IsNotVisible is part of the cat.Index interface.
+func (ti *Index) IsNotVisible() bool {
+	return ti.NotVisible
+}
+
 // ColumnCount is part of the cat.Index interface.
 func (ti *Index) ColumnCount() int {
 	return len(ti.Columns)

diff --git a/pkg/sql/opt_catalog.go b/pkg/sql/opt_catalog.go
@@ -1457,6 +1457,11 @@ func (oi *optIndex) IsInverted() bool {
 	return oi.idx.GetType() == descpb.IndexDescriptor_INVERTED
 }
 
+// IsNotVisible is part of the cat.Index interface.
+func (oi *optIndex) IsNotVisible() bool {
+	return oi.idx.IsNotVisible()
+}
+
 // ColumnCount is part of the cat.Index interface.
 func (oi *optIndex) ColumnCount() int {
 	return oi.numCols
@@ -2231,6 +2236,11 @@ func (oi *optVirtualIndex) IsInverted() bool {
 	return false
 }
 
+// IsNotVisible is part of the cat.Index interface.
+func (oi *optVirtualIndex) IsNotVisible() bool {
+	return false
+}
+
 // ExplicitColumnCount is part of the cat.Index interface.
 func (oi *optVirtualIndex) ExplicitColumnCount() int {
 	return oi.idx.NumKeyColumns()

diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go
@@ -640,6 +640,7 @@ func addSecondaryIndexTargetsForAddColumn(
 		IsUnique:      desc.Unique,
 		IsInverted:    desc.Type == descpb.IndexDescriptor_INVERTED,
 		SourceIndexID: newPrimaryIdx.IndexID,
+		IsNotVisible:  desc.NotVisible,
 	}
 	tempIndexID := index.IndexID + 1 // this is enforced below
 	index.TemporaryIndexID = tempIndexID

diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go
@@ -43,6 +43,7 @@ func CreateIndex(b BuildCtx, n *tree.CreateIndex) {
 		IsUnique:       n.Unique,
 		IsInverted:     n.Inverted,
 		IsConcurrently: n.Concurrently,
+		IsNotVisible:   false, // TODO(wenyihu6): populate not visible property after CREATE
 	}
 	var relation scpb.Element
 	var source *scpb.PrimaryIndex

diff --git a/pkg/sql/schemachanger/scdecomp/decomp.go b/pkg/sql/schemachanger/scdecomp/decomp.go
@@ -456,6 +456,7 @@ func (w *walkCtx) walkIndex(tbl catalog.TableDescriptor, idx catalog.Index) {
 			IsInverted:          idx.GetType() == descpb.IndexDescriptor_INVERTED,
 			IsCreatedExplicitly: idx.IsCreatedExplicitly(),
 			ConstraintID:        idx.GetConstraintID(),
+			IsNotVisible:        idx.IsNotVisible(),
 		}
 		for i, c := range cpy.KeyColumnIDs {
 			w.ev(scpb.Status_PUBLIC, &scpb.IndexColumn{

diff --git a/pkg/sql/schemachanger/scdecomp/testdata/sequence b/pkg/sql/schemachanger/scdecomp/testdata/sequence
@@ -143,6 +143,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -422,6 +423,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0

diff --git a/pkg/sql/schemachanger/scdecomp/testdata/table b/pkg/sql/schemachanger/scdecomp/testdata/table
@@ -64,6 +64,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -271,6 +272,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -283,6 +285,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: false
     sharding: null
     sourceIndexId: 0
@@ -641,6 +644,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -882,6 +886,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0

diff --git a/pkg/sql/schemachanger/scdecomp/testdata/type b/pkg/sql/schemachanger/scdecomp/testdata/type
@@ -153,6 +153,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: true
     sharding: null
     sourceIndexId: 0
@@ -165,6 +166,7 @@ ElementState:
     isConcurrently: false
     isCreatedExplicitly: false
     isInverted: false
+    isNotVisible: false
     isUnique: false
     sharding: null
     sourceIndexId: 0

diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/index.go b/pkg/sql/schemachanger/scexec/scmutationexec/index.go
@@ -79,6 +79,7 @@ func addNewIndexMutation(
 		ID:                          opIndex.IndexID,
 		Name:                        tabledesc.IndexNamePlaceholder(opIndex.IndexID),
 		Unique:                      opIndex.IsUnique,
+		NotVisible:                  opIndex.IsNotVisible,
 		Version:                     indexVersion,
 		Type:                        indexType,
 		CreatedExplicitly:           true,

diff --git a/pkg/sql/schemachanger/scpb/elements.proto b/pkg/sql/schemachanger/scpb/elements.proto
@@ -214,6 +214,8 @@ message Index {
   uint32 source_index_id = 21 [(gogoproto.customname) = "SourceIndexID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.IndexID"];
   uint32 temporary_index_id = 22 [(gogoproto.customname) = "TemporaryIndexID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.IndexID"];
 
+  // IsNotVisible specifies whether this index is not visible.
+  bool is_not_visible = 23;
   reserved 3, 4, 5, 6, 7;
 }