Fix SQLite AUTOINCREMENT constraint ordering#7664
Open
russellromney wants to merge 1 commit into
Open
Conversation
26c60e5 to
fc7f4de
Compare
VaggelisD
reviewed
May 21, 2026
Collaborator
VaggelisD
left a comment
There was a problem hiding this comment.
Hey @russellromney, thank you for the PR! Just a couple of nits:
| "sqlite": "CREATE TABLE z (a INTEGER PRIMARY KEY AUTOINCREMENT)", | ||
| "mysql": "CREATE TABLE z (a INT PRIMARY KEY AUTO_INCREMENT)", | ||
| }, | ||
| ) |
Collaborator
There was a problem hiding this comment.
This test seems like a duplicate of the other, I think I'd wrap it in a for loop e.g
for constraint in ("PRIMARY KEY AUTO_INCREMENT", "AUTO_INCREMENT PRIMARY KEY"):
self.validate_all(
...,
read={
"mysql": f"CREATE TABLE z (a INT {constraint})",
},
...
)
Collaborator
There was a problem hiding this comment.
Adding to this^, make sure to do with self.subTest if you do the loop.
| return expression | ||
|
|
||
|
|
||
| def _ensure_auto_increment_follows_primary_key(column: exp.ColumnDef) -> None: |
Collaborator
There was a problem hiding this comment.
I think the previous for loop was a bit easier to understand plus we also don't have to loop over constraints twice with next; Should we revert and patch it with insert instead?
fc7f4de to
c52b36b
Compare
Author
|
All feedback addressed. |
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.
Summary
Fix SQLite generation for MySQL auto-incrementing primary keys so
AUTOINCREMENTis preserved regardless of source constraint order and emitted in a SQLite-valid position.Before this change, semantically equivalent MySQL definitions produced different SQLite SQL:
This also fixes a table-level primary key case that previously generated SQLite syntax rejected by SQLite itself:
SQLite accepts
AUTOINCREMENTafterINTEGER PRIMARY KEY; placing it beforePRIMARY KEYis a syntax error.Changes
AutoIncrementColumnConstraintfollows the columnPrimaryKeyColumnConstraintwhen both are present.AUTOINCREMENTforAUTO_INCREMENT PRIMARY KEYas well asPRIMARY KEY AUTO_INCREMENT.Tests
I also sanity-checked the fixed SQLite output with Python's
sqlite3module.