Computed column migration file generated incorrectly

[deuce2367]

SQLAlchemy version 1.3.11 (just released on November 11th, 2019) includes support for Computed columns. From their documentation an example Computed column definition would be something like:

Column("area", Integer, Computed("side * side"))

After adding such a Computed column definition to my table model and trying to use alembic to autogenerate a migration file I encounter the following error:

    resolved_id, branch_label = self._resolve_revision_number(id_)
  File "/home/dev.internal/developer/sandbox/mbs/venv/lib/python3.7/site-packages/alembic/script/revision.py", line 501, in _resolve_revision_number
    self._revision_map
  File "/home/dev.internal/developer/sandbox/mbs/venv/lib/python3.7/site-packages/alembic/util/langhelpers.py", line 230, in __get__
    obj.__dict__[self.__name__] = result = self.fget(obj)
  File "/home/dev.internal/developer/sandbox/mbs/venv/lib/python3.7/site-packages/alembic/script/revision.py", line 123, in _revision_map
    for revision in self._generator():
  File "/home/dev.internal/developer/sandbox/mbs/venv/lib/python3.7/site-packages/alembic/script/base.py", line 112, in _load_revisions
    script = Script._from_filename(self, vers, file_)
  File "/home/dev.internal/developer/sandbox/mbs/venv/lib/python3.7/site-packages/alembic/script/base.py", line 906, in _from_filename
    module = util.load_python_file(dir_, filename)
  File "/home/dev.internal/developer/sandbox/mbs/venv/lib/python3.7/site-packages/alembic/util/pyfiles.py", line 98, in load_python_file
    module = load_module_py(module_id, path)
  File "/home/dev.internal/developer/sandbox/mbs/venv/lib/python3.7/site-packages/alembic/util/compat.py", line 173, in load_module_py
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 724, in exec_module
  File "<frozen importlib._bootstrap_external>", line 860, in get_code
  File "<frozen importlib._bootstrap_external>", line 791, in source_to_code
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/dev.internal/developer/sandbox/mbs/migrations/versions/9396eb21e075_tip_add_user_index_generated_user_.py", line 21
    op.add_column('usertable', sa.Column('user', sa.String(length=36), server_default=Computed(<sqlalchemy.sql.elements.TextClause object at 0x7febef2d30f0>, persisted=False), nullable=True))

I found that that if I hacked in a str() implementation to the Computed class definition in SQLAlchemy I was able to use alembic successfully however I'm not sure if that's the most elegant solution.

I'm using alembic version 1.3.0, sqlalchemy 1.3.11 (and MySQL 8.0.17 although I don't believe that matters as far as this issue is concerned).

I really appreciate any help or suggestions!

[zzzeek]

well yes, thanks for pointing that out, alembic needs a renderer for the compute construct.

[zzzeek]

@CaselIT any interest here?

[CaselIT]

I can take a look, but I won't have much time in the next couple of weeks.

[provinzio]

I am in need of a quick dirty fix, so I made one. Might me usefull for somebody :)
The SQL Query might include quotes, so I am using """-quotes in sa.text to avoid problems.

in alembic/autogenerate/render.py edit:

def _render_column(column, autogen_context):
    [...]
    opts = []
    **if column.computed:
        opts.append(('server_default', f'sa.Computed(sa.text("""{column.server_default.sqltext}"""), persisted={column.server_default.persisted})'))
    elif column.server_default:**
        rendered = _render_server_default(
            column.server_default, autogen_context
        )
    [...]

[CaselIT]

I've started trying to solve this in #631

@zzzeek I think that without support for computed in sqlalchemy inspection it will not be possible to implement a version that is 100% working

[zzzeek]

I've started trying to solve this in #631

@zzzeek I think that without support for computed in sqlalchemy inspection it will not be possible to implement a version that is 100% working

OK...usually we start with the "render only" model, that is, if someone has computed columns in their Table metadata and they just generate a plain create_table(), the computed shows up. that's the basic level of functionality. I'm assuming that you don't see any issue with this part proceeding.

the next level is support of changes in "computed", which has to do with reading the table columns in the DB and reading them in the model and comparing, and I'm assuming this is what you're referring towards. we don't implement this feature for CHECK constraints either right now as well as for expression-based indexes and lots of other things. this is not a critical feature though, it only means that we aren't detecting if someone changed the expression in their computed column. we would need to support ALTER COLUMN that changes that for each target DB also.

[CaselIT]

the next level is support of changes in "computed"

The problem is that the computed column is loaded as a normal default. so the case must be handled in alembic, even to just say "no change here"

[CaselIT]

I'll omit the support for changes for now

[zzzeek]

is this...in compare_server_default things are going wrong?

[CaselIT]

it seems that currently sqlalchemy when inspecting a computed column sets the server default to the normal text of the generated alwayas,

-- table in db
create table foo (
  bar indeger,
  foo integer GENERATED ALWAYS AS (bar + 1) STORED
)
-- is inspected as 
create table foo (
  bar indeger,
  foo integer DEFAULT (bar + 1)
)

I've just tried mysql and it does not have this behavior

[zzzeek]

so interesting that it even comes up.

the server default comparison logic needs to look at the metadata-side column; if there's a "computed", ignore the whole thing and report no change.

[CaselIT]

check the comment #631 (comment) in the pr, I've tried to explain it a bit better