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

Improper UUID serialisation dropping leading zeros ▶️ "ValueError: badly formed hexadecimal UUID string" #25

Closed
8 tasks done
andrewbolster opened this issue Aug 26, 2021 · 7 comments · Fixed by #26, macrosfirst/sqlmodel#2, gnillev/sqlmodel#1 or spectacletheater/sqlmodel#2
Labels
bug Something isn't working

Comments

@andrewbolster
Copy link
Contributor

andrewbolster commented Aug 26, 2021

First Check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the SQLModel documentation, with the integrated search.
  • I already searched in Google "How to X in SQLModel" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to SQLModel but to Pydantic.
  • I already checked if it is not related to SQLModel but to SQLAlchemy.

Commit to Help

  • I commit to help with one of those options 👆

Example Code

import uuid
from typing import Optional

import sqlalchemy.exc
from sqlmodel import SQLModel, Field
from sqlmodel import create_engine, Session, select


class Hero(SQLModel, table=True):
    id: Optional[uuid.UUID] = Field(primary_key=True,
                                    default_factory=uuid.uuid4)
    name: str
    secret_name: str
    age: Optional[int] = None

    def __str__(self):
        return f'{self.name} (AKA {self.secret_name}) [{self.age if self.age is not None else "Unknown"}]'


sqlite_file_name = "database.db"
sqlite_url = f"sqlite:///{sqlite_file_name}"

engine = create_engine(sqlite_url, echo=True)  # remove echo for prod


def create_db_and_tables():
    try:
        Hero.__table__.drop(engine)
    except sqlalchemy.exc.OperationalError:  # table doesn't exist yet
        pass
    SQLModel.metadata.create_all(engine)


def create_heroes():
    heros = [
        Hero(name="Deadpond", secret_name="Dive Wilson"),
        Hero(name="Spider-Boy", secret_name="Pedro Parqueador"),
        Hero(name="Rusty-Man", secret_name="Tommy Sharp", age=48),
        Hero(name="Tarantula", secret_name="Natalia Roman-on", age=32),
        Hero(name="Black Lion", secret_name="Trevor Challa", age=35),
        Hero(name="Dr. Weird", secret_name="Steve Weird", age=36),
        Hero(name="Captain North America", secret_name="Esteban Rogelios", age=93),
    ]

    with Session(engine) as session:
        for hero in heros:
            session.add(hero)
        session.commit()


def select_heroes():
    with Session(engine) as session:
        for hero in session.exec(select(Hero)):
            print(hero.id)


if __name__ == "__main__":
    while True:
        try:
            create_db_and_tables()
            create_heroes()
            select_heroes()
        except ValueError:
            raise

Cleansed Traceback

Traceback (most recent call last):
  File “/…/sqlmodel_tutorial/minimal_example.py", line 89, in <module>
    select_heroes()
  File “/…/sqlmodel_tutorial/minimal_example.py", line 80, in select_heroes
    for hero in session.exec(select(Hero)):
  File "/.../lib/python3.8/site-packages/sqlalchemy/engine/result.py", line 381, in iterrows
    for row in self._fetchiter_impl():
  File "/.../lib/python3.8/site-packages/sqlalchemy/orm/loading.py", line 147, in chunks
    fetch = cursor._raw_all_rows()
  File "/.../lib/python3.8/site-packages/sqlalchemy/engine/result.py", line 392, in _raw_all_rows
    return [make_row(row) for row in rows]
  File "/.../lib/python3.8/site-packages/sqlalchemy/engine/result.py", line 392, in <listcomp>
    return [make_row(row) for row in rows]
  File "/.../lib/python3.8/site-packages/sqlalchemy/sql/type_api.py", line 1430, in process
    return process_value(value, dialect)
  File "/.../lib/python3.8/site-packages/sqlmodel/sql/sqltypes.py", line 61, in process_result_value
    value = uuid.UUID(value)
  File “/…/lib/python3.8/uuid.py", line 171, in __init__
    raise ValueError('badly formed hexadecimal UUID string')
ValueError: badly formed hexadecimal UUID string

Description

Running through the tutorial up until around this point, I tried swapping out the int id key for a UUID key.

This works except whenever the uuid.uuid4 default factory creates a UUID with a hexstring that starts with a zero.

It appears that here in sqltypes.py that, rather than using UUID().hex, sqltypes.py carries on sqlalchemy's integer stringification apprach, however, doesn't include any padding declaration.

IMO there are two options;

  1. faithfully fix the replication of SQLAlchemy's UUID ▶️ int ▶️ str approach with the padding fixed (f"{v.int:032x}")
  2. switch to using the stdlib UUID.hex implementation for serialisation

Personally I'm a fan of the latter so will spin up a PR

Operating System

macOS

Operating System Details

No response

SQLModel Version

0.0.4

Python Version

3.8.8

Additional Context

No response

@andrewbolster andrewbolster added the question Further information is requested label Aug 26, 2021
andrewbolster added a commit to andrewbolster/sqlmodel that referenced this issue Aug 26, 2021
Rather than integer based serialization grandfathered in from sqlalchemy, use the stdlib [`UUID.hex`](https://docs.python.org/3/library/uuid.html#uuid.UUID.hex) method.

This also fixes tiangolo#25
@alucarddelta
Copy link
Contributor

I am also hitting this issue with UUIDs being "badly formed".

@dit7ya
Copy link

dit7ya commented Sep 27, 2021

I am also getting this issue. A new release with the PR merged will be very helpful.

dit7ya added a commit to dit7ya/roamex that referenced this issue Sep 27, 2021
Using this fork until tiangolo/sqlmodel#25
is fixed in the master and a new release is done
dit7ya added a commit to dit7ya/roamex that referenced this issue Sep 27, 2021
Using this fork until tiangolo/sqlmodel#25
is fixed in the master and a new release is done

`poetry add git+ssh://git@github.com:macrosfirst/sqlmodel.git#main`
@chriswhite199
Copy link
Contributor

chriswhite199 commented Nov 29, 2021

For those awaiting a fix for this, here's a method to ensure the generated UUIDs don't start with leading zeros:

def new_uuid() -> uuid.UUID:
    # Note: Work around UUIDs with leading zeros: https://github.com/tiangolo/sqlmodel/issues/25
    # by making sure uuid str does not start with a leading 0
    val = uuid.uuid4()
    while val.hex[0] == '0':
        val = uuid.uuid4()

    return val


class UuidMixin(SQLModel):
    # amend index / primary_key as needed
    id: uuid.UUID = Field(index=False, primary_key=True, default_factory=new_uuid)

@frankie567
Copy link

Other solution: installing the version of @andrewbolster's PR:

pip install git+https://github.com/andrewbolster/sqlmodel.git@patch-1

@maxmouchet
Copy link

Another (temporary) solution, with SQLAlchemy-Utils:

from sqlalchemy_utils import UUIDType
from sqlmodel import Column, Field, SQLModel
from uuid import UUID, uuid4

class MyModel(SQLModel, table=True):
    uuid: UUID = Field(default_factory=uuid4, sa_column=Column(UUIDType(), primary_key=True))

Ae-Mc added a commit to Ae-Mc/climbing-app-backend that referenced this issue Jan 25, 2022
@mkarbo
Copy link

mkarbo commented Mar 16, 2022

@tiangolo is there any update on this?

leynier added a commit to leynier/drones that referenced this issue May 28, 2022
synodriver pushed a commit to synodriver/sqlmodel that referenced this issue Jun 24, 2022
@tiangolo
Copy link
Owner

Thanks for the report and fix in #26 @andrewbolster! 🍰

And thanks everyone for the discussion. ☕

The fix will be available in SQLModel 0.0.7, released in the next hours. 🎉

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