detect Postgresql SERIAL sequences from reflected nextval('table_id_seq'::regclass)

[sqlalchemy-bot]

Migrated issue, originally created by Michael Armida (@marmida)

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" {}

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

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:

#!python


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.

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• edited description

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• removed labels: bug
• added labels: feature

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "Autogenerate creates bad downgrade for dropped tab" to "More PG nextval. it needs the text() wrapper plus"

[sqlalchemy-bot]

Michael Armida (@marmida) wrote:

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?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

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.

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• edited description

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "More PG nextval. it needs the text() wrapper plus" to "More PG / postgresql nextval. it needs the text()"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

see also #111.

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• added labels: autogenerate - detection

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

another case, product_ids | text[] | not null default '{}'::text[] . see https://bitbucket.org/zzzeek/sqlalchemy/issue/3157/invalid-default-value-for-array.

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• removed labels: feature
• added labels: bug

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• removed labels: autogenerate - detection
• added labels: autogenerate - rendering

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "More PG / postgresql nextval. it needs the text()" to "generalized solution for rendering of Postgresql s"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

per #197 we now generate valid SQL:

sa.Column('id', sa.INTEGER(), server_default=sa.text("nextval('my_table_id_seq'::regclass)"), nullable=False),

however, this will fail unless you also add the CREATE SEQUENCE manually, or manually convert the server_default here to a SERIAL for Postgresql.

So the generalized solution is here. but the one that folds the SEQUENCE into SERIAL, is not, and that is still this issue.

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "generalized solution for rendering of Postgresql s" to "detect Postgresql SERIAL sequences from reflected "

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

• Added a routine by which the Postgresql Alembic dialect inspects
  the server default of INTEGER/BIGINT columns as they are reflected
  during autogenerate for the pattern nextval(<name>...) containing
  a potential sequence name, then queries pg_catalog to see if this
  sequence is "owned" by the column being reflected; if so, it assumes
  this is a SERIAL or BIGSERIAL column and the server default is
  omitted from the column reflection as well as any kind of
  server_default comparison or rendering, along with an INFO message
  in the logs indicating this has taken place. This allows SERIAL/BIGSERIAL
  columns to keep the SEQUENCE from being unnecessarily present within
  the autogenerate operation.
  fixes #73

→ ff74edf

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed