Fix auto_increment on string columns with integer initial#49
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes incorrect auto-increment behavior when the database column is a string/text type but the configured initial value is an integer (default 1). In that scenario, using SQL MAX() on a string column can compare lexicographically (e.g., '9' > '10'), causing duplicates; this PR instead detects the DB column type and applies the string-safe “length-aware ordering + String#next” strategy.
Changes:
- Detect string/text DB columns via
columns_hashand treat them as string sequences even wheninitialis an integer. - Add regression coverage for the
9 → 10boundary on string columns with integer initial values. - Update README wording/badges to reflect current behavior/docs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/support/active_record.rb | Adds a posts table with a string ref column to reproduce the reported scenario. |
| spec/models/post.rb | Introduces a Post model using auto_increment :ref for integration-style verification. |
| spec/lib/incrementor_spec.rb | Adds a unit/regression spec ensuring length-aware ordering is used for string DB columns even with integer initial. |
| spec/lib/active_record_spec.rb | Adds an end-to-end spec verifying increments continue correctly past 9 on a string column. |
| README.md | Updates the maintainability badge and clarifies that string sequences follow Ruby’s String#next. |
| lib/auto_increment/incrementor.rb | Extends string detection to include string/text DB column types via columns_hash. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
felipediesel
force-pushed
the
fix-string-column-increment
branch
2 times, most recently
from
5587af6 to
b789aef
Compare
When the DB column is a string type (e.g. varchar in PostgreSQL) but initial is an integer (the default), the SQL MAX() function does lexicographic comparison, causing '9' > '10' and producing duplicate values from record 10 onward. Fix by checking the actual DB column type via columns_hash. If the column is :string or :text, use the length-aware ORDER BY query strategy and String#next for incrementing, regardless of the @initial type.
string? now only relies on column_string? to detect string/text columns, instead of also checking @initial.instance_of?(String). This avoids incorrect LENGTH ordering when a string initial is used on an integer column. Also reorganize incrementor_spec.rb for clarity.
felipediesel
force-pushed
the
fix-string-column-increment
branch
from
a9d54fe to
05d6db7
Compare
felipediesel
force-pushed
the
fix-string-column-increment
branch
from
a0b6c07 to
5cb1923
Compare
| ``` | ||
|
|
||
| String sequences follow the same pattern as Excel columns. | ||
| String sequences follow Ruby's [`String#next`](https://ruby-doc.org/3.4/Object.html#method-i-next) logic. The column type is inferred from the database schema. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Auto-detect initial value type
When
initialis not explicitly set,Incrementornow infers the default based on the database column type:"1"for string/text columns,1for integer columns. This prevents the common pitfall of a mismatched default.Deprecation warning
When
initialis explicitly passed with a type that doesn't match the column (e.g.auto_increment :ref, initial: 1on astringcolumn), a deprecation warning is emitted at class-load time. This fires once per model, not on every record creation.String column fix
When the DB column is a string type but
initialis an integer, the SQLMAX()function does lexicographic comparison, causing'9' > '10'and duplicate values from record 10 onward. Fixed by checking the actual DB column type viacolumns_hash, string/text columns now use length-awareORDER BYandString#nextfor incrementing.Closes #18