Skip to content

Commit 70c7a72

Browse files
mattlordmattlord
authored
OnlineDDL: Make ENUM/SET eligibility for INSTANT DDL more robust (#19425)
Signed-off-by: Matt Lord <mattalord@gmail.com>
1 parent 0506181 commit 70c7a72

2 files changed

Lines changed: 174 additions & 12 deletions

File tree

‎go/vt/schemadiff/capability.go‎

Lines changed: 144 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package schemadiff
22

33
import (
4+
"fmt"
5+
"log/slog"
46
"strings"
57

68
"vitess.io/vitess/go/mysql/capabilities"
9+
"vitess.io/vitess/go/vt/log"
710
"vitess.io/vitess/go/vt/sqlparser"
811
)
912

@@ -13,7 +16,13 @@ const (
1316

1417
// alterOptionAvailableViaInstantDDL checks if the specific alter option is eligible to run via ALGORITHM=INSTANT
1518
// reference: https://dev.mysql.com/doc/refman/8.0/en/innodb-online-ddl-operations.html
16-
func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTable *sqlparser.CreateTable, capableOf capabilities.CapableOf) (bool, error) {
19+
func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTable *sqlparser.CreateTable, capableOf capabilities.CapableOf) (eligible bool, err error) {
20+
log.Info("Checking if ALTER is eligible for INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
21+
defer func() {
22+
if eligible {
23+
log.Info("ALTER is eligible for INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
24+
}
25+
}()
1726
// A table with FULLTEXT index won't support adding/removing columns instantly.
1827
tableHasFulltextIndex := false
1928
for _, key := range createTable.TableSpec.Indexes {
@@ -68,11 +77,39 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
6877
strippedCol.Type.Options.DefaultLiteral = false
6978
// strip `visibility`
7079
strippedCol.Type.Options.Invisible = nil
80+
// Normalize explicit NULL to implicit nullable: SHOW CREATE TABLE may add the NULL
81+
// keyword explicitly for nullable columns, but user ALTER statements typically omit it.
82+
// Both mean the same thing, so we treat them as equivalent.
83+
if strippedCol.Type.Options.Null != nil && *strippedCol.Type.Options.Null {
84+
strippedCol.Type.Options.Null = nil
85+
}
86+
// Normalize the type name to lowercase. The CanonicalString formatter uses %#s for the
87+
// column type name which writes it as-is (case-sensitive), so "enum" and "ENUM" would
88+
// produce different canonical strings without this normalization.
89+
strippedCol.Type.Type = strings.ToLower(strippedCol.Type.Type)
90+
// Strip charset and collation: SHOW CREATE TABLE adds the column's collation explicitly
91+
// when it is inherited from a table-level COLLATE clause. User ALTER statements typically
92+
// omit the collation. Stripping both sides allows a semantic comparison.
93+
// Genuine charset/collation changes are detected separately via isCharsetOrCollationChange.
94+
strippedCol.Type.Charset.Name = ""
95+
strippedCol.Type.Options.Collate = ""
7196
if stripEnum {
7297
strippedCol.Type.EnumValues = nil
7398
}
7499
return sqlparser.CanonicalString(strippedCol)
75100
}
101+
// isCharsetOrCollationChange returns true if the new column definition explicitly specifies
102+
// a different charset or collation than the old one. A missing value in newCol means the
103+
// user did not specify it, which is treated as "no change".
104+
isCharsetOrCollationChange := func(col, newCol *sqlparser.ColumnDefinition) bool {
105+
if newCol.Type.Charset.Name != "" && !strings.EqualFold(col.Type.Charset.Name, newCol.Type.Charset.Name) {
106+
return true
107+
}
108+
if newCol.Type.Options.Collate != "" && !strings.EqualFold(col.Type.Options.Collate, newCol.Type.Options.Collate) {
109+
return true
110+
}
111+
return false
112+
}
76113
hasPrefix := func(vals []string, prefix []string) bool {
77114
if len(vals) < len(prefix) {
78115
return false
@@ -92,8 +129,15 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
92129
// is instant-table.
93130
tableColDefinition := colStringStrippedDown(col, false)
94131
newColDefinition := colStringStrippedDown(newCol, false)
95-
if tableColDefinition == newColDefinition {
96-
return capableOf(capabilities.InstantChangeColumnDefaultFlavorCapability)
132+
if tableColDefinition == newColDefinition && !isCharsetOrCollationChange(col, newCol) {
133+
capable, err := capableOf(capabilities.InstantChangeColumnDefaultFlavorCapability)
134+
if !capable {
135+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support changing a column default with INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
136+
}
137+
if err != nil {
138+
log.Error("Error while checking MySQL server INSTANT DDL capability for ALTER", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.Any("error", err))
139+
}
140+
return capable, err
97141
}
98142
// Check if:
99143
// 1. this an ENUM/SET
@@ -103,28 +147,49 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
103147
if len(col.Type.EnumValues) > 0 && len(newCol.Type.EnumValues) > 0 {
104148
// both are enum or set
105149
if !hasPrefix(newCol.Type.EnumValues, col.Type.EnumValues) {
150+
log.Info("ALTER is not eligible for INSTANT DDL because the new ENUM/SET values do not have the old values as prefix",
151+
slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("old_columns", strings.Join(col.Type.EnumValues, ",")), slog.String("new_columns", strings.Join(newCol.Type.EnumValues, ",")))
106152
return false, nil
107153
}
108154
// we know the new column definition is identical to, or extends, the old definition.
109155
// Now validate storage:
110156
if strings.EqualFold(col.Type.Type, "enum") {
111157
if len(col.Type.EnumValues) <= 255 && len(newCol.Type.EnumValues) > 255 {
112-
// this increases the SET storage size (1 byte for up to 8 values, 2 bytes beyond)
158+
// this increases the ENUM storage size (1 byte for up to 255 values, 2 bytes beyond)
159+
log.Info("ALTER is not eligible for INSTANT DDL because we would be crossing the 255 enum value count, which increases the storage size from 1 to 2 bytes",
160+
slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("old_columns", strings.Join(col.Type.EnumValues, ",")), slog.String("new_columns", strings.Join(newCol.Type.EnumValues, ",")))
113161
return false, nil
114162
}
115163
}
116164
if strings.EqualFold(col.Type.Type, "set") {
117165
if (len(col.Type.EnumValues)+7)/8 != (len(newCol.Type.EnumValues)+7)/8 {
118166
// this increases the SET storage size (1 byte for up to 8 values, 2 bytes for 8-15, etc.)
167+
log.Info("ALTER is not eligible for INSTANT DDL because we would be crossing the 8 SET value count, which increases the storage size from 1 to 2 bytes.",
168+
slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("old_columns", strings.Join(col.Type.EnumValues, ",")), slog.String("new_columns", strings.Join(newCol.Type.EnumValues, ",")))
119169
return false, nil
120170
}
121171
}
122172
// Now don't care about change of default:
123173
tableColDefinition := colStringStrippedDown(col, true)
124174
newColDefinition := colStringStrippedDown(newCol, true)
125175
if tableColDefinition == newColDefinition {
126-
return capableOf(capabilities.InstantExpandEnumCapability)
176+
if isCharsetOrCollationChange(col, newCol) {
177+
log.Info("ALTER is not eligible for INSTANT DDL because the charset or collation of the column is being changed.",
178+
slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("old_column", sqlparser.CanonicalString(col)), slog.String("new_column", sqlparser.CanonicalString(newCol)))
179+
return false, nil
180+
}
181+
capable, err := capableOf(capabilities.InstantExpandEnumCapability)
182+
if !capable {
183+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support INSTANT DDL for expanding ENUM/SET columns",
184+
slog.String("alter", sqlparser.CanonicalString(alterOption)))
185+
}
186+
if err != nil {
187+
log.Error(fmt.Sprintf("Error while checking MySQL server INSTANT DDL capability for ALTER %q: %v", sqlparser.CanonicalString(alterOption), err))
188+
}
189+
return capable, err
127190
}
191+
log.Info("ALTER is not eligible for INSTANT DDL because the base column definition has been changed",
192+
slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("old_column", tableColDefinition), slog.String("new_column", newColDefinition))
128193
}
129194
return false, nil
130195
}
@@ -134,65 +199,103 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
134199
case *sqlparser.AddColumns:
135200
if tableHasFulltextIndex {
136201
// not supported if the table has a FULLTEXT index
202+
log.Info("ALTER is not eligible for INSTANT DDL because the table has a FULLTEXT index",
203+
slog.String("alter", sqlparser.CanonicalString(alterOption)))
137204
return false, nil
138205
}
139206
// Not supported in COMPRESSED tables
140207
if tableIsCompressed {
208+
log.Info("ALTER is not eligible for INSTANT DDL because the table is compressed", slog.String("alter", sqlparser.CanonicalString(alterOption)))
141209
return false, nil
142210
}
143211
for _, column := range opt.Columns {
144212
if isGenerated, storage := IsGeneratedColumn(column); isGenerated {
145213
if storage == sqlparser.StoredStorage {
146214
// Adding a generated "STORED" column is unsupported
215+
log.Info("ALTER is not eligible for INSTANT DDL because the column is a generated value",
216+
slog.String("alter", sqlparser.CanonicalString(alterOption)))
147217
return false, nil
148218
}
149219
}
150220
if column.Type.Options.Default != nil && !column.Type.Options.DefaultLiteral {
151221
// Expression default values are not supported
222+
log.Info("ALTER is not eligible for INSTANT DDL because the column has a DEFAULT function",
223+
slog.String("alter", sqlparser.CanonicalString(alterOption)))
152224
return false, nil
153225
}
154226
if strings.EqualFold(column.Type.Type, "datetime") {
155227
e := &ColumnDefinitionEntity{ColumnDefinition: column}
156228
if !e.IsNullable() && !e.HasDefault() {
157229
// DATETIME columns must have a default value
230+
log.Info("ALTER is not eligible for INSTANT DDL because the datetime column has no DEFAULT",
231+
slog.String("alter", sqlparser.CanonicalString(alterOption)))
158232
return false, nil
159233
}
160234
}
161235
}
162236
if opt.First || opt.After != nil {
163237
// not a "last" column. Only supported as of 8.0.29
164-
return capableOf(capabilities.InstantAddDropColumnFlavorCapability)
238+
capable, err := capableOf(capabilities.InstantAddDropColumnFlavorCapability)
239+
if !capable {
240+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support FIRST or AFTER clauses with INSTANT DDL",
241+
slog.String("alter", sqlparser.CanonicalString(alterOption)))
242+
}
243+
if err != nil {
244+
log.Error(fmt.Sprintf("Error while checking MySQL server INSTANT DDL capability for ALTER %q: %v", sqlparser.CanonicalString(alterOption), err))
245+
}
246+
return capable, err
165247
}
166248
// Adding a *last* column is supported in 8.0
167-
return capableOf(capabilities.InstantAddLastColumnFlavorCapability)
249+
capable, err := capableOf(capabilities.InstantAddLastColumnFlavorCapability)
250+
if !capable {
251+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support LAST clauses with INSTANT DDL",
252+
slog.String("alter", sqlparser.CanonicalString(alterOption)))
253+
}
254+
if err != nil {
255+
log.Error("Error while checking MySQL server INSTANT DDL capability for ALTER",
256+
slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.Any("error", err))
257+
}
258+
return capable, err
168259
case *sqlparser.DropColumn:
169260
col := findColumn(opt.Name.Name.String())
170261
if col == nil {
171262
// column not found
263+
log.Info("ALTER is not eligible for INSTANT DDL because the column was not found", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("column", opt.Name.Name.String()))
172264
return false, nil
173265
}
174266
if tableHasFulltextIndex {
175267
// not supported if the table has a FULLTEXT index
268+
log.Info("ALTER is not eligible for INSTANT DDL because the table has a FULLTEXT index", slog.String("alter", sqlparser.CanonicalString(alterOption)))
176269
return false, nil
177270
}
178271
// Not supported in COMPRESSED tables
179272
if tableIsCompressed {
273+
log.Info("ALTER is not eligible for INSTANT DDL because the table is compressed", slog.String("alter", sqlparser.CanonicalString(alterOption)))
180274
return false, nil
181275
}
182276
if findIndexCoveringColumn(opt.Name.Name.String()) != nil {
183277
// not supported if the column is part of an index
278+
log.Info("ALTER is not eligible for INSTANT DDL because the column is part of an index", slog.String("alter", sqlparser.CanonicalString(alterOption)))
184279
return false, nil
185280
}
186281
if isGenerated, _ := IsGeneratedColumn(col); isGenerated {
187282
// supported by all 8.0 versions
188283
// Note: according to the docs dropping a STORED generated column is not INSTANT-able,
189284
// but in practice this is supported. This is why we don't test for STORED here, like
190285
// we did for `AddColumns`.
191-
return capableOf(capabilities.InstantAddDropVirtualColumnFlavorCapability)
286+
capable, err := capableOf(capabilities.InstantAddDropVirtualColumnFlavorCapability)
287+
if !capable {
288+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support dropping virtual generated columns with INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
289+
}
290+
if err != nil {
291+
log.Error("Error while checking MySQL server INSTANT DDL capability for ALTER", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.Any("error", err))
292+
}
293+
return capable, err
192294
}
193295
return capableOf(capabilities.InstantAddDropColumnFlavorCapability)
194296
case *sqlparser.ChangeColumn:
195297
if opt.First || opt.After != nil {
298+
log.Info("ALTER is not eligible for INSTANT DDL because CHANGE COLUMN does not support FIRST or AFTER clauses with INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
196299
return false, nil
197300
}
198301
// We do not support INSTANT for renaming a column (ALTER TABLE ...CHANGE) because:
@@ -201,26 +304,52 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
201304
// 3. The success of the operation depends on whether the column is referenced by a foreign key
202305
// in another table. Which is a bit too much to compute here.
203306
if opt.OldColumn.Name.String() != opt.NewColDefinition.Name.String() {
307+
log.Info("ALTER is not eligible for INSTANT DDL as the column name is being changed and that is not supported with INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
204308
return false, nil
205309
}
206310
if col := findColumn(opt.OldColumn.Name.String()); col != nil {
207-
return changeModifyColumnCapableOfInstantDDL(col, opt.NewColDefinition)
311+
capable, err := changeModifyColumnCapableOfInstantDDL(col, opt.NewColDefinition)
312+
if err != nil {
313+
log.Error("Error while checking ALTER for INSTANT DDL capability", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.Any("error", err))
314+
}
315+
return capable, err
208316
}
317+
log.Info("ALTER is not eligible for INSTANT DDL because the column was not found", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("column", opt.OldColumn.Name.String()))
209318
return false, nil
210319
case *sqlparser.ModifyColumn:
211320
if opt.First || opt.After != nil {
321+
log.Info("ALTER is not eligible for INSTANT DDL as the MODIFY COLUMN clause does not support FIRST or AFTER clauses with INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
212322
return false, nil
213323
}
214324
if col := findColumn(opt.NewColDefinition.Name.String()); col != nil {
215-
return changeModifyColumnCapableOfInstantDDL(col, opt.NewColDefinition)
325+
capable, err := changeModifyColumnCapableOfInstantDDL(col, opt.NewColDefinition)
326+
if err != nil {
327+
log.Error("Error while checking ALTER for INSTANT DDL capability", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.Any("error", err))
328+
}
329+
return capable, err
216330
}
331+
log.Info("ALTER is not eligible for INSTANT DDL because the column was not found", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.String("column", opt.NewColDefinition.Name.String()))
217332
return false, nil
218333
case *sqlparser.AlterColumn:
219334
if opt.DropDefault || opt.DefaultLiteral || opt.DefaultVal != nil {
220-
return capableOf(capabilities.InstantChangeColumnDefaultFlavorCapability)
335+
capable, err := capableOf(capabilities.InstantChangeColumnDefaultFlavorCapability)
336+
if !capable {
337+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support changing column defaults with INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
338+
}
339+
if err != nil {
340+
log.Error("Error while checking MySQL server INSTANT DDL capability for ALTER", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.Any("error", err))
341+
}
342+
return capable, err
221343
}
222344
if opt.Invisible != nil {
223-
return capableOf(capabilities.InstantChangeColumnVisibilityCapability)
345+
capable, err := capableOf(capabilities.InstantChangeColumnVisibilityCapability)
346+
if !capable {
347+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support changing column visibility with INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterOption)))
348+
}
349+
if err != nil {
350+
log.Error("Error while checking MySQL server INSTANT DDL capability for ALTER", slog.String("alter", sqlparser.CanonicalString(alterOption)), slog.Any("error", err))
351+
}
352+
return capable, err
224353
}
225354
return false, nil
226355
case sqlparser.AlgorithmValue:
@@ -243,10 +372,12 @@ func AlterTableCapableOfInstantDDL(alterTable *sqlparser.AlterTable, createTable
243372
return false, err
244373
}
245374
if !capable {
375+
log.Info("ALTER is not eligible for INSTANT DDL because the MySQL server version does not support INSTANT DDL", slog.String("alter", sqlparser.CanonicalString(alterTable)))
246376
return false, nil
247377
}
248378
if alterTable.PartitionOption != nil || alterTable.PartitionSpec != nil {
249379
// no INSTANT for partitions
380+
log.Info("ALTER is not eligible for INSTANT DDL because the table is partitioned", slog.String("alter", sqlparser.CanonicalString(alterTable)))
250381
return false, nil
251382
}
252383
// For the ALTER statement to qualify for ALGORITHM=INSTANT, all alter options must each qualify.
@@ -267,6 +398,7 @@ func AlterTableCapableOfInstantDDL(alterTable *sqlparser.AlterTable, createTable
267398
if len(createTable.TableSpec.Columns)+numAddedColumns > maxColumnsForInstantAddColumn {
268399
// Per MySQL docs:
269400
// > The maximum number of columns in the internal representation of the table cannot exceed 1022 after column addition with the INSTANT algorithm
401+
log.Info("ALTER is not eligible for INSTANT DDL because the table would exceed the maximum number of columns", slog.String("alter", sqlparser.CanonicalString(alterTable)))
270402
return false, nil
271403
}
272404
return true, nil

0 commit comments

Comments
 (0)