Skip to content

Commit 03f9710

Browse files
committed
compose: deflake TestComposeCompare
Previously, the compose test had a mutation that added partial indexes to random indexes. This could cause unique indexes to become unusable for foreign keys, an issue that was being incorrectly ignored in the schema changer. A recent change fixed this bug, which now causes these scenarios to fail correctly. To address this, this patch modifies the mutator inside compose to only add partial indexes on new, duplicate indexes so that original unique constraints are not lost. Fixes: #150817 Fixes: #149565 Release note: None
1 parent 83a437d commit 03f9710

File tree

2 files changed

+84
-1
lines changed

2 files changed

+84
-1
lines changed

pkg/compose/compare/compare/compare_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func TestCompare(t *testing.T) {
9393
randgen.ForeignKeyMutator,
9494
randgen.ColumnFamilyMutator,
9595
randgen.IndexStoringMutator,
96-
randgen.PartialIndexMutator,
96+
randgen.DupPartialIndexMutator,
9797
},
9898
},
9999
},

pkg/sql/randgen/mutator.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"bytes"
1010
"context"
1111
"encoding/json"
12+
"fmt"
1213
"math/rand"
1314
"regexp"
1415
"sort"
@@ -44,6 +45,10 @@ var (
4445
// indexes.
4546
PartialIndexMutator MultiStatementMutation = partialIndexMutator
4647

48+
// DupPartialIndexMutator create a new random partial index based on an
49+
// existing index that is selected.
50+
DupPartialIndexMutator MultiStatementMutation = duplicatePartialIndexMutator
51+
4752
// PostgresMutator modifies strings such that they execute identically
4853
// in both Postgres and Cockroach (however this mutator does not remove
4954
// features not supported by Postgres; use PostgresCreateTableMutator
@@ -1140,6 +1145,84 @@ func partialIndexMutator(rng *rand.Rand, stmts []tree.Statement) ([]tree.Stateme
11401145
return stmts, changed
11411146
}
11421147

1148+
// duplicatePartialIndexMutator is a mutations.MultiStatementMutator that will
1149+
// copy a random index definition and make it a partial index. Unlike
1150+
// partialIndexMutator, the existing definition will stay the same.
1151+
func duplicatePartialIndexMutator(rng *rand.Rand, stmts []tree.Statement) ([]tree.Statement, bool) {
1152+
changed := false
1153+
tables := getTableInfoFromDDLStatements(stmts)
1154+
var newStmts []tree.Statement
1155+
for _, stmt := range stmts {
1156+
switch ast := stmt.(type) {
1157+
case *tree.CreateIndex:
1158+
info, ok := tables[ast.Table.ObjectName]
1159+
if !ok {
1160+
continue
1161+
}
1162+
1163+
// If the index is not already a partial index, make it a partial index
1164+
// with a 50% chance. Do not mutate an index that was created to satisfy a
1165+
// FK constraint.
1166+
if ast.Predicate == nil &&
1167+
!hasReferencingConstraint(info, ast.Columns) &&
1168+
rng.Intn(2) == 0 {
1169+
astString := tree.AsStringWithFlags(ast, tree.FmtParsableNumerics)
1170+
astCopy, err := parser.ParseOne(astString)
1171+
if err != nil {
1172+
panic(err)
1173+
}
1174+
createIndexCopy := astCopy.AST.(*tree.CreateIndex)
1175+
if len(createIndexCopy.Name) > 0 {
1176+
createIndexCopy.Name = tree.Name(fmt.Sprintf("%s_partial", createIndexCopy.Name))
1177+
}
1178+
tn := tree.MakeUnqualifiedTableName(createIndexCopy.Table.ObjectName)
1179+
createIndexCopy.Predicate = randPartialIndexPredicateFromCols(rng, info.columnsTableDefs, &tn)
1180+
changed = true
1181+
newStmts = append(newStmts, createIndexCopy)
1182+
}
1183+
case *tree.CreateTable:
1184+
info, ok := tables[ast.Table.ObjectName]
1185+
if !ok {
1186+
panic("table info could not be found")
1187+
}
1188+
var newDefs []tree.TableDef
1189+
for _, def := range ast.Defs {
1190+
var idx *tree.IndexTableDef
1191+
switch defType := def.(type) {
1192+
case *tree.IndexTableDef:
1193+
idx = defType
1194+
case *tree.UniqueConstraintTableDef:
1195+
if !defType.PrimaryKey && !defType.WithoutIndex {
1196+
idx = &defType.IndexTableDef
1197+
}
1198+
}
1199+
1200+
if idx == nil {
1201+
continue
1202+
}
1203+
1204+
// If the index is not already a partial index, make it a partial
1205+
// index with a 50% chance.
1206+
if idx.Predicate == nil &&
1207+
!hasReferencingConstraint(info, idx.Columns) &&
1208+
rng.Intn(2) == 0 {
1209+
idxCopy := *idx
1210+
if len(idxCopy.Name) > 0 {
1211+
idxCopy.Name = tree.Name(fmt.Sprintf("%s_partial", idxCopy.Name))
1212+
}
1213+
tn := tree.MakeUnqualifiedTableName(ast.Table.ObjectName)
1214+
idxCopy.Predicate = randPartialIndexPredicateFromCols(rng, info.columnsTableDefs, &tn)
1215+
changed = true
1216+
newDefs = append(newDefs, &idxCopy)
1217+
}
1218+
}
1219+
ast.Defs = append(ast.Defs, newDefs...)
1220+
}
1221+
}
1222+
stmts = append(stmts, newStmts...)
1223+
return stmts, changed
1224+
}
1225+
11431226
// hasReferencingConstraint returns true if the tableInfo has any referencing
11441227
// columns that match idxColumns.
11451228
func hasReferencingConstraint(info tableInfo, idxColumns tree.IndexElemList) bool {

0 commit comments

Comments
 (0)