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

support field max_length #857

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

proever
Copy link

@proever proever commented Mar 25, 2024

Currently, when max_length is specified in a Field, it has no real effect on the corresponding SQL schema (#126, #746). As pointed out by @chris-beedie, this is because the configured max_length is stored as an annotated_types.MaxLen, rather than a PydanticMetadata object, and is therefore ignored in get_field_metadata.

This PR adds a simple extra check for that specific case. I have confirmed it works using PostgreSQL 16.2.

I also wanted to write a test for it, along the following lines:

def test_field_max_length_is_enforced(clear_sqlmodel):
    class Hero(SQLModel, table=True):
        id: Optional[int] = Field(default=None, primary_key=True)
        name: str = Field(max_length=5)

    engine = create_engine("sqlite://")

    SQLModel.metadata.create_all(engine)

    with pytest.raises(IntegrityError):
        with Session(engine) as session:
            hero_1 = Hero(name="Deadpond")

            session.add(hero_1)
            session.commit()
            session.refresh(hero_1)

Unfortunately, this doesn't work, as sqlite does not actually enforce something like VARCHAR(5).

One solution would be to use a testcontainer (see here) to run a PostgreSQL instance and test against that, but it would significantly increase the complexity & dependencies of the unit tests.

@mbwhite
Copy link

mbwhite commented May 19, 2024

FYI _ stumbled across this trying to debug SQLModel using DB2. The create table SQL ends up using VARCHAR(None) that DB2 rejects; I tried the max_length that didn't work hence, came here.

Not sure if that helps but DB2 doesn't like unbounded varchars

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

Successfully merging this pull request may close these issues.

None yet

2 participants