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

Size of ULID is 20 bytes not 16 as expected #37

Open
etylermoss opened this issue Feb 29, 2024 · 9 comments
Open

Size of ULID is 20 bytes not 16 as expected #37

etylermoss opened this issue Feb 29, 2024 · 9 comments
Labels
C: type Context: Type D2: medium Difficulty: Medium to implement T: bug Type: Bug fix U1: want to have Urgency: Want to have Z: help wanted Z: Need help

Comments

@etylermoss
Copy link

I'm interfacing with the JVM / JDBC, and when I call ResultSet.getBytes(col), I am getting an exception because the column is not 16 bytes.

I confirmed this is the case by executing the below command in postgres:

select gen_ulid(), pg_column_size(gen_ulid()), gen_random_uuid(), pg_column_size(gen_random_uuid())
pg_column_size(gen_ulid()) pg_column_size(gen_random_uuid())
20 16

Any idea how to resolve this?

@pksunkara
Copy link
Owner

That is weird. I have u128 all over in my code and nowhere do I store it as 20 Bytes.

@pksunkara pksunkara added the Z: help wanted Z: Need help label Feb 29, 2024
@etylermoss
Copy link
Author

This may be an issue related to JDBC or casting operator, I just debugged the ResultSet.getBytes(col), and the resulting ByteArray is 26 bytes long corresponding to the ULID string. Although I don't think that explains why pg_column_size is 20 and not 16.

@pksunkara
Copy link
Owner

pksunkara commented Feb 29, 2024

The 26 bytes in the result is correct. But yeah, I'm unsure of why pg_column_size is 20.

@etylermoss
Copy link
Author

Perhaps pg_column_size includes padding / alignment?

Anyway, I have found the source of the issue, the custom type needs to be registered by the JDBC driver: pgjdbc/pgjdbc#2554.

I will update once I have a solution for anyone else who encounters this.

@etylermoss
Copy link
Author

@pksunkara can you think of anything that may need to be done on the database to support transfer? I'm new to PG so unsure exactly what I'm looking for - but I'm debugging the JDBC driver and it looks like the raw data being received is already serialized when it comes in. Do you know if the driver / connection needs to request the data be sent in a binary format; or if such a configuration is done for the type itself on database side?

@pksunkara
Copy link
Owner

such a configuration is done for the type itself on database side

Yeah, it's a feature on extension side. We store the data in a space efficient way, but we show the data to the user in a readable way.

Unfortunately, I don't know what else I can do to help you debug. #27 might be related though. You can try asking around in https://discord.gg/PMrpdJsqcJ which is the framework for Postgres Extensions I use.

@arjunyel
Copy link

arjunyel commented May 9, 2024

I created my ulids client using the JS library https://www.npmjs.com/package/ulidx, my pg_column_size is coming back as 17

@elektro-wolle
Copy link

It's caused by an internal representation in postgresql as varlena: 4 bytes for the length and 16 bytes for the data.
Root cause: in the pgrx-sql-entity-graph crate, the sql for the type is rendered in src/postgres_type/entity.rs as

CREATE TYPE ulid (
	INTERNALLENGTH = variable,
	INPUT = ulid_in, /* ulid::ulid_in */
	OUTPUT = ulid_out, /* ulid::ulid_out */
	STORAGE = extended
);

But it should be

CREATE TYPE ulid (
        INTERNALLENGTH = 16,
        INPUT = ulid_in, /* ulid::ulid_in */
        OUTPUT = ulid_out, /* ulid::ulid_out */
        STORAGE = plain
);

@pksunkara
Copy link
Owner

@elektro-wolle Thank you. Really appreciate it.

Looks related to pgcentralfoundation/pgrx#143. @workingjubilee Should PostgresType support a length derive property to support this?

@pksunkara pksunkara added the T: bug Type: Bug fix label Jun 4, 2024
@pksunkara pksunkara pinned this issue Jun 4, 2024
@pksunkara pksunkara added D2: medium Difficulty: Medium to implement U1: want to have Urgency: Want to have C: type Context: Type labels Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: type Context: Type D2: medium Difficulty: Medium to implement T: bug Type: Bug fix U1: want to have Urgency: Want to have Z: help wanted Z: Need help
Projects
None yet
Development

No branches or pull requests

4 participants