Skip to content

.tometadata() fails to copy ExcludeConstraints with column-based expressions #5850

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.

Sign up for GitHub

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

Jump to bottom
Closed
gordthompson opened this issue Jan 18, 2021 · 4 comments
Closed

.tometadata() fails to copy ExcludeConstraints with column-based expressions #5850

gordthompson opened this issue Jan 18, 2021 · 4 comments
Assignees
@gordthompson
Labels
bug Something isn't working postgresql schema things related to the DDL related objects like Table, Column, CreateIndex, etc.
Milestone
1.3.x

Comments

@gordthompson
Copy link
Member

(From https://groups.google.com/g/sqlalchemy/c/4wgEadxCkyk)

We noticed that the copying of ExcludeConstraints as part of a tometadata invocation fails for constraints with column-based expressions. The copy implementation of ExcludeConstraint passes the strname's of its columns to the construction of the new constraint, which is fine for regular columns, but not for more complex expressions.

We've added a small test case (in test_metadata) that reproduces the failure below.

We also implemented a temporary solution that seems to fix the issue by passing copies of the constraint's column expressions instead.

We are on version 1.3.18.

Temporary solution:

#elements = [(_copy_expression(col_expr, self.table, kw['target_table']), self.operators[col]) for col, col_expr in zip(self.columns.keys(), self.columns.values())]
        elements = [(col, self.operators[col]) for col in self.columns.keys()]

Test case

from sqlalchemy.dialects.postgresql import ExcludeConstraint
from sqlalchemy import column, Date, literal_column

class ToMetaDataExcludeConstraint(fixtures.TestBase, ComparesTables):
    @testing.requires.check_constraints
    def test_copy(self):
        from sqlalchemy.testing.schema import Table

        meta = MetaData()

        table = Table(
            "mytable",
            meta,
            Column("myid", Integer, Sequence("foo_id_seq"), primary_key=True),
            Column("valid_from_date", Date(), nullable=True),
            Column("valid_thru_date", Date(), nullable=True),
            ExcludeConstraint(
                (literal_column("daterange(valid_from_date, valid_thru_date, '[]')"), '&&'),
                where=column("valid_from_date") <= column("valid_thru_date"),
                name='ex_mytable_valid_date_range',
                deferrable=True, initially='deferred'
            )
        )

        meta.create_all(testing.db)
       
        try:
            meta2 = MetaData()
            table_c = table.tometadata(meta2)
        finally:
            meta.drop_all(testing.db)

Here's the stacktrace of the test case's failure:

=============================================== FAILURES ================================================
_________________________________ ToMetaDataExcludeConstraint.test_copy _________________________________
Traceback (most recent call last):
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/sql/test_metadata.py", line 5238, in test_copy
    table_c = table.tometadata(meta2)
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/sql/schema.py", line 1063, in tometadata
    table.append_constraint(
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/sql/schema.py", line 867, in append_constraint
    constraint._set_parent_with_dispatch(self)
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/sql/base.py", line 463, in _set_parent_with_dispatch
    self._set_parent(parent)
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/sql/schema.py", line 3065, in _set_parent
    ColumnCollectionMixin._set_parent(self, table)
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/sql/schema.py", line 3022, in _set_parent
    for col in self._col_expressions(table):
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/sql/schema.py", line 3016, in _col_expressions
    return [
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/sql/schema.py", line 3017, in <listcomp>
    table.c[col] if isinstance(col, util.string_types) else col
  File "/home/jeffh/workspace/vortex-workspace/vfinance/subrepos/SQLAlchemy-1.3.18/test/../lib/sqlalchemy/util/_collections.py", line 194, in __getitem__
    return self._data[key]
KeyError: "daterange(valid_from_date, valid_thru_date, '[]')"
======================================== short test summary info ========================================
FAILED test/sql/test_metadata.py::ToMetaDataExcludeConstraint::test_copy - KeyError: "daterange(valid_...
@gordthompson gordthompson added the requires triage New issue that requires categorization label Jan 18, 2021
@zzzeek zzzeek added bug Something isn't working schema things related to the DDL related objects like Table, Column, CreateIndex, etc. and removed requires triage New issue that requires categorization labels Jan 18, 2021
@zzzeek zzzeek added this to the 1.3.x milestone Jan 18, 2021
@sqla-tester
Copy link
Collaborator

Gord Thompson has proposed a fix for this issue in the master branch:

Fix .to_metadata() not copying some ExcludeConstraints https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2509

@gordthompson gordthompson self-assigned this Jan 25, 2021
@zzzeek
Copy link
Member

zzzeek commented Jan 29, 2021

why are ad-hoc column() objects being used in the ExcludeConstraint and not the actual Column objects bound to the Table ? is that part of the problem here? That would look like a more general class of problem.

@zzzeek
Copy link
Member

zzzeek commented Jan 29, 2021

plain UniqueConstraint etc have a slight problem with that as well, will make that work too, even though the default constraints aren't normally used in this way.

@sqla-tester
Copy link
Collaborator

Gord Thompson has proposed a fix for this issue in the rel_1_3 branch:

Use schema._copy_expression() fully in column collection constraints https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2522

sqlalchemy-bot pushed a commit that referenced this issue Jan 29, 2021
@gordthompson @zzzeek
Use schema._copy_expression() fully in column collection constraints
7dd3381 
Fixed issue where using :meth:`_schema.Table.to_metadata` (called
:meth:`_schema.Table.tometadata` in 1.3) in conjunction with a PostgreSQL
:class:`_postgresql.ExcludeConstraint` that made use of ad-hoc column
expressions would fail to copy correctly.

Fixes: #5850
Change-Id: I062480afb23f6f60962b7b55bc93f5e4e6ff05e4
(cherry picked from commit 81896c31ffc4db081f1f2bba199a52328398a236)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gordthompson gordthompson

Labels
bug Something isn't working postgresql schema things related to the DDL related objects like Table, Column, CreateIndex, etc.
Projects
None yet
Milestone
1.3.x
Development

No branches or pull requests
3 participants
@zzzeek @sqla-tester @gordthompson