dbal
dbal copied to clipboard
doctrine
Fix "bigint" type introspection for PK column with AI for SQLite
| Q | A |
|---|---|
| Type | bug |
| Fixed issues | #6396 |
Summary
Because SQLite does not support AUTOINCREMENT keyword on any other type than INTEGER [1] [2], bigint type is mapped to INTEGER SQLite database column by DBAL already [3].
As we remap the type on table creation, we need to remap the type also on table introspection. The reason, we remap integer to bigint, is bigint is subtype of integer - PK defined/created originally as bigint must be always introspected as bigint.
[1] https://www.sqlite.org/autoinc.html [2] https://dbfiddle.uk/yyXvHV-6 [3] https://github.com/doctrine/dbal/blob/3.8.4/src/Platforms/SqlitePlatform.php#L271-L274
It would seem that this is going to cause the opposite problem: a primary key with type INTEGER is wrongly going to be introspected as BIGINT, isn't it?
This is unavoidable because of #5107 and expected - INT is subtype of BIGINT, so BIGINT must be introspected because of https://github.com/doctrine/dbal/blob/3.8.4/src/Platforms/SqlitePlatform.php#L271-L274 transformation done when creating the table.
The auto increment are in the sequence table and these are integers probably. You could select from this table to check if there is auto increment.
Is it really wanted to do additional query in _getPortableTableColumnDefinition method?
I'm not comfortable with merging this change into 3.8, tbh.
As you've already noticed, SQLite's implementation of integer PKs is a bit odd. The main issue is that SQLite does not allow you to create BIGINT PKs which is why the DBAL creates an INTEGER PRIMARY KEY column instead which has the value range of a BIGINT.
This also means that when introspecting, we cannot tell the difference between a column that was created by DBAL from a IntegerType and one that was created from a BigIntType. This is why the DBAL guesses one of two possible types. Apparently the wrong one in your case. Your "fix" would simply change that guess to a different one. That guess is not more accurate that the currently implemented one. It's just wrong for a different group of apps.
You could also solve this in documentation: say in sqlite primary keys are just ints, not bigints and do nothing about the code or throw an exception when a primary key is not an int in sqlite
throw an exception when a primary key is not an int in sqlite
We probably don't want this. If someone uses BIGINT PKs on their entity model, e.g. because they're using MySQL in production, they should still be able to deploy their model to SQLite for testing purposes.
I think the point was that it shouldn't fail. Prod uses MySQL and a Test environment SQLite. It would break projects if SQLite would suddenly throw an exception. If it would stay that way, I wouldn't expect such a change in 3.8 or 4.0.
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.