Skip to content

Commit

Permalink
sql: added NotVisible to index descriptor
Browse files Browse the repository at this point in the history
This PR added a new field `NotVisible` to the struct `IndexDescriptor`. Since
primary indexes cannot be not visible, it added another test 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: #83471

Assists: #72576

Release note: none
  • Loading branch information
wenyihu6 committed Jul 21, 2022
1 parent 4a8f1be commit a4610f6
Show file tree
Hide file tree
Showing 24 changed files with 127 additions and 16 deletions.
3 changes: 3 additions & 0 deletions pkg/ccl/schemachangerccl/testdata/decomp/multiregion
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down Expand Up @@ -368,6 +369,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down Expand Up @@ -627,6 +629,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down
4 changes: 4 additions & 0 deletions pkg/ccl/schemachangerccl/testdata/decomp/partitioning
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand All @@ -106,6 +107,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: true
isNotVisible: false
isUnique: false
sharding: null
sourceIndexId: 0
Expand Down Expand Up @@ -459,6 +461,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand All @@ -471,6 +474,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down
12 changes: 6 additions & 6 deletions pkg/cli/testdata/doctor/test_recreate_zipdir

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions pkg/sql/catalog/catformat/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ func indexForDisplay(
}
}

if index.NotVisible {
f.WriteString(" NOT VISIBLE")
}

return f.CloseAndGetString(), nil
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/sql/catalog/descpb/structured.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/catalog/table_elements.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ type Index interface {
IsUnique() bool
IsDisabled() bool
IsSharded() bool
IsNotVisible() bool
IsCreatedExplicitly() bool
GetPredicate() string
GetType() descpb.IndexDescriptor_Type
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/catalog/tabledesc/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/catalog/tabledesc/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
31 changes: 31 additions & 0 deletions pkg/sql/catalog/tabledesc/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/opt/cat/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion pkg/sql/opt/cat/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
16 changes: 8 additions & 8 deletions pkg/sql/opt/exec/execbuilder/testdata/show_trace

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions pkg/sql/opt/exec/explain/plan_gist_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/opt/indexrec/hypothetical_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ type hypotheticalIndex struct {

// inverted indicates if an index is inverted.
inverted bool

// notVisible indicates if an index is not visible to the optimizer.
notVisible bool
}

var _ cat.Index = &hypotheticalIndex{}
Expand Down Expand Up @@ -111,6 +114,11 @@ func (hi *hypotheticalIndex) IsInverted() bool {
return hi.inverted
}

// IsNotVisible is part of the cat.Index interface.
func (hi *hypotheticalIndex) IsNotVisible() bool {
return hi.notVisible
}

// ColumnCount is part of the cat.Index interface.
func (hi *hypotheticalIndex) ColumnCount() int {
return len(hi.cols) + len(hi.suffixKeyColsOrdList) + hi.storedColsOrdSet.Len()
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/opt/testutils/testcat/test_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions pkg/sql/opt_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 hidden property after CREATE
}
var relation scpb.Element
var source *scpb.PrimaryIndex
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/schemachanger/scdecomp/decomp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/schemachanger/scdecomp/testdata/sequence
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down Expand Up @@ -422,6 +423,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/schemachanger/scdecomp/testdata/table
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down Expand Up @@ -271,6 +272,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand All @@ -283,6 +285,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: false
sharding: null
sourceIndexId: 0
Expand Down Expand Up @@ -641,6 +644,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down Expand Up @@ -882,6 +886,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/schemachanger/scdecomp/testdata/type
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: true
sharding: null
sourceIndexId: 0
Expand All @@ -165,6 +166,7 @@ ElementState:
isConcurrently: false
isCreatedExplicitly: false
isInverted: false
isNotVisible: false
isUnique: false
sharding: null
sourceIndexId: 0
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/schemachanger/scexec/scmutationexec/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/schemachanger/scpb/elements.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit a4610f6

Please sign in to comment.