Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLite reflection: parentheses wrongly stripped from expression in default clause #579

Closed
zmwangx opened this issue Jun 25, 2019 · 7 comments

Comments

Projects
None yet
3 participants
@zmwangx
Copy link

commented Jun 25, 2019

I have a table (SQLite) with a column that looks like

created_at DATETIME DEFAULT (datetime('now', 'localtime')) NOT NULL,

The parentheses around datetime('now', 'localtime') are syntactically necessary, without which one would get a syntax error:

The DEFAULT clause specifies a default value to use for the column if no value is explicitly provided by the user when doing an INSERT. If there is no explicit DEFAULT clause attached to a column definition, then the default value of the column is NULL. An explicit DEFAULT clause may specify that the default value is NULL, a string constant, a blob constant, a signed-number, or any constant expression enclosed in parentheses.

https://www.sqlite.org/lang_createtable.html (emphasis mine)

When I use op.batch_alter_table, the reflected column's server_default is a text clause datetime('now', 'localtime'), without parentheses, so when alembic attempts to create the new table it fails:

CREATE TABLE _alembic_tmp_foo (
    ...
	created_at DATETIME DEFAULT datetime('now', 'localtime') NOT NULL, -- syntax error
)

I'm new to SQLA and Alembic so please excuse me if the fault is on me.

@zmwangx

This comment has been minimized.

Copy link
Author

commented Jun 25, 2019

(By the way, I know how to work around this: explicitly defining the column in reflected_args.)

@zzzeek

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

OK SQLite doesn't give us back those parenthesis from PRAGMA table_info. It gives them back to us in the "sql" column of "sqlite_master".

however, this can be a vastly easier bug if we just say that we can apply parenthesis in all cases to column defaults. Seems to work for my arbitrary version of sqlite:

sqlite> CREATE TABLE foo (x integer default 5, y datetime DEFAULT (datetime('now', 'localtime')));
sqlite> CREATE TABLE bar (x integer default (5), y datetime DEFAULT (datetime('now', 'localtime')));
sqlite> pragma table_info(foo);
0|x|integer|0|5|0
1|y|datetime|0|datetime('now', 'localtime')|0
sqlite> pragma table_info(bar);
0|x|integer|0|5|0
1|y|datetime|0|datetime('now', 'localtime')|0
sqlite> insert into foo DEFAULT VALUES;
sqlite> insert into bar DEFAULT VALUES;
sqlite> select * from foo;
5|2019-06-25 09:48:24
sqlite> select * from bar;
5|2019-06-25 09:48:27
sqlite> select * from foo, bar where foo.x = bar.x;
5|2019-06-25 09:48:24|5|2019-06-25 09:48:27

I was going to say that SQLAlchemy reflection should report on these parens but that seems a little strange.

@zzzeek

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

this might affect autogenerate too, I would think

@zzzeek

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

So your workaround is pretty good, you can also use a refleciton listener like this:

import re

def autogen_column_reflect(inspector, table, column_info):
    if "default" in column_info:
        if not re.match(r'^\(.+\)$', column_info["default"]):
            column_info["default"] = "(%s)" % (column_info['default'],)

op.batch_alter_table(table_name, reflect_kwargs={"listeners": ("column_reflect", autogen_column_reflect)})     

I greatly prefer to fix these issues on the "render" side but in this case it's just not happening so I'll be using the above approach internally as well

@sqla-tester

This comment has been minimized.

Copy link
Collaborator

commented Jun 25, 2019

Mike Bayer has proposed a fix for this issue in the master branch:

Ensure SQLite default expressions are parenthesized https://gerrit.sqlalchemy.org/1327

@zmwangx

This comment has been minimized.

Copy link
Author

commented Jun 26, 2019

Thank you for the fast turnaround!

@zzzeek

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

welp there was no way around this one but through it so I figured get it done before I forget all about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.