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

Alembic treats output of custom render_item as compiled sql breaking server_default comparisons for custom funcs #1180

Closed
lukesmurray opened this issue Feb 16, 2023 · 2 comments

Comments

@lukesmurray
Copy link

Describe the bug

Alembic treats the output of render_item as both python code to be added to tables and compiled code to be rendered in sql statements.

When rendering a custom function type for a table we want the output to be something like app.migration_types.custom_func.

However internally that type of output will cause an error.
For example when alembic renders a field for comparison it uses the user_defined_render.

def _render_server_default_for_compare(
metadata_default: Optional[Any],
metadata_col: Column,
autogen_context: AutogenContext,
) -> Optional[str]:
rendered = _user_defined_render(
"server_default", metadata_default, autogen_context
)
Later in the postgres server_default_comparison, and maybe in others, the rendered item is compared in sql.

    return not self.connection.scalar(
        text(
            "SELECT %s = %s"
            % (conn_col_default, rendered_metadata_default)
        )
    )

This comparison is the source of the reported error. The output should be compiled sql but its just the python path to the user's function.

SELECT timezone('utc'::text, now()) = CustomUtcNow()
Currently trying to fix this with an overload for compare_server_default and it seems possible but I think this is a bit of a flaw in either the instructions for rendering custom types or the logic for comparing custom types.

Expected behavior

Default functionality should probably compare the compiled output to the rendered output.

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.

I don't have a MCV or stack trace. Just opening the issue to provide context since I ran into a similar issue as another user #641 (comment).
If one of the maintainers asks for an MCV I can provide.

# Insert code here

Error

# Copy error here. Please include the full stack trace.

Versions.

  • OS:
  • Python:
  • Alembic:
  • SQLAlchemy:
  • Database:
  • DBAPI:

Additional context

Have a nice day!

@lukesmurray lukesmurray added the requires triage New issue that requires categorization label Feb 16, 2023
@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

dont use server_default render_item for SQL compare https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4453

@zzzeek zzzeek added bug Something isn't working autogenerate - defaults autogenerate - detection and removed requires triage New issue that requires categorization labels Feb 16, 2023
@zzzeek
Copy link
Member

zzzeek commented Feb 16, 2023

thanks, I can just remove that code. feel free to test locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants