Skip to content

Computed statements containing newlines cause warnings on autogenerate #1391

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

Closed
GeorchW opened this issue Jan 9, 2024 · 2 comments
Closed
Labels
autogenerate - detection use case not quite a feature and not quite a bug, something we just didn't think of

Comments

@GeorchW
Copy link

GeorchW commented Jan 9, 2024

Describe the bug
When using ORM code that contains multi-line computed statements, they always generate warnings when being fed into Alembic. Like this:

class MyModel(Base):
    ...
    whatever: Mapped[int] = mapped_column(Computed("""1
        + 2""")) # just a simple multi-line expression for demo purposes
    # makes more sense with longer expressions

Alembic will always complain with "UserWarning: Computed default on MyModel.start_holding cannot be modified".

Expected behavior
No warning should appear as long as the expression does not change.

To Reproduce

Please try to provide a Minimal, Complete, and Verifiable example, with the migration script and/or the SQLAlchemy tables or models involved.
See also Reporting Bugs on the website.

This is the model:

from sqlalchemy import Computed
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column


class Base(DeclarativeBase):
    pass


class MyModel(Base):
    __tablename__ = "mymodel"
    id: Mapped[int] = mapped_column(primary_key=True)
    whatever: Mapped[int] = mapped_column(Computed("""1
        + 2"""))  # just a simple multi-line expression for demo purposes
    # makes more sense with longer expressions

The reproduction does not work with sqlite, since it stores the SQL expressions verbatim, but the warning does appear for MySQL. Start a MySQL container:

docker run --rm -p 1234:3306 --env MYSQL_ALLOW_EMPTY_PASSWORD=1 --name tmp mysql

Then initialize alembic and create a first revision:

alembic init alembic

Set the correct URL in the alembic.ini file:

sqlalchemy.url = mysql+pymysql://root:@localhost:1234/mysql

Set the correct metadata in env.py:

...
import demo
target_metadata = demo.Base.metadata
...

Generate a first revision and upgrade the database:

alembic revision --autogenerate
alembic upgrade head

Now, when generating additional revisions, a warning will appear:

alembic revision --autogenerate

Error

venv/lib/python3.11/site-packages/alembic/autogenerate/compare.py:1037: UserWarning: Computed default on mymodel.whatever cannot be modified
  util.warn("Computed default on %s.%s cannot be modified" % (tname, cname))
  Generating ./alembic/versions/3b750bbd8b7c_.py ...  done

Versions.

  • OS: Linux
  • Python: 3.11.7
  • Alembic: 1.13.1
  • SQLAlchemy: 2.0.25
  • Database: MySQL 8.2.0
  • DBAPI: pymysql

Additional context

Phew, that was a lot of work to write for a two-line PR!

Have a nice day!

@GeorchW GeorchW added the requires triage New issue that requires categorization label Jan 9, 2024
GeorchW pushed a commit to GeorchW/alembic that referenced this issue Jan 9, 2024
@sqla-tester
Copy link
Collaborator

Georg Wicke-Arndt has proposed a fix for this issue in the main branch:

Ignore newlines in expressions for Computed https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5105

@CaselIT CaselIT added autogenerate - detection use case not quite a feature and not quite a bug, something we just didn't think of and removed requires triage New issue that requires categorization labels Jan 10, 2024
@CaselIT
Copy link
Member

CaselIT commented Jan 10, 2024

thanks for reporting

sorry for the process that may have a bit too much ceremony at times

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autogenerate - detection use case not quite a feature and not quite a bug, something we just didn't think of
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants