Issue #73 new

More PG / postgresql nextval. it needs the text() wrapper plus the Sequence. Consider regex/guessing ?

Michael Armida avatarMichael Armida created an issue

alembic revision --autogenerate creates SQLAlchemy code that, on postgres, generates SQL syntax errors. I've pasted the stack trace below, and written a fixture to reproduce the problem.

In general, table creation upgrades and downgrades aren't symmetric; the generated code for a new table in an upgrade isn't the same as that in a downgrade. This may be by design.

On sqlite, this doesn't cause a problem. Unfortunately, it hits a snag on postgres trying to handle the default value for the PK - it doesn't reference the sequence properly.

I've written a test fixture that (hopefully) isolates and reproduces the problem. Because it's many files and crosses shell commands, I've stored it as a git repo: https://github.com/marmida/alembic-downgrade-fixture

==Environment==

  • alembic 0.3.6
  • SQLAlchemy 0.7.8
  • psycopg2 2.4.5
  • Postgres 9.1+129
  • Ubuntu 12.04

== Stack trace ==

{{{ INFO [alembic.migration] Running downgrade 73627e675dc -> 3f5579680f8b Traceback (most recent call last): File "venv/bin/alembic", line 9, in <module> load_entry_point('alembic==0.3.6', 'console_scripts', 'alembic')() File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/config.py", line 229, in main dict((k, getattr(options, k)) for k in kwarg) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/command.py", line 145, in downgrade script.run_env() File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/script.py", line 192, in run_env util.load_python_file(self.dir, 'env.py') File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/util.py", line 185, in load_python_file module = imp.load_source(module_id, path, open(path, 'rb')) File "migrations/env.py", line 72, in <module> run_migrations_online() File "migrations/env.py", line 65, in run_migrations_online context.run_migrations() File "<string>", line 7, in run_migrations File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/environment.py", line 467, in run_migrations self.get_context().run_migrations(kw) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/migration.py", line 211, in run_migrations change(kw) File "migrations/versions/73627e675dc_drop.py", line 28, in downgrade sa.PrimaryKeyConstraint(u'id', name=u'stuff_pkey') File "<string>", line 7, in create_table File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/operations.py", line 527, in create_table self._table(name, *columns, kw) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/ddl/impl.py", line 145, in create_table self._exec(schema.CreateTable(table)) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/alembic/ddl/impl.py", line 75, in _exec conn.execute(construct, multiparams, *params) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1449, in execute params) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1542, in _execute_ddl compiled File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1698, in _execute_context context) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1691, in _execute_context context) File "/home/marmida/develop/t/venv/local/lib/python2.7/site-packages/sqlalchemy/engine/default.py", line 331, in do_execute cursor.execute(statement, parameters) sqlalchemy.exc.ProgrammingError: (ProgrammingError) syntax error at or near "stuff_id_seq" LINE 3: id INTEGER DEFAULT 'nextval('stuff_id_seq'::regclass)' NOT ... ^ "\nCREATE TABLE stuff (\n\tid INTEGER DEFAULT 'nextval('stuff_id_seq'::regclass)' NOT NULL, \n\tname VARCHAR(5), \n\tCONSTRAINT stuff_pkey PRIMARY KEY (id)\n)\n\n" {} }}}

Comments (8)

  1. Mike Bayer

    unfortunately Bitbucket's idiotic new UX just blew away all my comments here.

    So let's try again.

    This is a well known limitation of the Postgres dialect. The "nextval()" expression here ideally wouldn't be generated, and we'd know this is the default SERIAL case. But we don't get that info from PG.

    Sorry you went through all that for the repro case, this is easy to reproduce just by sticking a table on the "metadata", running the upgrade, then removing the table and creating another autogen, producing this code:

    def downgrade():
        ### commands auto generated by Alembic - please adjust! ###
        op.create_table(u'x',
        sa.Column(u'id', sa.INTEGER(), server_default="nextval('x_id_seq'::regclass)", nullable=False),
        sa.PrimaryKeyConstraint(u'id', name=u'x_pkey')
        )
        ### end Alembic commands ###
    

    The only solution here would be a hardcoded rule/regex in Alembic to have it completely guess that this exact pattern of default means the whole expression should be removed. Usually, any function-based default with PG needs the text() expression surrounding it in any case, so this kind of issue always requires that the user review and edit the migration file. But here, even if you put the text() around the expression so that it is no longer quoted, you still are missing the presence of the sequence itself.

    I've never been comfortable with SQLAlchemy making this guess. For Alembic, it's slightly less unacceptable, though I'd still consider placing a prominent warning or even an exception (to ensure the user reviews the file) so that these can be manually reviewed/corrected. Autogenerated revisions should always be reviewed and they make no guarantee that they can produce accurate CREATE statements based on reflection, since this cannot be done without guessing which is something SQLA/Alembic aren't too enthused about doing.

  2. Michael Armida

    No worries about the test case. It actually does exactly what you described - it adds a table, runs the upgrade, and then removes it.

    I'm confused about one thing (and this is more for curiosity than a solution): why is it that creating a table on an upgrade works fine, but not on a downgrade? Is it that on upgrade, the sequence is created implicitly, but on downgrade, alembic is trying to reference the sequence explicitly, in order to be faithful to the original sequence name?

    Even if the quoting issue is fixed and an independent sequence is created, the sequence is slightly different from the original in that it doesn't get dropped along with the table. But maybe that can be solved with some constraint (I am not very good with postgres, if you can't tell).

    Would a potentially hacky but pragmatic solution be to detect this scenario and emit the same creation statement in upgrades as downgrades? That is, to assume that postgres will do the right thing with implicit sequence creation?

  3. Mike Bayer

    when autogenerate generates the "upgrade" instruction, that's based on what you've given it as your in-application Table metadata. The Column/Integer/primary_key you've defined is there, and the PG dialect knows that should be generated as SERIAL.

    The "downgrade" instruction is completely different. autogenerate, via SQLAlchemy, reads the pg_catalog tables to infer information about existing tables, and for all those table names that aren't in your model, needs to reverse-engineer a Table structure out of it. This is an error prone process, and in this case PG doesn't tell us the column was created as SERIAL - it gives us integer along with the server default you see, and no information that there is also a SEQUENCE that needs to go along with it.

    the best solution here would be to just guess, in an educated way, that this "nextval(tablename_colname_seq)::regclass" default we're getting means, hey this is just SERIAL, we don't need to render this in our Python Table instruction.

  4. Log in to comment
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.