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

Drop/CreateTableOp.from_table() no longer copy the table's SchemaItem.info #879

Closed
cansjt opened this issue Jul 30, 2021 · 0 comments
Closed
Labels
bug Something isn't working op directives

Comments

@cansjt
Copy link
Contributor

cansjt commented Jul 30, 2021

Describe the bug

Since 1.6.0 it seems CreateTableOp.from_table() and DropTableOp.from_table() no longer copy the table's SchemaItem.info dictionary. The code of the methods substantially changed between 1.5.8 and 1.6.0

Expected behavior

The info dictionary should still contain the same data as the one of the original table.

Not sure if other bits of the code could be impacted (other SchemaItem loosing their info data)

To Reproduce

# Here is the shortest I could come up with
from sqlalchemy import create_engine, MetaData, Table
from sqlalchemy.ext.compiler import compiles
from sqlalchemy.schema import DropTable

from alembic.migration import MigrationContext
from alembic.operations import Operations
from alembic.operations.ops import DropTableOp


@compiles(DropTable)
def _add_if_exists(element, compiler, **kw):
    """Patches DROP TABLE statements to add IF EXISTS clause

    Shamelessly stolen from https://github.com/sqlalchemy/alembic/issues/151
    """
    assert element.element.info == THE_TABLE.info
    output = compiler.visit_drop_table(element, **kw)
    # if isinstance(element.element.info, dict) and element.element.info.get('ifexists'):
    #     output = re.sub(
    #         r"^\s*DROP TABLE", "DROP TABLE IF EXISTS", output, re.S)
    return output


THE_TABLE = Table('name', MetaData(), info={'ifexists': True})
operation = DropTableOp.from_table(THE_TABLE)

assert operation.to_table().info == THE_TABLE.info
engine = create_engine('sqlite://')
with engine.connect() as connection:
    ctx = MigrationContext.configure(connection)
    operations = Operations(ctx)
    operations.invoke(operation)

Actually we can do much shorter:

from sqlalchemy import create_engine, MetaData, Table

from alembic.operations.ops import DropTableOp
THE_TABLE = Table('name', MetaData(), info={'ifexists': True})
operation = DropTableOp.from_table(THE_TABLE)

assert operation.to_table().info == THE_TABLE.info

Or for a more comprensive test case:

import pytest
from sqlalchemy import create_engine, MetaData, Table

from alembic.operations.ops import DropTableOp, CreateTableOp


@pytest.mark.parametrize(('operation', ), (
   pytest.param(DropTableOp, id='drop table'),
   pytest.param(CreateTableOp, id='create table'),
))
def test_operations_copy_schema_item_info_dict(operation):
    # Given
    table = Table('name', MetaData(), info={'ifexists': True})

    # Then
    op = operation.from_table(table)

    # Then
    assert op.to_table().info == table.info

Error

Traceback (most recent call last):
  File "minimal.py", line 37, in <module>
    operations.invoke(operation)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/alembic/operations/base.py", line 354, in invoke
    return fn(self, operation)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/alembic/operations/toimpl.py", line 72, in drop_table
    operations.impl.drop_table(
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/alembic/ddl/impl.py", line 297, in drop_table
    self._exec(schema.DropTable(table))
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/alembic/ddl/impl.py", line 146, in _exec
    return conn.execute(construct, multiparams)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1011, in execute
    return meth(self, multiparams, params)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/sql/ddl.py", line 72, in _execute_on_connection
    return connection._execute_ddl(self, multiparams, params)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1062, in _execute_ddl
    compiled = ddl.compile(
  File "<string>", line 1, in <lambda>
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/sql/elements.py", line 481, in compile
    return self._compiler(dialect, bind=bind, **kw)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/sql/ddl.py", line 29, in _compiler
    return dialect.ddl_compiler(dialect, self, **kw)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/sql/compiler.py", line 322, in __init__
    self.string = self.process(self.statement, **compile_kwargs)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/sql/compiler.py", line 352, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/ext/compiler.py", line 443, in <lambda>
    lambda *arg, **kw: existing(*arg, **kw),
  File "~/.virtualenvs/dtp38/lib/python3.8/site-packages/sqlalchemy/ext/compiler.py", line 490, in __call__
    return fn(element, compiler, **kw)
  File "minimal.py", line 23, in _add_if_exists
    assert element.element.info == THE_TABLE.info
AssertionError

Works fine with Alembic up to 1.5.8

With the second example:

Traceback (most recent call last):
  File "minimal.py", line 31, in <module>
    assert operation.to_table().info == THE_TABLE.info
AssertionError

Versions.

  • OS: Debian 10
  • Python: 3.8 & 3.9
  • Alembic: 1.6.0 onwards up to 1.6.5
  • SQLAlchemy: 1.3.24
  • Database: PostgreSQL & SQLite
  • DBAPI: psycopg2 & Stdlib's SQLite DBAPI

Additional context

issue discovered because we use this to conditionally patch DROP|CREATE TABLE statements with IF EXISTS using this receipe (cf. commented code in the first code example).

Have a nice day!

Thanks, you too !

@cansjt cansjt added the requires triage New issue that requires categorization label Jul 30, 2021
@cansjt cansjt changed the title Drop/CreateTableOp.from_table() no longer copy the table's Schema.info Drop/CreateTableOp.from_table() no longer copy the table's SchemaItem.info Jul 30, 2021
cans added a commit to cans/alembic that referenced this issue Jul 31, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info`.

This commit fixes both issues.

Closes sqlalchemy#879.
cans added a commit to cans/alembic that referenced this issue Jul 31, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info`.

This commit fixes both issues.

Fixes sqlalchemy#879.
cans added a commit to cans/alembic that referenced this issue Jul 31, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info` attribute.

This commit fixes both issues.

Fixes sqlalchemy#879.
@CaselIT CaselIT added bug Something isn't working op directives and removed requires triage New issue that requires categorization labels Aug 1, 2021
cans added a commit to cans/alembic that referenced this issue Aug 4, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info` attribute.

This commit fixes both issues.

Fixes sqlalchemy#879.
cans added a commit to cans/alembic that referenced this issue Aug 4, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info` attribute.

This commit fixes both issues.

Fixes sqlalchemy#879.
cans added a commit to cans/alembic that referenced this issue Aug 4, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info` attribute.

This commit fixes both issues.

Fixes sqlalchemy#879.
cans added a commit to cans/alembic that referenced this issue Aug 4, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info` attribute.

This commit fixes both issues.

Fixes sqlalchemy#879.
cans added a commit to cans/alembic that referenced this issue Aug 5, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info` attribute.

This commit fixes both issues.

Fixes sqlalchemy#879.
cans added a commit to cans/alembic that referenced this issue Aug 5, 2021
The `DropTableOp.from_table()` method loses the table's
`SchemaItem.info` and `comment` attributes while the
`CreateTableOp` only loses the `info` attribute.

This commit fixes both issues.

Fixes sqlalchemy#879.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working op directives
Projects
None yet
Development

No branches or pull requests

2 participants