-
Notifications
You must be signed in to change notification settings - Fork 96
1280 UUID v7 support #1284
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
base: master
Are you sure you want to change the base?
1280 UUID v7 support #1284
Conversation
piccolo/columns/defaults/uuid.py
Outdated
| # Supported? | ||
| return self.postgres |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cockroach doesn't have UUID 7 support yet (as far as I can see).
I guess we can just raise NotImplemented for now.
| def postgres(self): | ||
| return "uuid_generate_v4()" | ||
| """ | ||
| Historically we had to use `uuid_generate_v4()` from the `uuid-ossp` | ||
| extension. | ||
| Since Postgres 13 there is a built-in `gen_random_uuid` function which | ||
| generates UUID v4 values. | ||
| In Postgres 18, `uuidv4` was added, which is the same as | ||
| `gen_random_uuid`, but more precisely named. We will move to this at | ||
| some point in the future. | ||
| """ | ||
| return "gen_random_uuid()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to split these changes out into a separate PR.
|
@dantownsend Just one question. How would you solve this Postgres error without having to install some |
I was going to just put in the docs that it's Postgres 18 only for now. I think there are some scripts out there somewhere which would allow us to use uuid 7 on older versions. If you can find one I'm not averse to using it (it might also solve the problem for CockroachDB too). |
|
I tried this custom function and by adding this function I was able to pass the tests. I don't know if this is the correct way to use it, but I added that custom function to postgres.py like this if current_transaction:
await current_transaction.connection.execute(
"""
-- Source: https://gist.github.com/kjmph/5bd772b2c2df145aa645b837da7eca74
create or replace function uuidv7()
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;
"""
)
response = await current_transaction.connection.fetch(ddl)I also tried those changes in the app using |
Thanks for this. I'm still a bit undecided how deep to go on this. Maybe we add this to the docs if people want to use UUID7 on older versions of Postgres. |
|
Here is another example of a custom Postgres uuid v7 function (from Piccolo issues). What I like about |
Resolves #1280