From c9917fac3b86e560ccb8e2d7dede11d5a761e6b8 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 12 May 2026 07:35:51 -0600 Subject: [PATCH 1/2] lint: evaluate schema linters against post-state to fix conversion false positives has_timestamp, has_float, and zero_date previously walked existingTables unconditionally, so a declarative diff or ALTER MODIFY/CHANGE/DROP that fixed a problematic column still produced a warning for the pre-state. Extracts the post-state helper that type_pedantic already used into a shared utility (PostState, PreStateColumns) and reworks the three linters to iterate the after-image. has_timestamp preserves its Warning/Error severity split by consulting the pre-state for untouched columns. --- pkg/lint/lint_has_float.go | 71 +++--- pkg/lint/lint_has_float_test.go | 25 +-- pkg/lint/lint_has_timestamp.go | 170 ++++----------- pkg/lint/lint_has_timestamp_test.go | 135 ++++-------- pkg/lint/lint_type_pedantic.go | 225 +------------------ pkg/lint/lint_zero_date.go | 26 +-- pkg/lint/post_state.go | 326 ++++++++++++++++++++++++++++ 7 files changed, 460 insertions(+), 518 deletions(-) create mode 100644 pkg/lint/post_state.go diff --git a/pkg/lint/lint_has_float.go b/pkg/lint/lint_has_float.go index 538b1c76..54065801 100644 --- a/pkg/lint/lint_has_float.go +++ b/pkg/lint/lint_has_float.go @@ -2,9 +2,9 @@ package lint import ( "fmt" + "strings" "github.com/block/spirit/pkg/statement" - "github.com/pingcap/tidb/pkg/parser/ast" "github.com/pingcap/tidb/pkg/parser/mysql" ) @@ -26,58 +26,41 @@ func (l *HasFloatLinter) Description() string { return "Detects usage of FLOAT or DOUBLE data types in table definitions" } +// Lint operates on a post-state view of the schema, so a column being +// converted away from FLOAT/DOUBLE in an ALTER doesn't generate a false +// positive. The violation message distinguishes pre-existing columns from +// columns added/modified by the changeset to preserve actionability. func (l *HasFloatLinter) Lint(existingTables []*statement.CreateTable, changes []*statement.AbstractStatement) (violations []Violation) { - // Loop over all table definitions - for table := range CreateTableStatements(existingTables, changes) { - violations = append(violations, l.checkCreateTable(table)...) - } + addedOrModified := columnsAddedOrModifiedInChanges(changes) + createdInChanges := newTablesInChanges(changes) - // Loop over ALTER TABLE statements that add/modify FLOAT/DOUBLE columns - for _, change := range changes { - alter, ok := change.AsAlterTable() - if !ok { - continue - } - for _, spec := range alter.Specs { - var message string - switch spec.Tp { //nolint:exhaustive - case ast.AlterTableAddColumns: - message = "New column %q in table %q uses floating-point data type" - case ast.AlterTableModifyColumn, ast.AlterTableChangeColumn: - message = "Column %q in table %q modified to use floating-point data type" - default: + for _, ct := range PostState(existingTables, changes) { + tKey := strings.ToLower(ct.TableName) + for _, col := range ct.Columns { + if col.Raw == nil || col.Raw.Tp == nil { continue } - - // Check all columns in this spec for FLOAT/DOUBLE - for _, col := range spec.NewColumns { - if col.Tp.GetType() == mysql.TypeFloat || col.Tp.GetType() == mysql.TypeDouble { - violations = append(violations, Violation{ - Linter: l, - Location: &Location{ - Table: change.Table, - Column: &col.Name.Name.O, - }, - Message: fmt.Sprintf(message, col.Name.Name.O, change.Table), - Severity: SeverityWarning, - }) - } + tp := col.Raw.Tp.GetType() + if tp != mysql.TypeFloat && tp != mysql.TypeDouble { + continue + } + colName := col.Name + colKey := strings.ToLower(colName) + modified := !createdInChanges[tKey] && addedOrModified[tKey][colKey] + var message string + switch { + case modified: + message = fmt.Sprintf("Column %q in table %q modified to use floating-point data type", colName, ct.TableName) + default: + message = fmt.Sprintf("Column %q in table %q uses %s data type", colName, ct.TableName, col.Raw.Tp.String()) } - } - } - return violations -} - -func (l *HasFloatLinter) checkCreateTable(table *statement.CreateTable) (violations []Violation) { - for _, col := range table.Columns { - if col.Raw.Tp.GetType() == mysql.TypeFloat || col.Raw.Tp.GetType() == mysql.TypeDouble { violations = append(violations, Violation{ Linter: l, Location: &Location{ - Table: table.TableName, - Column: &col.Name, + Table: ct.TableName, + Column: &colName, }, - Message: fmt.Sprintf("Column %q in table %q uses %s data type", col.Name, table.TableName, col.Type), + Message: message, Severity: SeverityWarning, }) } diff --git a/pkg/lint/lint_has_float_test.go b/pkg/lint/lint_has_float_test.go index 44ed9234..b0bc78c9 100644 --- a/pkg/lint/lint_has_float_test.go +++ b/pkg/lint/lint_has_float_test.go @@ -1177,20 +1177,18 @@ func TestHasFloatLinter_UltraComplexScenario(t *testing.T) { linter := &HasFloatLinter{} violations := linter.Lint([]*statement.CreateTable{existing1, existing2}, newTableStmts) - // Should detect: - // 1. existing1.value (FLOAT in existing table) - // 2. new_table.score (DOUBLE in new table) - // 3. existing2.pressure (FLOAT via ADD) - // 4. existing2.temperature (DOUBLE via MODIFY) - // 5. existing1.measurement (FLOAT via CHANGE) - require.Len(t, violations, 5) - - // Verify we have violations from all sources - var foundExisting1, foundNewTable, foundAdd, foundModify, foundChange bool + // Post-state contains: + // 1. new_table.score — DOUBLE in new CREATE TABLE + // 2. existing2.pressure — FLOAT via ADD + // 3. existing2.temperature — DOUBLE via MODIFY (was INT) + // 4. existing1.measurement — FLOAT via CHANGE (was existing1.value, renamed) + // + // existing1.value is renamed away by CHANGE COLUMN, so it no longer + // appears in the post-state — only the renamed `measurement` does. + require.Len(t, violations, 4) + + var foundNewTable, foundAdd, foundModify, foundChange bool for _, v := range violations { - if v.Location.Table == "existing1" && *v.Location.Column == "value" { - foundExisting1 = true - } if v.Location.Table == "new_table" && *v.Location.Column == "score" { foundNewTable = true } @@ -1204,7 +1202,6 @@ func TestHasFloatLinter_UltraComplexScenario(t *testing.T) { foundChange = true } } - require.True(t, foundExisting1, "Should find violation in existing1.value") require.True(t, foundNewTable, "Should find violation in new_table.score") require.True(t, foundAdd, "Should find violation in ADD COLUMN") require.True(t, foundModify, "Should find violation in MODIFY COLUMN") diff --git a/pkg/lint/lint_has_timestamp.go b/pkg/lint/lint_has_timestamp.go index 7ecf540d..b6eb66b5 100644 --- a/pkg/lint/lint_has_timestamp.go +++ b/pkg/lint/lint_has_timestamp.go @@ -2,9 +2,9 @@ package lint import ( "fmt" + "strings" "github.com/block/spirit/pkg/statement" - "github.com/pingcap/tidb/pkg/parser/ast" "github.com/pingcap/tidb/pkg/parser/mysql" ) @@ -26,142 +26,64 @@ func (l *HasTimestampLinter) Description() string { return "Detects usage of TIMESTAMP data type which overflows on 2038-01-19" } +// Lint operates on a post-state view of the schema. For each TIMESTAMP column +// in the post-state, the severity is: +// +// - Warning, if the column existed in the pre-state with TIMESTAMP type and +// no incoming change touches it (legacy schema — don't boil the ocean). +// - Error, if the column is newly added in this changeset, or modified to +// TIMESTAMP, or appears in a CREATE TABLE in the changeset. +// +// Columns that were TIMESTAMP in the pre-state but are being dropped or +// converted to a different type in the changeset don't appear in the post-state +// and therefore produce no violation — which is exactly the desired behavior +// for migrations that fix legacy TIMESTAMP columns. func (l *HasTimestampLinter) Lint(existingTables []*statement.CreateTable, changes []*statement.AbstractStatement) (violations []Violation) { - // Existing tables with TIMESTAMP get Warning — don't boil the ocean on legacy schemas. - for _, ct := range existingTables { - for _, col := range ct.Columns { - if col.Raw.Tp.GetType() == mysql.TypeTimestamp { - violations = append(violations, Violation{ - Linter: l, - Location: &Location{ - Table: ct.TableName, - Column: &col.Name, - }, - Message: fmt.Sprintf("Column %q uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.", col.Name), - Severity: SeverityWarning, - }) - } - } - } + post := PostState(existingTables, changes) + pre := PreStateColumns(existingTables) + createdInChanges := newTablesInChanges(changes) + addedOrModifiedCols := columnsAddedOrModifiedInChanges(changes) - // CREATE TABLE statements in changes get Error — don't introduce new TIMESTAMP. - for _, change := range changes { - if change.IsCreateTable() { - ct, err := change.ParseCreateTable() - if err != nil { + for _, ct := range post { + tKey := strings.ToLower(ct.TableName) + for _, col := range ct.Columns { + if col.Raw == nil || col.Raw.Tp == nil { continue } - for _, col := range ct.Columns { - if col.Raw.Tp.GetType() == mysql.TypeTimestamp { - violations = append(violations, Violation{ - Linter: l, - Location: &Location{ - Table: ct.TableName, - Column: &col.Name, - }, - Message: fmt.Sprintf("Column %q uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.", col.Name), - Severity: SeverityError, - }) - } + if col.Raw.Tp.GetType() != mysql.TypeTimestamp { + continue } + colName := col.Name + severity := l.severityFor(tKey, colName, pre, createdInChanges, addedOrModifiedCols) + violations = append(violations, Violation{ + Linter: l, + Location: &Location{ + Table: ct.TableName, + Column: &colName, + }, + Message: fmt.Sprintf("Column %q uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.", colName), + Severity: severity, + }) } } - // Check ALTER TABLE statements - violations = append(violations, l.checkAlterStatements(existingTables, changes)...) - return violations } -// checkAlterStatements checks ALTER TABLE statements for TIMESTAMP usage. -// Adding or modifying a column to TIMESTAMP is an Error. Modifying a table that -// already has TIMESTAMP columns (but not adding new ones) is a Warning — unless -// the ALTER is actively removing/converting those TIMESTAMP columns. -func (l *HasTimestampLinter) checkAlterStatements(existingTables []*statement.CreateTable, changes []*statement.AbstractStatement) (violations []Violation) { - // Build a map of existing tables for quick lookup - existingTableMap := make(map[string]*statement.CreateTable) - for _, table := range existingTables { - existingTableMap[table.GetTableName()] = table +func (l *HasTimestampLinter) severityFor(tableKey, colName string, pre map[string]map[string]*statement.Column, createdTables map[string]bool, addedOrModified map[string]map[string]bool) Severity { + colKey := strings.ToLower(colName) + if createdTables[tableKey] { + return SeverityError } - - for _, change := range changes { - alter, ok := change.AsAlterTable() - if !ok { - continue + if tableMods, ok := addedOrModified[tableKey]; ok { + if tableMods[colKey] { + return SeverityError } - - // Track whether the ALTER is adding/modifying columns to TIMESTAMP (Error), - // and collect the set of columns being dropped or converted away from TIMESTAMP. - addingTimestamp := false - columnsBeingFixed := make(map[string]bool) - - for _, spec := range alter.Specs { - switch spec.Tp { //nolint:exhaustive - case ast.AlterTableAddColumns: - for _, col := range spec.NewColumns { - if col.Tp.GetType() == mysql.TypeTimestamp { - addingTimestamp = true - violations = append(violations, Violation{ - Linter: l, - Location: &Location{ - Table: change.Table, - Column: &col.Name.Name.O, - }, - Message: fmt.Sprintf("Column %q uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.", col.Name.Name.O), - Severity: SeverityError, - }) - } - } - case ast.AlterTableModifyColumn, ast.AlterTableChangeColumn: - for _, col := range spec.NewColumns { - if col.Tp.GetType() == mysql.TypeTimestamp { - addingTimestamp = true - violations = append(violations, Violation{ - Linter: l, - Location: &Location{ - Table: change.Table, - Column: &col.Name.Name.O, - }, - Message: fmt.Sprintf("Column %q uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.", col.Name.Name.O), - Severity: SeverityError, - }) - } else { - // Column is being changed to a non-TIMESTAMP type — it's being fixed. - // For CHANGE COLUMN, OldColumnName is the original name. - if spec.OldColumnName != nil { - columnsBeingFixed[spec.OldColumnName.Name.O] = true - } - columnsBeingFixed[col.Name.Name.O] = true - } - } - case ast.AlterTableDropColumn: - if spec.OldColumnName != nil { - columnsBeingFixed[spec.OldColumnName.Name.O] = true - } - } - } - - // If the ALTER is not itself introducing TIMESTAMP columns, check whether - // the existing table already has TIMESTAMP columns (Warning). - // Exclude columns that are being dropped or converted in this ALTER. - if !addingTimestamp { - if existing, ok := existingTableMap[change.Table]; ok { - for _, col := range existing.Columns { - if col.Raw.Tp.GetType() == mysql.TypeTimestamp && !columnsBeingFixed[col.Name] { - violations = append(violations, Violation{ - Linter: l, - Location: &Location{ - Table: change.Table, - Column: &col.Name, - }, - Message: fmt.Sprintf("Column %q uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.", col.Name), - Severity: SeverityWarning, - }) - } - } - } + } + if preTbl, ok := pre[tableKey]; ok { + if preCol, ok := preTbl[colKey]; ok && preCol.Raw != nil && preCol.Raw.Tp != nil && preCol.Raw.Tp.GetType() == mysql.TypeTimestamp { + return SeverityWarning } } - - return violations + return SeverityError } diff --git a/pkg/lint/lint_has_timestamp_test.go b/pkg/lint/lint_has_timestamp_test.go index e2631137..2c22b45b 100644 --- a/pkg/lint/lint_has_timestamp_test.go +++ b/pkg/lint/lint_has_timestamp_test.go @@ -516,6 +516,33 @@ func TestHasTimestampLinter_AlterAddTimestampToTableWithExistingTimestamp(t *tes // --- ALTER TABLE that fixes TIMESTAMP (no false-positive warnings) --- +// Regression test for the false positive reported in the wild: when a +// declarative diff converts an existing TIMESTAMP column to DATETIME via +// MODIFY COLUMN, the linter should not emit a Warning for the (no-longer- +// existing) pre-state TIMESTAMP. +func TestHasTimestampLinter_DeclarativeDiff_TimestampToDatetime_NoFalsePositive(t *testing.T) { + existingSQL := `CREATE TABLE executions ( + id int PRIMARY KEY AUTO_INCREMENT, + task_id int NOT NULL, + logs longtext, + termination_message text, + created_at timestamp NULL DEFAULT CURRENT_TIMESTAMP, + KEY task_id (task_id), + KEY created_at (created_at) + )` + ct, err := statement.ParseCreateTable(existingSQL) + require.NoError(t, err) + + alterSQL := `ALTER TABLE executions MODIFY COLUMN created_at datetime(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3)` + stmts, err := statement.New(alterSQL) + require.NoError(t, err) + + linter := &HasTimestampLinter{} + violations := linter.Lint([]*statement.CreateTable{ct}, stmts) + + require.Empty(t, violations, "converting created_at TIMESTAMP → DATETIME should produce zero violations") +} + func TestHasTimestampLinter_AlterDropTimestampColumn(t *testing.T) { // Existing table has two TIMESTAMP columns existingSQL := `CREATE TABLE users ( @@ -534,40 +561,12 @@ func TestHasTimestampLinter_AlterDropTimestampColumn(t *testing.T) { linter := &HasTimestampLinter{} violations := linter.Lint([]*statement.CreateTable{ct}, stmts) - // Filter to just ALTER-path warnings (exclude existing table warnings) - var alterWarnings []Violation - for _, v := range violations { - if v.Severity == SeverityWarning { - alterWarnings = append(alterWarnings, v) - } - } - - // Should warn about created_at (still TIMESTAMP) but NOT about updated_at (being dropped) - // Existing table produces 2 warnings, ALTER path should only warn about created_at - // Total warnings: 2 from existing table + 1 from ALTER path = but updated_at is being fixed - // Let's check that we don't have warnings for updated_at from the ALTER path - // The existing table warnings are separate from ALTER warnings - // With the fix: existing table warns about both, ALTER only warns about created_at (not updated_at) - createdAtCount := 0 - updatedAtCount := 0 - for _, v := range alterWarnings { - if v.Location != nil && v.Location.Column != nil { - switch *v.Location.Column { - case "created_at": - createdAtCount++ - case "updated_at": - updatedAtCount++ - } - } - } - - // created_at appears in both existing table warning and ALTER warning - require.GreaterOrEqual(t, createdAtCount, 1) - // updated_at: existing table still warns (it doesn't know about the ALTER), - // but the ALTER path should NOT warn about it since it's being dropped. - // We expect exactly 1 warning for updated_at (from existing table only), - // not 2 (which would mean the ALTER path also warned about it). - require.Equal(t, 1, updatedAtCount, "updated_at should only be warned about from existing table, not from ALTER path") + // Post-state contains only created_at (still TIMESTAMP) — updated_at is + // dropped, so it doesn't generate any warning. We expect exactly one + // Warning for created_at. + require.Len(t, violations, 1) + require.Equal(t, SeverityWarning, violations[0].Severity) + require.Equal(t, "created_at", *violations[0].Location.Column) } func TestHasTimestampLinter_AlterModifyTimestampToDatetime(t *testing.T) { @@ -588,38 +587,12 @@ func TestHasTimestampLinter_AlterModifyTimestampToDatetime(t *testing.T) { linter := &HasTimestampLinter{} violations := linter.Lint([]*statement.CreateTable{ct}, stmts) - // The ALTER is fixing created_at, so the ALTER path should only warn about updated_at - var alterPathWarnings []Violation - for _, v := range violations { - if v.Severity == SeverityWarning { - alterPathWarnings = append(alterPathWarnings, v) - } - } - - // Existing table warns about both (2 warnings) - // ALTER path warns about updated_at only (1 warning), created_at is being fixed - createdAtCount := 0 - updatedAtCount := 0 - for _, v := range alterPathWarnings { - if v.Location != nil && v.Location.Column != nil { - switch *v.Location.Column { - case "created_at": - createdAtCount++ - case "updated_at": - updatedAtCount++ - } - } - } - - // created_at: 1 from existing table (the ALTER path excludes it since it's being fixed) - require.Equal(t, 1, createdAtCount) - // updated_at: 1 from existing table + 1 from ALTER path = 2 - require.Equal(t, 2, updatedAtCount) - - // No errors — nothing is introducing TIMESTAMP - for _, v := range violations { - require.NotEqual(t, SeverityError, v.Severity) - } + // Post-state: created_at is DATETIME (fixed, no violation); updated_at + // remains TIMESTAMP. We expect exactly one Warning for updated_at and + // no Errors. + require.Len(t, violations, 1) + require.Equal(t, SeverityWarning, violations[0].Severity) + require.Equal(t, "updated_at", *violations[0].Location.Column) } func TestHasTimestampLinter_AlterChangeTimestampToDatetime(t *testing.T) { @@ -639,21 +612,8 @@ func TestHasTimestampLinter_AlterChangeTimestampToDatetime(t *testing.T) { linter := &HasTimestampLinter{} violations := linter.Lint([]*statement.CreateTable{ct}, stmts) - // Existing table warns about old_ts (1 warning) - // ALTER path should NOT warn about old_ts since it's being converted away - var errors []Violation - for _, v := range violations { - if v.Severity == SeverityError { - errors = append(errors, v) - } - } - require.Empty(t, errors, "converting TIMESTAMP to DATETIME should not produce errors") - - // The ALTER path should not add a warning for old_ts since it's being fixed - // Only the existing table warning for old_ts should remain - require.Len(t, violations, 1) - require.Equal(t, SeverityWarning, violations[0].Severity) - require.Equal(t, "old_ts", *violations[0].Location.Column) + // Post-state: old_ts is gone, new_dt is DATETIME — nothing to flag. + require.Empty(t, violations) } func TestHasTimestampLinter_AlterDropAllTimestampColumns(t *testing.T) { @@ -674,17 +634,8 @@ func TestHasTimestampLinter_AlterDropAllTimestampColumns(t *testing.T) { linter := &HasTimestampLinter{} violations := linter.Lint([]*statement.CreateTable{ct}, stmts) - // Existing table still warns (2 warnings) — it doesn't know about the ALTER - // ALTER path should NOT warn since both columns are being dropped - var alterPathViolations []Violation - for _, v := range violations { - // All violations should be Warning from existing table - require.Equal(t, SeverityWarning, v.Severity) - alterPathViolations = append(alterPathViolations, v) - } - - // Only 2 warnings from existing table, none from ALTER path - require.Len(t, alterPathViolations, 2) + // Post-state: both columns are dropped — no TIMESTAMP columns remain. + require.Empty(t, violations) } func TestHasTimestampLinter_AlterModifyTimestampToDatetimeNoExisting(t *testing.T) { diff --git a/pkg/lint/lint_type_pedantic.go b/pkg/lint/lint_type_pedantic.go index dfb44718..a6b2ecac 100644 --- a/pkg/lint/lint_type_pedantic.go +++ b/pkg/lint/lint_type_pedantic.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/block/spirit/pkg/statement" - "github.com/pingcap/tidb/pkg/parser/ast" ) func init() { @@ -145,7 +144,7 @@ func (l *TypePedanticLinter) Lint(existingTables []*statement.CreateTable, chang l.setDefaults() } - tables := l.buildPostState(existingTables, changes) + tables := PostState(existingTables, changes) tableByName := make(map[string]*statement.CreateTable, len(tables)) for _, t := range tables { tableByName[strings.ToLower(t.TableName)] = t @@ -161,228 +160,6 @@ func (l *TypePedanticLinter) Lint(existingTables []*statement.CreateTable, chang return violations } -// buildPostState returns a deterministic post-state view of the schema: -// existingTables augmented with CREATE TABLE statements from changes, and -// existing tables patched with ADD/DROP/MODIFY/CHANGE COLUMN and -// ADD/DROP INDEX (and DROP PRIMARY KEY) specs from ALTER TABLE statements. -// Other ALTER specs are ignored — they don't affect cross-table type -// consistency. -func (l *TypePedanticLinter) buildPostState(existing []*statement.CreateTable, changes []*statement.AbstractStatement) []*statement.CreateTable { - byName := make(map[string]*statement.CreateTable, len(existing)) - for _, t := range existing { - byName[strings.ToLower(t.TableName)] = t - } - - for _, change := range changes { - if change == nil { - continue - } - if change.IsCreateTable() { - ct, err := change.ParseCreateTable() - if err == nil && ct != nil { - byName[strings.ToLower(ct.TableName)] = ct - } - continue - } - at, ok := change.AsAlterTable() - if !ok { - continue - } - key := strings.ToLower(change.Table) - base, found := byName[key] - if !found { - continue - } - byName[key] = tpApplyAlter(base, at) - } - - out := make([]*statement.CreateTable, 0, len(byName)) - for _, t := range byName { - out = append(out, t) - } - sort.Slice(out, func(i, j int) bool { return out[i].TableName < out[j].TableName }) - return out -} - -// tpApplyAlter returns a shallow clone of t with the relevant subset of alter -// specs applied. The original table is not mutated. -// -// Note: GetIndexes() synthesizes PRIMARY KEY / UNIQUE entries from inline -// column-level flags (col.PrimaryKey, col.Unique), so the drop paths below -// must clear those flags in addition to scrubbing cloned.Indexes — otherwise -// the synthesis would resurrect the dropped index in the post-state. -func tpApplyAlter(t *statement.CreateTable, at *ast.AlterTableStmt) *statement.CreateTable { - cloned := *t - cloned.Columns = append(statement.Columns(nil), t.Columns...) - cloned.Indexes = append(statement.Indexes(nil), t.Indexes...) - - for _, spec := range at.Specs { - switch spec.Tp { //nolint:exhaustive - case ast.AlterTableAddColumns: - for _, colDef := range spec.NewColumns { - cloned.Columns = append(cloned.Columns, tpColumnFromAst(colDef)) - } - case ast.AlterTableDropColumn: - if spec.OldColumnName != nil { - cloned.Columns = tpRemoveColumn(cloned.Columns, spec.OldColumnName.Name.O) - } - case ast.AlterTableModifyColumn: - if len(spec.NewColumns) > 0 { - colDef := spec.NewColumns[0] - cloned.Columns = tpReplaceColumn(cloned.Columns, colDef.Name.Name.O, tpColumnFromAst(colDef)) - } - case ast.AlterTableChangeColumn: - if len(spec.NewColumns) > 0 && spec.OldColumnName != nil { - colDef := spec.NewColumns[0] - cloned.Columns = tpReplaceColumn(cloned.Columns, spec.OldColumnName.Name.O, tpColumnFromAst(colDef)) - } - case ast.AlterTableAddConstraint: - if spec.Constraint == nil { - continue - } - if idx, ok := tpIndexFromConstraint(spec.Constraint); ok { - cloned.Indexes = append(cloned.Indexes, idx) - // An inline-flag column whose constraint we just promoted to - // the table-level Indexes list could be double-counted. The - // duplication is harmless for the indexed-column set (a set, - // not a multiset), so we don't dedupe here. - } - case ast.AlterTableDropIndex: - before := len(cloned.Indexes) - cloned.Indexes = tpRemoveIndex(cloned.Indexes, spec.Name, "") - if len(cloned.Indexes) == before { - // Nothing removed at the table level — this may target an - // implicit index created by an inline column-level UNIQUE, - // whose server-assigned index name is the column's name. - cloned.Columns = tpClearInlineUniqueByName(cloned.Columns, spec.Name) - } - case ast.AlterTableDropPrimaryKey: - cloned.Indexes = tpRemoveIndex(cloned.Indexes, "", "PRIMARY KEY") - // Inline `col TYPE PRIMARY KEY` never appears in t.Indexes; it - // only surfaces via GetIndexes() synthesizing from this flag. - // Clearing it here makes the post-state honor DROP PRIMARY KEY. - cloned.Columns = tpClearAllInlinePrimaryKey(cloned.Columns) - } - } - return &cloned -} - -// tpColumnFromAst constructs a minimal statement.Column from an AST column def. -// Only Raw (for canonicalType), Name, and inline PrimaryKey/Unique flags are -// populated — that's enough for both rules. -func tpColumnFromAst(colDef *ast.ColumnDef) statement.Column { - col := statement.Column{ - Raw: colDef, - Name: colDef.Name.Name.O, - } - for _, opt := range colDef.Options { - switch opt.Tp { //nolint:exhaustive - case ast.ColumnOptionPrimaryKey: - col.PrimaryKey = true - case ast.ColumnOptionUniqKey: - col.Unique = true - } - } - return col -} - -func tpRemoveColumn(cols statement.Columns, name string) statement.Columns { - out := cols[:0] - for _, c := range cols { - if !strings.EqualFold(c.Name, name) { - out = append(out, c) - } - } - return out -} - -// tpReplaceColumn substitutes newCol in place of the column named oldName, -// preserving any inline PRIMARY KEY / UNIQUE flags from the old definition. -// MySQL semantics: MODIFY / CHANGE COLUMN re-types a column but does not drop -// existing constraints on it; those are removed only via DROP PRIMARY KEY / -// DROP INDEX. The new column def from the parser doesn't carry those over. -func tpReplaceColumn(cols statement.Columns, oldName string, newCol statement.Column) statement.Columns { - for i, c := range cols { - if strings.EqualFold(c.Name, oldName) { - if c.PrimaryKey && !newCol.PrimaryKey { - newCol.PrimaryKey = true - } - if c.Unique && !newCol.Unique { - newCol.Unique = true - } - cols[i] = newCol - return cols - } - } - return append(cols, newCol) -} - -// tpClearAllInlinePrimaryKey clears the inline PrimaryKey flag from every -// column. Used by DROP PRIMARY KEY since inline `col TYPE PRIMARY KEY` only -// shows up via GetIndexes() synthesizing from this flag. -func tpClearAllInlinePrimaryKey(cols statement.Columns) statement.Columns { - for i := range cols { - cols[i].PrimaryKey = false - } - return cols -} - -// tpClearInlineUniqueByName clears col.Unique on the column whose name matches -// the dropped index name. MySQL names the implicit index for an inline -// `col TYPE UNIQUE` after the column itself, so DROP INDEX can -// target it. We only run this when no table-level index of that name existed. -func tpClearInlineUniqueByName(cols statement.Columns, indexName string) statement.Columns { - if indexName == "" { - return cols - } - for i := range cols { - if strings.EqualFold(cols[i].Name, indexName) && cols[i].Unique { - cols[i].Unique = false - break - } - } - return cols -} - -func tpIndexFromConstraint(c *ast.Constraint) (statement.Index, bool) { - var typeStr string - switch c.Tp { //nolint:exhaustive - case ast.ConstraintPrimaryKey: - typeStr = "PRIMARY KEY" - case ast.ConstraintUniq, ast.ConstraintUniqKey, ast.ConstraintUniqIndex: - typeStr = "UNIQUE" - case ast.ConstraintKey, ast.ConstraintIndex: - typeStr = "INDEX" - default: - return statement.Index{}, false - } - cols := make([]string, 0, len(c.Keys)) - for _, k := range c.Keys { - if k.Column != nil { - cols = append(cols, k.Column.Name.O) - } - } - return statement.Index{ - Name: c.Name, - Type: typeStr, - Columns: cols, - }, true -} - -func tpRemoveIndex(indexes statement.Indexes, name, typeMatch string) statement.Indexes { - out := indexes[:0] - for _, idx := range indexes { - if name != "" && strings.EqualFold(idx.Name, name) { - continue - } - if typeMatch != "" && idx.Type == typeMatch { - continue - } - out = append(out, idx) - } - return out -} - // tpCollectIndexedColumns returns the lower-cased set of every column that // participates in any index on the table. Uses GetIndexes() so that inline // column-level PRIMARY KEY and UNIQUE declarations are honored — those don't diff --git a/pkg/lint/lint_zero_date.go b/pkg/lint/lint_zero_date.go index 9ff2ea12..7adaf5aa 100644 --- a/pkg/lint/lint_zero_date.go +++ b/pkg/lint/lint_zero_date.go @@ -28,34 +28,20 @@ func (l *ZeroDateLinter) String() string { return Stringer(l) } +// Lint operates on a post-state view of the schema so that ALTERs which fix +// zero-date columns don't produce false positives on the pre-state. func (l *ZeroDateLinter) Lint(existingTables []*statement.CreateTable, changes []*statement.AbstractStatement) (violations []Violation) { - for ct := range CreateTableStatements(existingTables, changes) { + for _, ct := range PostState(existingTables, changes) { for _, column := range ct.Columns { + if column.Raw == nil { + continue + } v := l.checkColumnZeroDate(column.Raw, ct.TableName) if v != nil { violations = append(violations, *v) } } } - - // Check ALTER TABLE statements - for _, change := range changes { - at, ok := change.AsAlterTable() - if !ok { - continue - } - for _, spec := range at.Specs { - switch spec.Tp { //nolint:exhaustive - case ast.AlterTableAddColumns, ast.AlterTableModifyColumn, ast.AlterTableChangeColumn: - for _, column := range spec.NewColumns { - v := l.checkColumnZeroDate(column, at.Table.Name.String()) - if v != nil { - violations = append(violations, *v) - } - } - } - } - } return violations } diff --git a/pkg/lint/post_state.go b/pkg/lint/post_state.go new file mode 100644 index 00000000..0a01aff2 --- /dev/null +++ b/pkg/lint/post_state.go @@ -0,0 +1,326 @@ +package lint + +import ( + "sort" + "strings" + + "github.com/block/spirit/pkg/statement" + "github.com/pingcap/tidb/pkg/parser/ast" +) + +// PostState returns a deterministic post-state view of the schema: the existing +// tables augmented with CREATE TABLE statements from changes, and existing tables +// patched with ADD/DROP/MODIFY/CHANGE COLUMN and ADD/DROP INDEX (and DROP PRIMARY +// KEY) specs from ALTER TABLE statements. Other ALTER specs are ignored. +// +// The result is sorted by table name. The input tables are not mutated. +// +// Linters that need to evaluate the schema as it will exist *after* a set of +// pending changes (rather than as it exists today) should iterate this slice +// rather than existingTables directly — otherwise they will produce false +// positives for ALTER statements that fix legacy issues. +func PostState(existing []*statement.CreateTable, changes []*statement.AbstractStatement) []*statement.CreateTable { + byName := make(map[string]*statement.CreateTable, len(existing)) + for _, t := range existing { + byName[strings.ToLower(t.TableName)] = t + } + + for _, change := range changes { + if change == nil { + continue + } + if change.IsCreateTable() { + ct, err := change.ParseCreateTable() + if err == nil && ct != nil { + byName[strings.ToLower(ct.TableName)] = ct + } + continue + } + at, ok := change.AsAlterTable() + if !ok { + continue + } + key := strings.ToLower(change.Table) + base, found := byName[key] + if !found { + // Synthesize a placeholder for ALTERs whose target isn't in + // existingTables. applyAlter still produces a best-effort + // post-state — column adds/modifies/changes show up; drops are + // harmless against an empty column list. This is the right + // behavior for linters that evaluate the affected columns + // individually (has_timestamp, has_float, zero_date) even + // without full schema context. + base = &statement.CreateTable{TableName: change.Table} + } + byName[key] = applyAlter(base, at) + } + + out := make([]*statement.CreateTable, 0, len(byName)) + for _, t := range byName { + out = append(out, t) + } + sort.Slice(out, func(i, j int) bool { return out[i].TableName < out[j].TableName }) + return out +} + +// newTablesInChanges returns the set of (lowercased) table names that are +// created by a CREATE TABLE statement in changes. Columns inside these tables +// are considered "new", not legacy. +func newTablesInChanges(changes []*statement.AbstractStatement) map[string]bool { + out := make(map[string]bool) + for _, change := range changes { + if change == nil || !change.IsCreateTable() { + continue + } + ct, err := change.ParseCreateTable() + if err != nil || ct == nil { + continue + } + out[strings.ToLower(ct.TableName)] = true + } + return out +} + +// columnsAddedOrModifiedInChanges returns, for each table, the set of +// (lowercased) column names that are added or modified by ALTER TABLE +// statements in changes. CHANGE COLUMN records both the old name and the +// new name as added/modified — neither is "pre-existing untouched". MODIFY +// COLUMN records the column name. ADD COLUMN records the new column name. +// DROP COLUMN is not recorded here (dropped columns don't appear in +// post-state at all). +func columnsAddedOrModifiedInChanges(changes []*statement.AbstractStatement) map[string]map[string]bool { + out := make(map[string]map[string]bool) + mark := func(table, col string) { + tKey := strings.ToLower(table) + if out[tKey] == nil { + out[tKey] = make(map[string]bool) + } + out[tKey][strings.ToLower(col)] = true + } + for _, change := range changes { + if change == nil { + continue + } + at, ok := change.AsAlterTable() + if !ok { + continue + } + for _, spec := range at.Specs { + switch spec.Tp { //nolint:exhaustive + case ast.AlterTableAddColumns, ast.AlterTableModifyColumn: + for _, col := range spec.NewColumns { + if col.Name != nil { + mark(change.Table, col.Name.Name.O) + } + } + case ast.AlterTableChangeColumn: + if spec.OldColumnName != nil { + mark(change.Table, spec.OldColumnName.Name.O) + } + for _, col := range spec.NewColumns { + if col.Name != nil { + mark(change.Table, col.Name.Name.O) + } + } + } + } + } + return out +} + +// PreStateColumns returns a lookup of (lowercased table name) → (lowercased +// column name) → column from the existing tables. Linters use this to +// distinguish columns that pre-existed (severity Warning) from columns added +// or modified by changes (severity Error). +func PreStateColumns(existing []*statement.CreateTable) map[string]map[string]*statement.Column { + out := make(map[string]map[string]*statement.Column, len(existing)) + for _, t := range existing { + tKey := strings.ToLower(t.TableName) + cols := make(map[string]*statement.Column, len(t.Columns)) + for i := range t.Columns { + c := &t.Columns[i] + cols[strings.ToLower(c.Name)] = c + } + out[tKey] = cols + } + return out +} + +// applyAlter returns a shallow clone of t with the relevant subset of alter +// specs applied. The original table is not mutated. +// +// Note: GetIndexes() synthesizes PRIMARY KEY / UNIQUE entries from inline +// column-level flags (col.PrimaryKey, col.Unique), so the drop paths below +// must clear those flags in addition to scrubbing cloned.Indexes — otherwise +// the synthesis would resurrect the dropped index in the post-state. +func applyAlter(t *statement.CreateTable, at *ast.AlterTableStmt) *statement.CreateTable { + cloned := *t + cloned.Columns = append(statement.Columns(nil), t.Columns...) + cloned.Indexes = append(statement.Indexes(nil), t.Indexes...) + + for _, spec := range at.Specs { + switch spec.Tp { //nolint:exhaustive + case ast.AlterTableAddColumns: + for _, colDef := range spec.NewColumns { + cloned.Columns = append(cloned.Columns, columnFromAst(colDef)) + } + case ast.AlterTableDropColumn: + if spec.OldColumnName != nil { + cloned.Columns = removeColumn(cloned.Columns, spec.OldColumnName.Name.O) + } + case ast.AlterTableModifyColumn: + if len(spec.NewColumns) > 0 { + colDef := spec.NewColumns[0] + cloned.Columns = replaceColumn(cloned.Columns, colDef.Name.Name.O, columnFromAst(colDef)) + } + case ast.AlterTableChangeColumn: + if len(spec.NewColumns) > 0 && spec.OldColumnName != nil { + colDef := spec.NewColumns[0] + cloned.Columns = replaceColumn(cloned.Columns, spec.OldColumnName.Name.O, columnFromAst(colDef)) + } + case ast.AlterTableAddConstraint: + if spec.Constraint == nil { + continue + } + if idx, ok := indexFromConstraint(spec.Constraint); ok { + cloned.Indexes = append(cloned.Indexes, idx) + // An inline-flag column whose constraint we just promoted to + // the table-level Indexes list could be double-counted. The + // duplication is harmless for the indexed-column set (a set, + // not a multiset), so we don't dedupe here. + } + case ast.AlterTableDropIndex: + before := len(cloned.Indexes) + cloned.Indexes = removeIndex(cloned.Indexes, spec.Name, "") + if len(cloned.Indexes) == before { + // Nothing removed at the table level — this may target an + // implicit index created by an inline column-level UNIQUE, + // whose server-assigned index name is the column's name. + cloned.Columns = clearInlineUniqueByName(cloned.Columns, spec.Name) + } + case ast.AlterTableDropPrimaryKey: + cloned.Indexes = removeIndex(cloned.Indexes, "", "PRIMARY KEY") + // Inline `col TYPE PRIMARY KEY` never appears in t.Indexes; it + // only surfaces via GetIndexes() synthesizing from this flag. + // Clearing it here makes the post-state honor DROP PRIMARY KEY. + cloned.Columns = clearAllInlinePrimaryKey(cloned.Columns) + } + } + return &cloned +} + +// columnFromAst constructs a minimal statement.Column from an AST column def. +// Only Raw (for type information), Name, and inline PrimaryKey/Unique flags +// are populated — that's enough for the linters that need post-state. +func columnFromAst(colDef *ast.ColumnDef) statement.Column { + col := statement.Column{ + Raw: colDef, + Name: colDef.Name.Name.O, + } + for _, opt := range colDef.Options { + switch opt.Tp { //nolint:exhaustive + case ast.ColumnOptionPrimaryKey: + col.PrimaryKey = true + case ast.ColumnOptionUniqKey: + col.Unique = true + } + } + return col +} + +func removeColumn(cols statement.Columns, name string) statement.Columns { + out := cols[:0] + for _, c := range cols { + if !strings.EqualFold(c.Name, name) { + out = append(out, c) + } + } + return out +} + +// replaceColumn substitutes newCol in place of the column named oldName, +// preserving any inline PRIMARY KEY / UNIQUE flags from the old definition. +// MySQL semantics: MODIFY / CHANGE COLUMN re-types a column but does not drop +// existing constraints on it; those are removed only via DROP PRIMARY KEY / +// DROP INDEX. The new column def from the parser doesn't carry those over. +func replaceColumn(cols statement.Columns, oldName string, newCol statement.Column) statement.Columns { + for i, c := range cols { + if strings.EqualFold(c.Name, oldName) { + if c.PrimaryKey && !newCol.PrimaryKey { + newCol.PrimaryKey = true + } + if c.Unique && !newCol.Unique { + newCol.Unique = true + } + cols[i] = newCol + return cols + } + } + return append(cols, newCol) +} + +// clearAllInlinePrimaryKey clears the inline PrimaryKey flag from every +// column. Used by DROP PRIMARY KEY since inline `col TYPE PRIMARY KEY` only +// shows up via GetIndexes() synthesizing from this flag. +func clearAllInlinePrimaryKey(cols statement.Columns) statement.Columns { + for i := range cols { + cols[i].PrimaryKey = false + } + return cols +} + +// clearInlineUniqueByName clears col.Unique on the column whose name matches +// the dropped index name. MySQL names the implicit index for an inline +// `col TYPE UNIQUE` after the column itself, so DROP INDEX can +// target it. We only run this when no table-level index of that name existed. +func clearInlineUniqueByName(cols statement.Columns, indexName string) statement.Columns { + if indexName == "" { + return cols + } + for i := range cols { + if strings.EqualFold(cols[i].Name, indexName) && cols[i].Unique { + cols[i].Unique = false + break + } + } + return cols +} + +func indexFromConstraint(c *ast.Constraint) (statement.Index, bool) { + var typeStr string + switch c.Tp { //nolint:exhaustive + case ast.ConstraintPrimaryKey: + typeStr = "PRIMARY KEY" + case ast.ConstraintUniq, ast.ConstraintUniqKey, ast.ConstraintUniqIndex: + typeStr = "UNIQUE" + case ast.ConstraintKey, ast.ConstraintIndex: + typeStr = "INDEX" + default: + return statement.Index{}, false + } + cols := make([]string, 0, len(c.Keys)) + for _, k := range c.Keys { + if k.Column != nil { + cols = append(cols, k.Column.Name.O) + } + } + return statement.Index{ + Name: c.Name, + Type: typeStr, + Columns: cols, + }, true +} + +func removeIndex(indexes statement.Indexes, name, typeMatch string) statement.Indexes { + out := indexes[:0] + for _, idx := range indexes { + if name != "" && strings.EqualFold(idx.Name, name) { + continue + } + if typeMatch != "" && idx.Type == typeMatch { + continue + } + out = append(out, idx) + } + return out +} From 9ea6f7579c8de6a292cb0d3420599c56202b502c Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 12 May 2026 07:48:38 -0600 Subject: [PATCH 2/2] lint(has_float): restore "New column" message wording for ADD COLUMN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous post-state refactor collapsed ADD COLUMN and MODIFY/CHANGE into a single addedOrModified set, so newly-added FLOAT/DOUBLE columns were emitted as "modified to use floating-point data type" — confusing phrasing for what is actually a brand-new column. Tracks MODIFY/CHANGE in a separate columnsModifiedInChanges helper and selects message wording by (pre-state existence, modified flag): - ADD COLUMN or column inside a new CREATE TABLE → "New column …" - MODIFY / CHANGE COLUMN on a pre-existing column → "… modified to …" - Pre-existing untouched FLOAT/DOUBLE → "… uses …" Adds positive/negative assertions on the ADD COLUMN test so this can't regress silently again. --- pkg/lint/lint_has_float.go | 23 ++++++++++++----- pkg/lint/lint_has_float_test.go | 3 +++ pkg/lint/post_state.go | 45 +++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/pkg/lint/lint_has_float.go b/pkg/lint/lint_has_float.go index 54065801..a68a8687 100644 --- a/pkg/lint/lint_has_float.go +++ b/pkg/lint/lint_has_float.go @@ -28,11 +28,17 @@ func (l *HasFloatLinter) Description() string { // Lint operates on a post-state view of the schema, so a column being // converted away from FLOAT/DOUBLE in an ALTER doesn't generate a false -// positive. The violation message distinguishes pre-existing columns from -// columns added/modified by the changeset to preserve actionability. +// positive. The violation message uses three forms to preserve +// actionability: +// +// - "New column …" when the column did not exist in the pre-state +// (ADD COLUMN, or a column in a CREATE TABLE). +// - "Column … modified to use …" when the column existed pre-state and +// is being retyped by MODIFY / CHANGE COLUMN. +// - "Column … uses …" for pre-existing untouched columns. func (l *HasFloatLinter) Lint(existingTables []*statement.CreateTable, changes []*statement.AbstractStatement) (violations []Violation) { - addedOrModified := columnsAddedOrModifiedInChanges(changes) - createdInChanges := newTablesInChanges(changes) + pre := PreStateColumns(existingTables) + modified := columnsModifiedInChanges(changes) for _, ct := range PostState(existingTables, changes) { tKey := strings.ToLower(ct.TableName) @@ -46,11 +52,16 @@ func (l *HasFloatLinter) Lint(existingTables []*statement.CreateTable, changes [ } colName := col.Name colKey := strings.ToLower(colName) - modified := !createdInChanges[tKey] && addedOrModified[tKey][colKey] + _, preExisted := pre[tKey][colKey] var message string switch { - case modified: + case modified[tKey][colKey]: + // MODIFY / CHANGE COLUMN — the column existed pre-state + // (or is asserted to) and is being retyped. message = fmt.Sprintf("Column %q in table %q modified to use floating-point data type", colName, ct.TableName) + case !preExisted: + // ADD COLUMN, or a column inside a new CREATE TABLE. + message = fmt.Sprintf("New column %q in table %q uses floating-point data type", colName, ct.TableName) default: message = fmt.Sprintf("Column %q in table %q uses %s data type", colName, ct.TableName, col.Raw.Tp.String()) } diff --git a/pkg/lint/lint_has_float_test.go b/pkg/lint/lint_has_float_test.go index b0bc78c9..fdf73b01 100644 --- a/pkg/lint/lint_has_float_test.go +++ b/pkg/lint/lint_has_float_test.go @@ -522,6 +522,9 @@ func TestHasFloatLinter_AlterTableAddFloatColumn(t *testing.T) { require.Equal(t, SeverityWarning, violations[0].Severity) require.Contains(t, violations[0].Message, "temperature") require.Contains(t, violations[0].Message, "measurements") + // ADD COLUMN should produce the "New column" message form, not "modified". + require.Contains(t, violations[0].Message, "New column") + require.NotContains(t, violations[0].Message, "modified") require.Equal(t, "measurements", violations[0].Location.Table) require.Equal(t, "temperature", *violations[0].Location.Column) } diff --git a/pkg/lint/post_state.go b/pkg/lint/post_state.go index 0a01aff2..7bf03139 100644 --- a/pkg/lint/post_state.go +++ b/pkg/lint/post_state.go @@ -128,6 +128,51 @@ func columnsAddedOrModifiedInChanges(changes []*statement.AbstractStatement) map return out } +// columnsModifiedInChanges returns, for each table, the (lowercased) column +// names that are retyped via MODIFY COLUMN or CHANGE COLUMN — i.e. operations +// that act on a column that should already exist. The map distinguishes +// retypes from ADD COLUMN, which is useful when a linter wants different +// messaging for "new column" vs "existing column being changed". +func columnsModifiedInChanges(changes []*statement.AbstractStatement) map[string]map[string]bool { + out := make(map[string]map[string]bool) + mark := func(table, col string) { + tKey := strings.ToLower(table) + if out[tKey] == nil { + out[tKey] = make(map[string]bool) + } + out[tKey][strings.ToLower(col)] = true + } + for _, change := range changes { + if change == nil { + continue + } + at, ok := change.AsAlterTable() + if !ok { + continue + } + for _, spec := range at.Specs { + switch spec.Tp { //nolint:exhaustive + case ast.AlterTableModifyColumn: + for _, col := range spec.NewColumns { + if col.Name != nil { + mark(change.Table, col.Name.Name.O) + } + } + case ast.AlterTableChangeColumn: + if spec.OldColumnName != nil { + mark(change.Table, spec.OldColumnName.Name.O) + } + for _, col := range spec.NewColumns { + if col.Name != nil { + mark(change.Table, col.Name.Name.O) + } + } + } + } + } + return out +} + // PreStateColumns returns a lookup of (lowercased table name) → (lowercased // column name) → column from the existing tables. Linters use this to // distinguish columns that pre-existed (severity Warning) from columns added