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

sql: Add unsigned int types #14304

Merged
merged 2 commits into from
Aug 24, 2022
Merged

sql: Add unsigned int types #14304

merged 2 commits into from
Aug 24, 2022

Conversation

jkosh44
Copy link
Contributor

@jkosh44 jkosh44 commented Aug 19, 2022

This commit adds 16-bit, 32-bit, and 64-bit unsigned integer types to
Materialize. In addition it also adds various casts to and from the
unsigned int types.

The binary format of these types is big endian, which is the same as
every other numeric type. These types have separate OIDs from the
signed integer types so psql and other drivers don't confuse these for
signed integers.

No arithmetic operators are added for unsigned integers in this commit.
Those can be added in a follow up commit.

Works towards resolving MaterializeInc/database-issues#2366

Motivation

This PR adds a known-desirable feature.

Checklist

@jkosh44 jkosh44 force-pushed the unsigned branch 17 times, most recently from a4d1c2e to fa146be Compare August 24, 2022 14:04
This commit adds 16-bit, 32-bit, and 64-bit unsigned integer types to
Materialize. In addition it also adds various casts to and from the
unsigned int types.

The binary format of these types is big endian, which is the same as
every other numeric type. These types have separate OIDs from the
signed integer types so psql and other drivers don't confuse these for
signed integers.

No arithmetic operators are added for unsigned integers in this commit.
Those can be added in a follow up commit.

Works towards resolving #7629
@jkosh44 jkosh44 changed the title [WIP] Add unsigned types sql: Add unsigned int types Aug 24, 2022
@jkosh44 jkosh44 marked this pull request as ready for review August 24, 2022 15:10
@benesch
Copy link
Member

benesch commented Aug 24, 2022

Woohoo! I hope this isn't too much of a hassle, but IMO let's not add the goofy SQL standard names for these types (biguint, smalluint, uinteger). I think better just to stick with the PostgreSQL style names (uint2, uint4, uint8). That matches pguint AFAICT. Much easier to add aliases later than remove them.

@jkosh44
Copy link
Contributor Author

jkosh44 commented Aug 24, 2022

Woohoo! I hope this isn't too much of a hassle, but IMO let's not add the goofy SQL standard names for these types (biguint, smalluint, uinteger). I think better just to stick with the PostgreSQL style names (uint2, uint4, uint8). That matches pguint AFAICT. Much easier to add aliases later than remove them.

@benesch Shouldn't be too difficult. What about the aliases uint for uint4?

@benesch
Copy link
Member

benesch commented Aug 24, 2022

@benesch Shouldn't be too difficult. What about the aliases uint for uint4?

I'd err towards leaving that one out too!

Copy link
Member

@benesch benesch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an epic PR. LGTM.

@jkosh44 jkosh44 merged commit a8131a5 into MaterializeInc:main Aug 24, 2022
@jkosh44 jkosh44 deleted the unsigned branch August 24, 2022 20:18
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

Successfully merging this pull request may close these issues.

2 participants