xzkostyan / clickhouse-sqlalchemy

ClickHouse dialect for SQLAlchemy
https://clickhouse-sqlalchemy.readthedocs.io
Other
428 stars 126 forks source link

_reflect_table() error on migration autogeneration #262

Open ObsidianDestroyer opened 1 year ago

ObsidianDestroyer commented 1 year ago

Describe the bug So, I have created a model for my table in Clickhouse:

from __future__ import annotations

from sqlalchemy.schema import Column

from clickhouse_sqlalchemy import types, engines
from collector.common.database.models import BaseModel

class TrackingPositionChangesOrdo(BaseModel):
    __tablename__ = 'TrackingPositionChangesOrdo'
    id = Column(types.UInt64, primary_key=True)
    srid = Column(types.String)
    user_id = Column(types.String)
    status_id = Column(types.UInt16)

    __table_args__ = (
        engines.MergeTree(
            partition_by=srid,
            key='srid',
        ),
    )

The BaseModel for my table models is defined like:

from sqlalchemy.schema import MetaData

from clickhouse_sqlalchemy.sql.schema import Table
from clickhouse_sqlalchemy.ext.declarative import get_declarative_base
from tricky.typing import String

from ..engine import engine

metadata = MetaData(bind=engine)

Base = get_declarative_base(metadata=metadata)

class BaseModel(Base):
    __name__: String
    __table__: Table
    __tablename__: String

    metadata: MetaData

    __abstract__ = True
    __selectable__ = None

With the models above i'm trying to run alembic with following command:

 clear && alembic revision --autogenerate -m "Initial migrations"

The result:

...
  File "/home/walther/.cache/pypoetry/virtualenvs/features-collector-NOfOGNwm-py3.10/lib/python3.10/site-packages/clickhouse_sqlalchemy/alembic/comparators.py", line 118, in compare_mat_view
    _reflect_table(inspector, table, None)
TypeError: _reflect_table() takes 2 positional arguments but 3 were given

What I'm expected after going to source code :

...
        if inner_name:
            drop = operations.DropMatViewToTableOp(
                name, params.as_select, inner_name
            )
        else:
            table = Table(name, existing_metadata)
            _reflect_table(inspector, table, None)

            drop = operations.DropMatViewOp(
                name, params.as_select, params.engine_full, *table.columns
            )

        upgrade_ops.ops.append(drop)
...

The problem is placed in line 118 of file (clickhouse_sqlalchemy/alembic/comparators.py), thats true that is _reflect_table function takes only 2 arguments, but 3 given. Source file path: alembic/util/sqla_compat.py

def _reflect_table(inspector: Inspector, table: Table) -> None:
    if sqla_14:
        return inspector.reflect_table(table, None)
    else:
        return inspector.reflecttable(  # type: ignore[attr-defined]
            table, None
        )

So in file where _reflect_table() (clickhouse_sqlalchemy/alembic/comparators.py) function is using I'm just removed third argument and it's started working, i've got migration what I expected.

To Reproduce Sorry, that I'm not giving a code to reproduce the problem, but as I think - I gave enough code and description of problem.

Expected behavior I was expected that migration will be created with a generated code which contains commands to create table by defined model.

Versions

ObsidianDestroyer commented 1 year ago

A temporary solution for every one who got the same (works for me) error which defined in above of env.py(alembic generated file) :

import types
import functools

def patch_reflect_table(function: t.Callable | types.FunctionType):
    _copy = types.FunctionType(
        function.__code__,
        function.__globals__,
        name=function.__name__,
        argdefs=function.__defaults__,
        closure=function.__closure__
    )
    _copy = functools.update_wrapper(_copy, function)

    def wrapper(inspector: Inspector, table: Table, redundant_arg: None) -> None:
        return _copy(inspector, table)

    return wrapper

comparators._reflect_table = patch_reflect_table(comparators._reflect_table)