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

Schema generation assumes incorrect default values for UUID columns #1070

Open
Keating950 opened this issue Aug 22, 2024 · 1 comment
Open

Comments

@Keating950
Copy link

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
@dantownsend
Copy link
Member

dantownsend commented Aug 23, 2024

Thanks for reporting this, and the detailed description.

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

class UUID4(Default):
@property
def postgres(self):
return "uuid_generate_v4()"
@property
def cockroach(self):
return self.postgres
@property
def sqlite(self):
return "''"
def python(self):
return uuid.uuid4()

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:

#1052

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

No branches or pull requests

2 participants