- attached sqlite_AUTOINCREMENT.patch
sqlite AUTOINCREMENT support for INT PRIMARY KEY column
sqlite allow use AUTOINCREMENT for PRIMARY KEY constraint, if specified inline with column definition
CREATE TABLE ( id INT PRIMARY KEY AUTOINCREMENT, .... )
this can be used only if one column of type INT is used as primary key.
I'v done small patch, which works well for my and I this this might be usefull for others.
Comments (14)
-
Account Deleted -
-
repo owner - marked as minor
unfortunately its not that simple. the AUTOINCREMENT keyword changes the behavior of the column and is not the behavior most sqlite users expect (note that sqlite primary key columns already "autoincrement" by applying the current rowid: described at http://www.sqlite.org/autoinc.html ), including that performance suffers. I'd want to go with a flag such as "sqlite_autoincrement=True", which suggests that the **kw system we use for
Table
objects (i.e., which allows you to saymysql_engine=InnoDB
) would be applied to column objects. -
repo owner to be clear:
Column
would receive an__extra_kwargs(**kwargs)
method similar to that ofTable
, which parses keywords of the form<dialect>_<option>
and stores them inColumn.kwargs
- remaining keywords that are not of this form raise an error. The SQLite dialect then readsColumn.kwargs['sqlite_autoincrement']('sqlite_autoincrement')
instead ofColumn.autoincrement
. -
Account Deleted I think sqlite-autoincrement.patch is such a patch. I wrote it against the 0.6 branch, but it seems to also apply to trunk.
It doesn't cause any of the tests to fail against either 0.5 or 0.6, although I don't quite understand the test layout well enough to write my own test for this.
(Also, could you please add broder@mit.edu as a CC on this ticket? thanks)
-
Account Deleted Err...that first round was a bit off. This one's a little closer, but in spite of the override of visit_primary_key_constraint in SQLiteDDLCompiler, it still gets visited, so an empty string with an extra trailing comma which is an error in sqlite.
-
repo owner - changed watchers to broder@mit.edu
- changed milestone to 0.6.xx
this should address the trailing comma issue:
Index: lib/sqlalchemy/sql/compiler.py =================================================================== --- lib/sqlalchemy/sql/compiler.py (revision 6007) +++ lib/sqlalchemy/sql/compiler.py (working copy) @@ -829,7 +829,9 @@ # On some DB order is significant: visit PK first, then the # other constraints (engine.ReflectionTest.testbasic failed on FB2) if table.primary_key: - text += ", \n\t" + self.process(table.primary_key) + pk = self.process(table.primary_key) + if pk: + text += ", \n\t" + pk const = ", \n\t".join( self.process(constraint) for constraint in table.constraints if constraint is not table.primary_key
also, I would rather have "sqlite_autoincrement" as one of the
Table()
**kwargs since we already have a mechanism for accepting dialect-specific arguments there, and this is a flag that is once-per-table.we also need a test added to test/dialect/sqlite.py which uses assert_sql() to verify the desired SQL is emitted for a CREATE TABLE statement.
-
Account Deleted Take 3, with the changes you suggested and a few tests.
I'm not really used to writing SQLAlchemy tests, so I kind of suspect that I got the style and flow completely wrong - feel free to make suggestions.
-
repo owner the test looks pretty good. I was suggesting the possibly simpler approach of http://www.sqlalchemy.org/trac/browser/sqlalchemy/branches/rel_0_6/test/sql/constraints.py#L270 but what you have does the job too.
-
Account Deleted Ah - that makes way more sense. New patch attached.
-
repo owner I'll patch this to 0.6 soon. let me know if you are in a hurry.
-
repo owner -
repo owner - changed status to resolved
thanks for this, patched in 33f2e2bfbbc090de9cd0e0d3bd63afda41999fa9 with some bonehead activity in 62b2fb87569c7bd7c72fde2f8f7933e3efc2629e 68da6c5e1d66b9d39f020a20c11696703ae2bc23
-
repo owner - removed milestone
Removing milestone: 0.6.0 (automated comment)
- Log in to comment
add AUTOINCREMENT suppert for sqlite (sqlalchemy 4.5)