sql: fix index visibility parsing for small values

This commit updates the parser and AST to keep track of whether or not
an index `VISIBILITY` float was provided in a `CREATE TABLE`,
`CREATE INDEX`, or `ALTER INDEX` statement. This allows the statements
to be round-tripped correctly with very low float values that would be
rounded to zero and change the `VISIBILITY` clause into `NOT VISIBLE`.

Fixes #108083

Release note: None

pkg/internal/sqlsmith/alter.go

@@ -364,11 +364,12 @@ func makeCreateIndex(s *Smither) (tree.Statement, bool) {
 		storing = append(storing, col.Name)
 	}
 
-	visibility := 1.0
+	invisibility := tree.IndexInvisibility{Value: 0.0}
 	if notvisible := s.d6() == 1; notvisible {
-		visibility = 0.0
+		invisibility.Value = 1.0
 		if s.coin() {
-			visibility = s.rnd.Float64() // [0.0, 1.0)
+			invisibility.Value = s.rnd.Float64() // [0.0, 1.0)
+			invisibility.FloatProvided = true
 		}
 	}
 
@@ -380,7 +381,7 @@ func makeCreateIndex(s *Smither) (tree.Statement, bool) {
 		Storing:      storing,
 		Inverted:     inverted,
 		Concurrently: s.coin(),
-		Invisibility: 1 - visibility,
+		Invisibility: invisibility,
 	}, true
 }
 

pkg/sql/alter_index_visible.go

@@ -82,19 +82,19 @@ func (p *planner) AlterIndexVisible(
 func (n *alterIndexVisibleNode) ReadingOwnWrites() {}
 
 func (n *alterIndexVisibleNode) startExec(params runParams) error {
-	if n.n.Invisibility != 0.0 && n.index.Primary() {
+	if n.n.Invisibility.Value != 0.0 && n.index.Primary() {
 		return pgerror.Newf(pgcode.FeatureNotSupported, "primary index cannot be invisible")
 	}
 
 	activeVersion := params.ExecCfg().Settings.Version.ActiveVersion(params.ctx)
 	if !activeVersion.IsActive(clusterversion.V23_2_PartiallyVisibleIndexes) &&
-		n.n.Invisibility > 0.0 && n.n.Invisibility < 1.0 {
+		n.n.Invisibility.Value > 0.0 && n.n.Invisibility.Value < 1.0 {
 		return unimplemented.New("partially visible indexes", "partially visible indexes are not yet supported")
 	}
 
 	// Warn if this invisible index may still be used to enforce constraint check
 	// behind the scene.
-	if n.n.Invisibility != 0.0 {
+	if n.n.Invisibility.Value != 0.0 {
 		if notVisibleIndexNotice := tabledesc.ValidateNotVisibleIndex(n.index, n.tableDesc); notVisibleIndexNotice != nil {
 			params.p.BufferClientNotice(
 				params.ctx,
@@ -103,13 +103,13 @@ func (n *alterIndexVisibleNode) startExec(params runParams) error {
 		}
 	}
 
-	if n.index.GetInvisibility() == n.n.Invisibility {
+	if n.index.GetInvisibility() == n.n.Invisibility.Value {
 		// Nothing needed if the index is already what they want.
 		return nil
 	}
 
-	n.index.IndexDesc().NotVisible = n.n.Invisibility != 0.0
-	n.index.IndexDesc().Invisibility = n.n.Invisibility
+	n.index.IndexDesc().NotVisible = n.n.Invisibility.Value != 0.0
+	n.index.IndexDesc().Invisibility = n.n.Invisibility.Value
 
 	if err := validateDescriptor(params.ctx, params.p, n.tableDesc); err != nil {
 		return err
@@ -126,8 +126,8 @@ func (n *alterIndexVisibleNode) startExec(params runParams) error {
 		&eventpb.AlterIndexVisible{
 			TableName:    n.n.Index.Table.FQString(),
 			IndexName:    n.index.GetName(),
-			NotVisible:   n.n.Invisibility != 0.0,
-			Invisibility: n.n.Invisibility,
+			NotVisible:   n.n.Invisibility.Value != 0.0,
+			Invisibility: n.n.Invisibility.Value,
 		})
 }
 func (n *alterIndexVisibleNode) Next(runParams) (bool, error) { return false, nil }

pkg/sql/alter_table.go

@@ -299,14 +299,14 @@ func (n *alterTableNode) startExec(params runParams) error {
 
 				activeVersion := params.ExecCfg().Settings.Version.ActiveVersion(params.ctx)
 				if !activeVersion.IsActive(clusterversion.V23_2_PartiallyVisibleIndexes) &&
-					d.Invisibility > 0.0 && d.Invisibility < 1.0 {
+					d.Invisibility.Value > 0.0 && d.Invisibility.Value < 1.0 {
 					return unimplemented.New("partially visible indexes", "partially visible indexes are not yet supported")
 				}
 				idx := descpb.IndexDescriptor{
 					Name:             string(d.Name),
 					Unique:           true,
-					NotVisible:       d.Invisibility != 0.0,
-					Invisibility:     d.Invisibility,
+					NotVisible:       d.Invisibility.Value != 0.0,
+					Invisibility:     d.Invisibility.Value,
 					StoreColumnNames: d.Storing.ToStrings(),
 					CreatedAtNanos:   params.EvalContext().GetTxnTimestamp(time.Microsecond).UnixNano(),
 				}

pkg/sql/create_index.go

@@ -200,7 +200,7 @@ func makeIndexDescriptor(
 	}
 
 	if !activeVersion.IsActive(clusterversion.V23_2_PartiallyVisibleIndexes) &&
-		n.Invisibility > 0.0 && n.Invisibility < 1.0 {
+		n.Invisibility.Value > 0.0 && n.Invisibility.Value < 1.0 {
 		return nil, unimplemented.New("partially visible indexes", "partially visible indexes are not yet supported")
 	}
 	indexDesc := descpb.IndexDescriptor{
@@ -209,8 +209,8 @@ func makeIndexDescriptor(
 		StoreColumnNames:  n.Storing.ToStrings(),
 		CreatedExplicitly: true,
 		CreatedAtNanos:    params.EvalContext().GetTxnTimestamp(time.Microsecond).UnixNano(),
-		NotVisible:        n.Invisibility != 0.0,
-		Invisibility:      n.Invisibility,
+		NotVisible:        n.Invisibility.Value != 0.0,
+		Invisibility:      n.Invisibility.Value,
 	}
 
 	if n.Inverted {

pkg/sql/create_table.go

@@ -708,7 +708,7 @@ func addUniqueWithoutIndexTableDef(
 			"partitioned unique constraints without an index are not supported",
 		)
 	}
-	if d.Invisibility != 0.0 {
+	if d.Invisibility.Value != 0.0 {
 		// Theoretically, this should never happen because this is not supported by
 		// the parser. This is just a safe check.
 		return pgerror.Newf(pgcode.FeatureNotSupported,
@@ -1829,15 +1829,15 @@ func NewTableDesc(
 				return nil, err
 			}
 			if !version.IsActive(clusterversion.V23_2_PartiallyVisibleIndexes) &&
-				d.Invisibility > 0.0 && d.Invisibility < 1.0 {
+				d.Invisibility.Value > 0.0 && d.Invisibility.Value < 1.0 {
 				return nil, unimplemented.New("partially visible indexes", "partially visible indexes are not yet supported")
 			}
 			idx := descpb.IndexDescriptor{
 				Name:             string(d.Name),
 				StoreColumnNames: d.Storing.ToStrings(),
 				Version:          indexEncodingVersion,
-				NotVisible:       d.Invisibility != 0.0,
-				Invisibility:     d.Invisibility,
+				NotVisible:       d.Invisibility.Value != 0.0,
+				Invisibility:     d.Invisibility.Value,
 			}
 			if d.Inverted {
 				idx.Type = descpb.IndexDescriptor_INVERTED
@@ -1948,16 +1948,16 @@ func NewTableDesc(
 				return nil, err
 			}
 			if !version.IsActive(clusterversion.V23_2_PartiallyVisibleIndexes) &&
-				d.Invisibility > 0.0 && d.Invisibility < 1.0 {
+				d.Invisibility.Value > 0.0 && d.Invisibility.Value < 1.0 {
 				return nil, unimplemented.New("partially visible indexes", "partially visible indexes are not yet supported")
 			}
 			idx := descpb.IndexDescriptor{
 				Name:             string(d.Name),
 				Unique:           true,
 				StoreColumnNames: d.Storing.ToStrings(),
 				Version:          indexEncodingVersion,
-				NotVisible:       d.Invisibility != 0.0,
-				Invisibility:     d.Invisibility,
+				NotVisible:       d.Invisibility.Value != 0.0,
+				Invisibility:     d.Invisibility.Value,
 			}
 			columns := d.Columns
 			if d.Sharded != nil {
@@ -2660,7 +2660,7 @@ func replaceLikeTableOpts(n *tree.CreateTable, params runParams) (tree.TableDefs
 					Inverted:     idx.GetType() == descpb.IndexDescriptor_INVERTED,
 					Storing:      make(tree.NameList, 0, idx.NumSecondaryStoredColumns()),
 					Columns:      make(tree.IndexElemList, 0, idx.NumKeyColumns()),
-					Invisibility: idx.GetInvisibility(),
+					Invisibility: tree.IndexInvisibility{Value: idx.GetInvisibility()},
 				}
 				numColumns := idx.NumKeyColumns()
 				if idx.IsSharded() {

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

@@ -863,7 +863,7 @@ func (tt *Table) addIndexWithVersion(
 		IdxZone:      cat.EmptyZone(),
 		table:        tt,
 		version:      version,
-		Invisibility: def.Invisibility,
+		Invisibility: def.Invisibility.Value,
 	}
 
 	// Look for name suffixes indicating this is a mutation index.