piccolo icon indicating copy to clipboard operation
piccolo copied to clipboard

Schema generation assumes incorrect default values for UUID columns

Open Keating950 opened this issue 1 year ago • 1 comments

Summary

Database: PostgreSQL 16 Version: 1.17.0

UUID columns generated by running piccolo schema generate have incorrect default values. This appears to be because piccolo.columns.column_types.UUID assumes that UUID columns with defaults are generated by a database-specific built-in function. This results in incorrect default values being generated.

Minimal reproducible example

Schema definition

-- Source: https://gist.github.com/kjmph/5bd772b2c2df145aa645b837da7eca74
create or replace function uuid_generate_v7()
returns uuid
as $$
begin
  -- use random v4 uuid as starting point (which has the same variant we need)
  -- then overlay timestamp
  -- then set version 7 by flipping the 2 and 1 bit in the version 4 string
  return encode(
    set_bit(
      set_bit(
        overlay(uuid_send(gen_random_uuid())
                placing substring(int8send(floor(extract(epoch from clock_timestamp()) * 1000)::bigint) from 3)
                from 1 for 6
        ),
        52, 1
      ),
      53, 1
    ),
    'hex')::uuid;
end
$$
language plpgsql
volatile;

create table example (
    id uuid primary key default gen_uuid_v7()
);

Generated table definition

from piccolo.table import Table
from piccolo.columns.column_types import Serial
from piccolo.columns.column_types import Timestamp
from piccolo.columns.column_types import UUID
from piccolo.columns.column_types import Varchar
from piccolo.columns.defaults.timestamp import TimestampNow
from piccolo.columns.defaults.uuid import UUID4
from piccolo.columns.indexes import IndexMethod


class Example(Table, tablename="example"):
    id = UUID(
        default=UUID4(),  # incorrect: Translates to gen_random_uuid on the server
        null=False,
        primary_key=True,
        unique=False,
        index=True,
        index_method=IndexMethod.btree,
        db_column_name=None,
        secret=False,
    )


class Migration(Table, tablename="migration"):
  ...  # etc

Keating950 avatar Aug 22 '24 17:08 Keating950

Thanks for reporting this, and the detailed description.

With UUID columns, this is what we use for default values:

https://github.com/piccolo-orm/piccolo/blob/e14b8a567b44028db9959bd742871fbb7fda4ee9/piccolo/columns/defaults/uuid.py#L8-L22

If generated on the database side, it uses uuid_generate_v4(), and if on the Python side it uses uuid.uuid4().

You could work around it by having your own default value:

class UUID7(UUID4):
    @property
    def postgres(self):
        return "uuid_generate_v7()"

    def python(self):
        # assuming this is installed: https://github.com/stevesimmons/uuid7
        from uuid_extensions import uuid7
        return uuid7()

The only problem at the moment is we validate the defaults for a column - so we need to get this merged in:

https://github.com/piccolo-orm/piccolo/pull/1052/

dantownsend avatar Aug 23 '24 09:08 dantownsend