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

Autogenerated postgresql.ExcludeConstraint() escaping #478

Closed
sqlalchemy-bot opened this Issue Jan 22, 2018 · 5 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Jan 22, 2018

Migrated issue, originally created by jahs (@jahs)

There appears to be an inconsistency within SQL Alchemy which autogenerated ExcludeConstraint migrations trigger, and an easy workaround. This is only seen for column names that need escaping, such as those containing upper case characters.

I'm using: alembic==0.9.7, SQLAlchemy==1.2.1

Adding an ExcludeConstraint to a Table results in a migration having a line such as:

postgresql.ExcludeConstraint(('"JB_NAM_JB_ID"', '='),
                             ('"JB_NAM_During"', '&&'),
                             using='gist', name='hkJB_NAM_Job_Name')

which appears to be correct following the ExcludeConstraint documentation, namely that the column names should be raw, escaped SQL strings.

However, at sqlalchemy/sql/schema.py(2668)_set_parent() we have

2665 	    def _set_parent(self, table):
2666 	        for col in self._pending_colargs:
2667 	            if isinstance(col, util.string_types):
2668 ->	                col = table.c[col]
2669 	            self.columns.add(col)

and so we get a KeyError as col is assumed not to be escaped.

Following the suggestion in the ExcludeConstraint docs, wrapping the column names in Column() rather than double quotes fixes it:

postgresql.ExcludeConstraint((sa.Column('JB_NAM_JB_ID'), '='),
                             (sa.Column('JB_NAM_During'), '&&'),
                             using='gist', name='hkJB_NAM_Job_Name')

This seems to be a better fix than trying to unescape in SQL Alchemy.

Many thanks, and thank you for Alembic.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 22, 2018

Michael Bayer (@zzzeek) wrote:

OK....I had to guess on the detail here, but apparently when you properly use the table Column in the model:

t = sa.Table(
    't', m,
    sa.Column('XColumn', sa.String),
    sa.Column('YColumn', sa.String),
)
ExcludeConstraint(
    (t.c.XColumn, ">"),
    using="gist",
    where='"XColumn" != 2',
    name="TExclX"
)

it still generates the string:

    op.create_table('t',
    sa.Column('XColumn', sa.String(), nullable=True),
    sa.Column('YColumn', sa.String(), nullable=True),
    postgresql.ExcludeConstraint((u'"XColumn"', '>'), where=sa.text(u'"XColumn" != 2'), using='gist', name='TExclX')
    )

fine, we use column().

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 22, 2018

Michael Bayer (@zzzeek) wrote:

this is in https://gerrit.sqlalchemy.org/#/c/zzzeek/alembic/+/640. if you'd like, let me know if you can verify this fixes what you need, as I only have limited testing on this end for this case. thanks!

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 22, 2018

Michael Bayer (@zzzeek) wrote:

Render ExcludeContraint Column as column, not plain string

Fixed bug where autogenerate of :class:.ExcludeConstraint
would render a raw quoted name for a Column that has case-sensitive
characters, which when invoked as an inline member of the Table
would produce a stack trace that the quoted name is not found.
An incoming Column object is now rendered as sa.column('name').

Change-Id: Ic84fc0b0fbaa5816ece1944043cd01a653bfe4ce
Fixes: #478

70dfb13

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 22, 2018

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jan 23, 2018

jahs (@jahs) wrote:

Perfect, yes this fixes it. Thank you very much.

@sqlalchemy-bot sqlalchemy-bot added the bug label Nov 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment