opt: make index recommendations deterministic with partially visible …

…indexes

Prior to this commit, index recommendations could change randomly between runs
of the same query if any partially visible indexes existed in the tables used
by the query. This was because the logic to randomly use a partially visible
index for some fraction of queries was also being applied when generating index
recommendations. This commit makes index recommendations deterministic in this
case, so that we treat partially visible indexes as fully invisible for the
purpose of index recommendations, and we always recommend making a partially
visible index fully visible if it can be used to improve a query plan.

Informs cockroachdb#82363

There is no release note since partially visible indexes are not yet part of
any release.

Release note: None

pkg/sql/opt/cat/table.go

@@ -176,6 +176,10 @@ type Table interface {
 	// GetDatabaseID returns the owning database id of the table, or zero, if the
 	// owning database could not be determined.
 	GetDatabaseID() descpb.ID
+
+	// IsHypothetical returns true if this is a hypothetical table (used when
+	// searching for index recommendations).
+	IsHypothetical() bool
 }
 
 // CheckConstraint contains the SQL text and the validity status for a check

pkg/sql/opt/exec/explain/plan_gist_factory.go

@@ -617,6 +617,11 @@ func (u *unknownTable) GetDatabaseID() descpb.ID {
 	return 0
 }
 
+// IsHypothetical is part of the cat.Table interface.
+func (u *unknownTable) IsHypothetical() bool {
+	return false
+}
+
 var _ cat.Table = &unknownTable{}
 
 // unknownTable implements the cat.Index interface and is used to represent

pkg/sql/opt/indexrec/hypothetical_table.go

@@ -144,6 +144,11 @@ func (ht *HypotheticalTable) Index(i cat.IndexOrdinal) cat.Index {
 	return &ht.hypotheticalIndexes[i-existingIndexCount]
 }
 
+// IsHypothetical is part of the cat.Table interface.
+func (ht *HypotheticalTable) IsHypothetical() bool {
+	return true
+}
+
 // FullyQualifiedName returns the fully qualified name of the hypothetical
 // table.
 func (ht *HypotheticalTable) FullyQualifiedName(ctx context.Context) (cat.DataSourceName, error) {

pkg/sql/opt/indexrec/testdata/index

@@ -2178,8 +2178,10 @@ CREATE TABLE t_notvisible (
   k INT PRIMARY KEY,
   v INT,
   i INT,
+  p INT,
   INDEX idx_i_visible(i) VISIBLE,
-  INDEX idx_v_invisible(v) NOT VISIBLE
+  INDEX idx_v_invisible(v) NOT VISIBLE,
+  INDEX idx_p_halfvisible(p) VISIBILITY 0.5
 )
 ----
 
@@ -2203,10 +2205,22 @@ SELECT v FROM t_notvisible WHERE v > 1
 alteration: ALTER INDEX t.public.t_notvisible@idx_v_invisible VISIBLE;
 --
 optimal plan:
-scan t_notvisible@_hyp_3
+scan t_notvisible@_hyp_4
  ├── columns: v:2!null
  ├── constraint: /2/1: [/2 - ]
- └── cost: 364.686667
+ └── cost: 368.02
+
+# Same with partially visible index.
+index-recommendations
+SELECT p FROM t_notvisible WHERE p > 1
+----
+alteration: ALTER INDEX t.public.t_notvisible@idx_p_halfvisible VISIBLE;
+--
+optimal plan:
+scan t_notvisible@_hyp_4
+ ├── columns: p:4!null
+ ├── constraint: /4/1: [/2 - ]
+ └── cost: 368.02
 
 # If the index is not visible and need to store more columns, still recommend
 # create index. We do not want to recommend dropping any invisible index.
@@ -2218,11 +2232,26 @@ creation: CREATE INDEX ON t.public.t_notvisible (v) STORING (i);
 optimal plan:
 project
  ├── columns: i:3
- ├── cost: 371.373333
- └── scan t_notvisible@_hyp_3
+ ├── cost: 374.706667
+ └── scan t_notvisible@_hyp_4
       ├── columns: v:2!null i:3
       ├── constraint: /2/1: [/2 - ]
-      └── cost: 368.02
+      └── cost: 371.353333
+
+# Same with partially visible index.
+index-recommendations
+SELECT i FROM t_notvisible WHERE p > 1
+----
+creation: CREATE INDEX ON t.public.t_notvisible (p) STORING (i);
+--
+optimal plan:
+project
+ ├── columns: i:3
+ ├── cost: 374.706667
+ └── scan t_notvisible@_hyp_4
+      ├── columns: i:3 p:4!null
+      ├── constraint: /4/1: [/2 - ]
+      └── cost: 371.353333
 
 # If there exists both not visible and visible indexes storing the same explicit
 # column, the index recommendation should not recommend alter index just because
@@ -2242,6 +2271,22 @@ scan t_notvisible@idx_v_visible
  ├── constraint: /2/1: [/2 - ]
  └── cost: 361.353333
 
+# Same with partially visible index.
+exec-ddl
+CREATE INDEX idx_p_visible ON t_notvisible(p) VISIBLE
+----
+
+index-recommendations
+SELECT p FROM t_notvisible WHERE p > 1
+----
+no index recommendations
+--
+optimal plan:
+scan t_notvisible@idx_p_visible
+ ├── columns: p:4!null
+ ├── constraint: /4/1: [/2 - ]
+ └── cost: 361.353333
+
 # If there exists both visible and invisible index, the index recommendation
 # should not recommend dropping the not visible index even if it stores the same
 # key column.
@@ -2253,11 +2298,26 @@ replacement: CREATE INDEX ON t.public.t_notvisible (v) STORING (i); DROP INDEX t
 optimal plan:
 project
  ├── columns: i:3
- ├── cost: 371.373333
- └── scan t_notvisible@_hyp_4
+ ├── cost: 374.706667
+ └── scan t_notvisible@_hyp_6
       ├── columns: v:2!null i:3
       ├── constraint: /2/1: [/2 - ]
-      └── cost: 368.02
+      └── cost: 371.353333
+
+# Same with partially visible index.
+index-recommendations
+SELECT i FROM t_notvisible WHERE p > 1
+----
+replacement: CREATE INDEX ON t.public.t_notvisible (p) STORING (i); DROP INDEX t.public.t_notvisible@idx_p_visible;
+--
+optimal plan:
+project
+ ├── columns: i:3
+ ├── cost: 374.706667
+ └── scan t_notvisible@_hyp_6
+      ├── columns: i:3 p:4!null
+      ├── constraint: /4/1: [/2 - ]
+      └── cost: 371.353333
 
 # Regression test for #108490. Alter the correct index when there are multiple
 # invisible indexes.

pkg/sql/opt/table_meta.go

@@ -226,11 +226,17 @@ func (tm *TableMeta) IsIndexNotVisible(indexOrd cat.IndexOrdinal, rng *rand.Rand
 	if tm.notVisibleIndexMap == nil {
 		tm.notVisibleIndexMap = make(map[cat.IndexOrdinal]bool)
 	}
+	// See if the visibility is already cached.
 	if val, ok := tm.notVisibleIndexMap[indexOrd]; ok {
 		return val
 	}
 	// Otherwise, roll the dice to assign index visibility.
 	indexInvisibility := tm.Table.Index(indexOrd).GetInvisibility()
+	// If we are making an index recommendation, we do not want to use partially
+	// visible indexes.
+	if tm.Table.IsHypothetical() && indexInvisibility != 0 {
+		indexInvisibility = 1
+	}
 
 	// If the index invisibility is 40%, we want to make this index invisible 40%
 	// of the time (invisible to 40% of the queries).

pkg/sql/opt/testutils/testcat/test_catalog.go

@@ -931,6 +931,11 @@ func (tt *Table) GetDatabaseID() descpb.ID {
 	return tt.DatabaseID
 }
 
+// IsHypothetical is part of the cat.Table interface.
+func (tt *Table) IsHypothetical() bool {
+	return false
+}
+
 // FindOrdinal returns the ordinal of the column with the given name.
 func (tt *Table) FindOrdinal(name string) int {
 	for i, col := range tt.Columns {

pkg/sql/opt_catalog.go

@@ -1369,6 +1369,11 @@ func (ot *optTable) GetDatabaseID() descpb.ID {
 	return ot.desc.GetParentID()
 }
 
+// IsHypothetical is part of the cat.Table interface.
+func (ot *optTable) IsHypothetical() bool {
+	return false
+}
+
 // lookupColumnOrdinal returns the ordinal of the column with the given ID. A
 // cache makes the lookup O(1).
 func (ot *optTable) lookupColumnOrdinal(colID descpb.ColumnID) (int, error) {
@@ -2358,6 +2363,11 @@ func (ot *optVirtualTable) GetDatabaseID() descpb.ID {
 	return 0
 }
 
+// IsHypothetical is part of the cat.Table interface.
+func (ot *optVirtualTable) IsHypothetical() bool {
+	return false
+}
+
 // CollectTypes is part of the cat.DataSource interface.
 func (ot *optVirtualTable) CollectTypes(ord int) (descpb.IDs, error) {
 	col := ot.desc.AllColumns()[ord]