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

autogenerate postgresql DOMAIN type column #1357

Open
ovangle opened this issue Nov 16, 2023 · 2 comments
Open

autogenerate postgresql DOMAIN type column #1357

ovangle opened this issue Nov 16, 2023 · 2 comments

Comments

@ovangle
Copy link

ovangle commented Nov 16, 2023

Describe the bug
I have the following postgres domain in my model

# adapted from html5 standard
# https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/email#basic_validation
_POSTGRES_EMAIL_RE = (
    r"^"
    r"[a-z0-9.!#$%&''*+/=?^_`{|}~-]+"
    r"@[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?"
    r"(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)*"
    r"$"
)

EMAIL_DOMAIN = pg_dialect.DOMAIN(
    'email',
    pg_dialect.CITEXT(),
    check=rf"value ~ '{_POSTGRES_EMAIL_RE}'"
) 

class Base(DeclarativeBase):
    pass

class Contact(Base):
    __tablename__ = 'contacts'
    id: Mapped[int] = mapped_column(Integer, primary_key=True)
    email: Mapped[str] = mapped_column(EMAIL_DOMAIN)

When autogenerating the field, alembic outputs a create_table command like the following

    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table('contacts',
    sa.Column('id', sa.Integer(), nullable=False),
    sa.Column('email', postgresql.DOMAIN('email', CITEXT()), nullable=False),
    sa.PrimaryKeyConstraint('id')
    )
    # ### end Alembic commands ###

Notably,

  • CITEXT is not imported from the postgresql dialect and there a syntax error in the generated migration.
  • The check constraint is dropped in the generated DOMAIN definition

Fixing these and applying the upgrade creates the correct schema, however every subsequent attempt to autogenerate for the model results in spurious alter_column commands like the following:

op.alter_column('contacts', 'email',
               existing_type=postgresql.CITEXT(),
               type_=postgresql.DOMAIN('email', CITEXT()),
               existing_nullable=False)

In particular,

  • the existing_type is inferred as the underlying datatype rather than the domain
  • the generated DOMAIN type suffers from the same issues listed above

Expected behavior

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.

# Insert code here

Error

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

Versions.

  • OS: Ubuntu 22.04.3 LTS
  • Python: 3.11.2
  • Alembic: 1.12.0
  • SQLAlchemy: 2.0.21
  • Database: 15.4
  • DBAPI: psycopg[c]==3.1.12

Additional context

Have a nice day!

@ovangle ovangle added the requires triage New issue that requires categorization label Nov 16, 2023
@ovangle ovangle changed the title autogenerate reissues alter_columns for unchanged domain columns autogenerate postgresql DOMAIN type column Nov 16, 2023
@zzzeek
Copy link
Member

zzzeek commented Nov 16, 2023

this would be two separate bugs. the false positive is more urgent. the render is not as easy to implement and can always be worked around. I suggest splitting into two issues

@ovangle
Copy link
Author

ovangle commented Nov 16, 2023

Definitely agree. If the column changes were only generated when the model changed then it'd be no bother to edit the output manually. I originally wrote it that way, but couldn't really describe the bug without mentioning the templating issues.

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