Skip to content

fix: resolve integer/enum types with limit in add_column#235

Open
cbisnett wants to merge 2 commits intoPNixx:masterfrom
huntresslabs:fix/add-column-type-resolution
Open

fix: resolve integer/enum types with limit in add_column#235
cbisnett wants to merge 2 commits intoPNixx:masterfrom
huntresslabs:fix/add-column-type-resolution

Conversation

@cbisnett
Copy link
Copy Markdown
Contributor

@cbisnett cbisnett commented Mar 7, 2026

Summary

  • add_column with :integer or :enum types ignored :limit and :unsigned options, always producing the default type (e.g. UInt32 for integers instead of Int16 when limit: 2, unsigned: false)
  • Root cause: type resolution only happened in TableDefinition (used by create_table), but add_column bypassed it and went through Rails' type_to_sql which only looks up NATIVE_DATABASE_TYPES
  • Extracts type resolution into resolve_type_with_limit (private method on the adapter) and calls it from add_column before delegating to super

Example

# Before fix: produces ALTER TABLE t ADD COLUMN col UInt32 (wrong)
# After fix:  produces ALTER TABLE t ADD COLUMN col Int16 (correct)
add_column :t, :col, :integer, limit: 2, unsigned: false, null: false

Test plan

  • Added migration fixture with add_column using various limit/unsigned combos for integer types
  • Added spec verifying correct ClickHouse types: Int16, Int32, Int64, UInt8, UInt16, UInt64, UInt32
  • Full migration spec suite passes (28 examples, 0 failures)

🤖 Generated with Claude Code

cbisnett and others added 2 commits March 7, 2026 12:51
add_column ignored :limit and :unsigned options for :integer and :enum
types because type resolution only happened in TableDefinition (used by
create_table). This caused add_column to always produce the default
type (e.g. UInt32 for integers) regardless of the limit specified.

Extract the type resolution logic into resolve_type_with_limit and
call it from add_column before delegating to super.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- TableDefinition#integer now delegates to the adapter's
  resolve_integer_kind, eliminating duplicated logic
- Fix signed limit: 5 falling through to UInt32 instead of Int64
  (condition was > 5, now >= 5)
- Add test coverage for limit: 5 signed integer case

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant