Skip to content

Commit fae9f64

Browse files
committed
fix
- Tighten reAutoRandom regex to only match TiDB /*T![auto_rand] comments - Add checkUnsupportedChanges for AUTO_RANDOM modification/removal detection - Refactor ColumnChange to detect AUTO_RANDOM removal (previously ignored) - Replace magic numbers with AutoRandom constants - Improve error messages in setCollate, setAutoRandom, setAutoIncrement - Add defensive nil checks in priority() and patchColumn() - Fix bytesToBitLiteral to handle >8 byte inputs - Rename shadowed variable in flat()
1 parent 61944de commit fae9f64

7 files changed

Lines changed: 327 additions & 48 deletions

File tree

hoge.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
aaa

internal/integration/tidb_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,9 @@ func TestTiDB_AutoRandom(t *testing.T) {
10741074
_, err := t.db.Exec("CREATE TABLE ar_nodrift (id bigint NOT NULL AUTO_RANDOM(5), PRIMARY KEY (id) CLUSTERED)")
10751075
require.NoError(t, err)
10761076
tbl := t.loadTable("ar_nodrift")
1077-
ensureNoChange(t, tbl)
1077+
// Compare with freshly loaded table to ensure no drift.
1078+
changes := t.diff(t.loadTable("ar_nodrift"), tbl)
1079+
require.Emptyf(t, changes, "expected no changes, got: %#v", changes)
10781080
})
10791081
})
10801082
t.Run("HCLRoundTrip", func(t *testing.T) {

sql/mysql/migrate_oss.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,8 @@ func (s *state) column(b *sqlx.Builder, t *schema.Table, c *schema.Column) error
581581
// ShardBits=0 is a zero-value placeholder; skip SQL generation.
582582
break
583583
}
584-
if a.RangeBits > 0 && a.RangeBits != 64 {
584+
// Only include range bits if explicitly set and not the default (64).
585+
if a.RangeBits > 0 && a.RangeBits != AutoRandomRangeBitsMax {
585586
b.P(fmt.Sprintf("AUTO_RANDOM(%d, %d)", a.ShardBits, a.RangeBits))
586587
} else {
587588
b.P(fmt.Sprintf("AUTO_RANDOM(%d)", a.ShardBits))

sql/mysql/migrate_oss_test.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,7 @@ func TestPlanChanges(t *testing.T) {
12981298
},
12991299
},
13001300
},
1301-
// ALTER TABLE: change AUTO_RANDOM shard bits.
1301+
// ALTER TABLE: change AUTO_RANDOM shard bits (not supported by TiDB).
13021302
{
13031303
version: "5.7.25-TiDB-v6.1.0",
13041304
changes: []schema.Change{
@@ -1318,16 +1318,7 @@ func TestPlanChanges(t *testing.T) {
13181318
},
13191319
},
13201320
},
1321-
wantPlan: &migrate.Plan{
1322-
Reversible: true,
1323-
Transactional: false,
1324-
Changes: []*migrate.Change{
1325-
{
1326-
Cmd: "ALTER TABLE `users` MODIFY COLUMN `id` bigint NOT NULL AUTO_RANDOM(10)",
1327-
Reverse: "ALTER TABLE `users` MODIFY COLUMN `id` bigint NOT NULL AUTO_RANDOM(5)",
1328-
},
1329-
},
1330-
},
1321+
wantErr: true, // TiDB does not support changing AUTO_RANDOM shard bits.
13311322
},
13321323
}
13331324
for i, tt := range tests {

sql/mysql/sqlspec_oss.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -286,22 +286,22 @@ func convertColumn(spec *sqlspec.Column, _ *schema.Table) (*schema.Column, error
286286
if err != nil {
287287
return nil, err
288288
}
289-
if v < 1 || v > 15 {
290-
return nil, fmt.Errorf("auto_random shard bits for column %q must be between 1 and 15, got %d", c.Name, v)
289+
if v < AutoRandomShardBitsMin || v > AutoRandomShardBitsMax {
290+
return nil, fmt.Errorf("auto_random shard bits for column %q must be between %d and %d, got %d", c.Name, AutoRandomShardBitsMin, AutoRandomShardBitsMax, v)
291291
}
292292
ar := &AutoRandom{ShardBits: v}
293293
if rangeAttr, ok := spec.Attr("auto_random_range"); ok {
294294
r, err := rangeAttr.Int()
295295
if err != nil {
296296
return nil, err
297297
}
298-
if r < 32 || r > 64 {
299-
return nil, fmt.Errorf("auto_random_range for column %q must be between 32 and 64, got %d", c.Name, r)
298+
if r < AutoRandomRangeBitsMin || r > AutoRandomRangeBitsMax {
299+
return nil, fmt.Errorf("auto_random_range for column %q must be between %d and %d, got %d", c.Name, AutoRandomRangeBitsMin, AutoRandomRangeBitsMax, r)
300300
}
301301
// Normalize the default range (64) to 0 so that diffs between
302302
// the inspected state (which also normalizes 64→0) and the
303303
// desired state from HCL are consistent.
304-
if r != 64 {
304+
if r != AutoRandomRangeBitsMax {
305305
ar.RangeBits = r
306306
}
307307
}
@@ -436,7 +436,7 @@ func columnSpec(c *schema.Column, t *schema.Table) (*sqlspec.Column, error) {
436436
}
437437
if ar := (&AutoRandom{}); sqlx.Has(c.Attrs, ar) && ar.ShardBits > 0 {
438438
spec.Extra.Attrs = append(spec.Extra.Attrs, schemahcl.IntAttr("auto_random", ar.ShardBits))
439-
if ar.RangeBits > 0 && ar.RangeBits != 64 {
439+
if ar.RangeBits > 0 && ar.RangeBits != AutoRandomRangeBitsMax {
440440
spec.Extra.Attrs = append(spec.Extra.Attrs, schemahcl.IntAttr("auto_random_range", ar.RangeBits))
441441
}
442442
}

sql/mysql/tidb.go

Lines changed: 146 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@ import (
1818
"ariga.io/atlas/sql/schema"
1919
)
2020

21+
// TiDB AUTO_RANDOM constraints.
22+
const (
23+
// AutoRandomShardBitsMin is the minimum value for AUTO_RANDOM shard bits.
24+
AutoRandomShardBitsMin = 1
25+
// AutoRandomShardBitsMax is the maximum value for AUTO_RANDOM shard bits.
26+
AutoRandomShardBitsMax = 15
27+
// AutoRandomRangeBitsMin is the minimum value for AUTO_RANDOM range bits.
28+
AutoRandomRangeBitsMin = 32
29+
// AutoRandomRangeBitsMax is the maximum/default value for AUTO_RANDOM range bits.
30+
AutoRandomRangeBitsMax = 64
31+
)
32+
2133
type (
2234
// tplanApply decorates MySQL planApply.
2335
tplanApply struct{ planApply }
@@ -36,10 +48,18 @@ type (
3648
func priority(change schema.Change) int {
3749
switch c := change.(type) {
3850
case *schema.ModifyTable:
39-
// each modifyTable should have a single change since we apply `flat` before we sort.
51+
// Each ModifyTable should have a single change since we apply `flat` before we sort.
52+
// Defensive check: if Changes is empty, return default priority.
53+
if len(c.Changes) == 0 {
54+
return 4
55+
}
4056
return priority(c.Changes[0])
4157
case *schema.ModifySchema:
42-
// each modifyTable should have a single change since we apply `flat` before we sort.
58+
// Each ModifySchema should have a single change since we apply `flat` before we sort.
59+
// Defensive check: if Changes is empty, return default priority.
60+
if len(c.Changes) == 0 {
61+
return 4
62+
}
4363
return priority(c.Changes[0])
4464
case *schema.AddColumn:
4565
return 1
@@ -56,28 +76,28 @@ func priority(change schema.Change) int {
5676
// with multiple AddColumn inside it). Note that, the only "changes" that include sub-changes are
5777
// `ModifyTable` and `ModifySchema`.
5878
func flat(changes []schema.Change) []schema.Change {
59-
var flat []schema.Change
79+
var result []schema.Change
6080
for _, change := range changes {
6181
switch m := change.(type) {
6282
case *schema.ModifyTable:
6383
for _, c := range m.Changes {
64-
flat = append(flat, &schema.ModifyTable{
84+
result = append(result, &schema.ModifyTable{
6585
T: m.T,
6686
Changes: []schema.Change{c},
6787
})
6888
}
6989
case *schema.ModifySchema:
7090
for _, c := range m.Changes {
71-
flat = append(flat, &schema.ModifySchema{
91+
result = append(result, &schema.ModifySchema{
7292
S: m.S,
7393
Changes: []schema.Change{c},
7494
})
7595
}
7696
default:
77-
flat = append(flat, change)
97+
result = append(result, change)
7898
}
7999
}
80-
return flat
100+
return result
81101
}
82102

83103
// PlanChanges returns a migration plan for the given schema changes.
@@ -86,6 +106,10 @@ func (p *tplanApply) PlanChanges(ctx context.Context, name string, changes []sch
86106
if err != nil {
87107
return nil, err
88108
}
109+
// Check for unsupported TiDB-specific changes before planning.
110+
if err := checkUnsupportedChanges(planned); err != nil {
111+
return nil, err
112+
}
89113
planned = flat(planned)
90114
sort.SliceStable(planned, func(i, j int) bool {
91115
return priority(planned[i]) < priority(planned[j])
@@ -114,6 +138,79 @@ func (p *tplanApply) PlanChanges(ctx context.Context, name string, changes []sch
114138
return &s.Plan, nil
115139
}
116140

141+
// checkUnsupportedChanges checks for TiDB-specific changes that cannot be
142+
// applied via ALTER TABLE and returns an error with guidance.
143+
func checkUnsupportedChanges(changes []schema.Change) error {
144+
for _, c := range changes {
145+
switch c := c.(type) {
146+
case *schema.ModifyTable:
147+
for _, tc := range c.Changes {
148+
if err := checkUnsupportedTableChange(c.T.Name, tc); err != nil {
149+
return err
150+
}
151+
}
152+
}
153+
}
154+
return nil
155+
}
156+
157+
func checkUnsupportedTableChange(tableName string, c schema.Change) error {
158+
switch c := c.(type) {
159+
case *schema.ModifyColumn:
160+
// Check for AUTO_RANDOM modifications (not additions).
161+
if c.From == nil || c.To == nil {
162+
break
163+
}
164+
var fromAR, toAR AutoRandom
165+
fromHas, toHas := sqlx.Has(c.From.Attrs, &fromAR), sqlx.Has(c.To.Attrs, &toAR)
166+
// Adding AUTO_RANDOM requires BIGINT column type.
167+
if !fromHas && toHas {
168+
if !isBigIntColumn(c.To) {
169+
return fmt.Errorf(
170+
"cannot add AUTO_RANDOM to column %q in table %q: "+
171+
"AUTO_RANDOM is only supported on BIGINT columns",
172+
c.To.Name, tableName,
173+
)
174+
}
175+
}
176+
// Modifying existing AUTO_RANDOM parameters is not supported.
177+
if fromHas && toHas && (fromAR.ShardBits != toAR.ShardBits || fromAR.RangeBits != toAR.RangeBits) {
178+
return fmt.Errorf(
179+
"cannot modify AUTO_RANDOM for column %q in table %q: "+
180+
"TiDB does not support changing AUTO_RANDOM shard bits or range bits after column creation; "+
181+
"you must recreate the table to change this setting",
182+
c.From.Name, tableName,
183+
)
184+
}
185+
// Removing AUTO_RANDOM is not supported.
186+
if fromHas && !toHas {
187+
return fmt.Errorf(
188+
"cannot remove AUTO_RANDOM from column %q in table %q: "+
189+
"TiDB does not support removing AUTO_RANDOM after it has been set; "+
190+
"you must recreate the table to remove this setting",
191+
c.From.Name, tableName,
192+
)
193+
}
194+
}
195+
return nil
196+
}
197+
198+
// isBigIntColumn checks if the column is a BIGINT type.
199+
// TiDB's AUTO_RANDOM only supports BIGINT columns (signed or unsigned).
200+
// The IntegerType.T field contains the base type name (e.g., "bigint"),
201+
// while the Unsigned field indicates if it's unsigned.
202+
func isBigIntColumn(c *schema.Column) bool {
203+
if c == nil || c.Type == nil || c.Type.Type == nil {
204+
return false
205+
}
206+
it, ok := c.Type.Type.(*schema.IntegerType)
207+
if !ok {
208+
return false
209+
}
210+
// Case-insensitive comparison to handle variations like "BIGINT", "bigint", etc.
211+
return strings.EqualFold(it.T, "bigint")
212+
}
213+
117214
func (p *tplanApply) ApplyChanges(ctx context.Context, changes []schema.Change, opts ...migrate.PlanOption) error {
118215
return sqlx.ApplyChanges(ctx, changes, p, opts...)
119216
}
@@ -127,15 +224,20 @@ func (d *tdiff) ColumnChange(fromT *schema.Table, from, to *schema.Column, opts
127224
}
128225
var fromAR, toAR AutoRandom
129226
fromHas, toHas := sqlx.Has(from.Attrs, &fromAR), sqlx.Has(to.Attrs, &toAR)
227+
// Check if AUTO_RANDOM attribute changed.
228+
autoRandomChanged := false
130229
switch {
131230
case !fromHas && toHas:
132231
// AUTO_RANDOM was added.
232+
autoRandomChanged = true
133233
case fromHas && toHas && (fromAR.ShardBits != toAR.ShardBits || fromAR.RangeBits != toAR.RangeBits):
134-
// AUTO_RANDOM parameters changed.
135-
default:
136-
// No AUTO_RANDOM change detected. Note that we intentionally
137-
// skip the case where AUTO_RANDOM is removed (fromHas && !toHas),
138-
// because TiDB does not support dropping AUTO_RANDOM from a column.
234+
// AUTO_RANDOM parameters changed (not supported by TiDB, will be caught by checkUnsupportedChanges).
235+
autoRandomChanged = true
236+
case fromHas && !toHas:
237+
// AUTO_RANDOM was removed (not supported by TiDB, will be caught by checkUnsupportedChanges).
238+
autoRandomChanged = true
239+
}
240+
if !autoRandomChanged {
139241
return change, nil
140242
}
141243
if change == sqlx.NoChange {
@@ -197,6 +299,9 @@ func (i *tinspect) patchSchema(ctx context.Context, s *schema.Schema) (*schema.S
197299
}
198300

199301
func (i *tinspect) patchColumn(_ context.Context, c *schema.Column) {
302+
if c == nil || c.Type == nil || c.Type.Type == nil {
303+
return
304+
}
200305
_, ok := c.Type.Type.(*BitType)
201306
if !ok {
202307
return
@@ -211,11 +316,14 @@ func (i *tinspect) patchColumn(_ context.Context, c *schema.Column) {
211316
// e.g. []byte{4} -> b'100', []byte{2,1} -> b'1000000001'.
212317
// See: https://github.com/pingcap/tidb/issues/32655.
213318
func bytesToBitLiteral(b []byte) string {
214-
bytes := make([]byte, 8)
215-
for i := 0; i < len(b); i++ {
216-
bytes[8-len(b)+i] = b[i]
319+
// MySQL BIT type supports up to 64 bits (8 bytes).
320+
// If input exceeds 8 bytes, truncate to the last 8 bytes.
321+
if len(b) > 8 {
322+
b = b[len(b)-8:]
217323
}
218-
val := binary.BigEndian.Uint64(bytes)
324+
buf := make([]byte, 8)
325+
copy(buf[8-len(b):], b)
326+
val := binary.BigEndian.Uint64(buf)
219327
return fmt.Sprintf("b'%b'", val)
220328
}
221329

@@ -226,11 +334,13 @@ var reColl = regexp.MustCompile(`(?i)CHARSET\s*=\s*(\w+)\s*COLLATE\s*=\s*(\w+)`)
226334
func (i *tinspect) setCollate(t *schema.Table) error {
227335
var c CreateStmt
228336
if !sqlx.Has(t.Attrs, &c) {
229-
return fmt.Errorf("missing CREATE TABLE statement in attributes for %q", t.Name)
337+
return fmt.Errorf("mysql: missing CREATE TABLE statement in attributes for table %q; "+
338+
"this may indicate an internal error during schema inspection", t.Name)
230339
}
231340
matches := reColl.FindStringSubmatch(c.S)
232341
if len(matches) != 3 {
233-
return fmt.Errorf("missing COLLATE and/or CHARSET information on CREATE TABLE statement for %q", t.Name)
342+
return fmt.Errorf("mysql: could not extract CHARSET and COLLATE from CREATE TABLE statement for table %q; "+
343+
"expected format 'CHARSET=... COLLATE=...' but got: %s", t.Name, c.S)
234344
}
235345
t.SetCharset(matches[1])
236346
t.SetCollation(matches[2])
@@ -241,10 +351,20 @@ func (i *tinspect) setCollate(t *schema.Table) error {
241351
// definition and captures the column name. TiDB wraps this in a special comment:
242352
//
243353
// `id` bigint NOT NULL /*T![auto_rand] AUTO_RANDOM(5) */
354+
// `id` bigint NOT NULL /*T![auto_rand] AUTO_RANDOM(5, 64) */
355+
//
356+
// Pattern breakdown:
357+
// - `([^`]+)` captures the column name (any chars except backtick)
358+
// - [^`]*/\*T!\[auto_rand\] matches the TiDB-specific comment marker
359+
// This ensures we only match AUTO_RANDOM in TiDB comments, not in SQL comments
360+
// - \s*AUTO_RANDOM\((\d+) captures the shard bits (required, 1-15)
361+
// - (?:\s*,\s*(\d+))? optionally captures range bits (32-64, default 64)
244362
//
245-
// The [^`]* ensures we match the column name closest to AUTO_RANDOM (not the table name).
246363
// Group 1: column name, Group 2: shard bits, Group 3: optional range bits.
247-
var reAutoRandom = regexp.MustCompile("`([^`]+)`[^`]*AUTO_RANDOM\\((\\d+)(?:\\s*,\\s*(\\d+))?\\)")
364+
//
365+
// Note: Column names with escaped backticks are extremely rare
366+
// and not supported by this pattern.
367+
var reAutoRandom = regexp.MustCompile("`([^`]+)`[^`]*/\\*T!\\[auto_rand\\]\\s*AUTO_RANDOM\\((\\d+)(?:\\s*,\\s*(\\d+))?\\)")
248368

249369
// setAutoRandom extracts the shard and range bits from CREATE TABLE statement.
250370
// TiDB allows at most one AUTO_RANDOM column per table. Unlike older TiDB versions
@@ -267,18 +387,18 @@ func (i *tinspect) setAutoRandom(t *schema.Table) error {
267387
}
268388
shard, err := strconv.Atoi(matches[2])
269389
if err != nil {
270-
return err
390+
return fmt.Errorf("parsing AUTO_RANDOM shard bits for column %q in table %q: %w", colName, t.Name, err)
271391
}
272392
ar := &AutoRandom{ShardBits: shard}
273393
if matches[3] != "" {
274394
rangeBits, err := strconv.Atoi(matches[3])
275395
if err != nil {
276-
return err
396+
return fmt.Errorf("parsing AUTO_RANDOM range bits for column %q in table %q: %w", colName, t.Name, err)
277397
}
278398
// Normalize the default range (64) to 0 so that HCL round-trips
279399
// are lossless: columnSpec omits auto_random_range when it equals
280400
// the default, and convertColumn reads the absence as 0.
281-
if rangeBits != 64 {
401+
if rangeBits != AutoRandomRangeBitsMax {
282402
ar.RangeBits = rangeBits
283403
}
284404
}
@@ -288,22 +408,22 @@ func (i *tinspect) setAutoRandom(t *schema.Table) error {
288408

289409
// setAutoIncrement extracts the actual AUTO_INCREMENT value from the CREATE TABLE statement.
290410
func (i *tinspect) setAutoIncrement(t *schema.Table) error {
291-
// patch only it is set (set falsely to '1' due to this bug:https://github.com/pingcap/tidb/issues/24702).
411+
// patch only it is set (set falsely to '1' due to this bug: https://github.com/pingcap/tidb/issues/24702).
292412
ai := &AutoIncrement{}
293413
if !sqlx.Has(t.Attrs, ai) {
294414
return nil
295415
}
296416
var c CreateStmt
297417
if !sqlx.Has(t.Attrs, &c) {
298-
return fmt.Errorf("missing CREATE TABLE statement in attributes for %q", t.Name)
418+
return fmt.Errorf("mysql: missing CREATE TABLE statement in attributes for table %q", t.Name)
299419
}
300420
matches := reAutoinc.FindStringSubmatch(c.S)
301421
if len(matches) != 2 {
302422
return nil
303423
}
304424
v, err := strconv.ParseInt(matches[1], 10, 64)
305425
if err != nil {
306-
return err
426+
return fmt.Errorf("parsing AUTO_INCREMENT for table %q: %w", t.Name, err)
307427
}
308428
ai.V = v
309429
schema.ReplaceOrAppend(&t.Attrs, ai)

0 commit comments

Comments
 (0)