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

Nullable type #1602

Merged
merged 28 commits into from
Mar 20, 2024
Merged

Conversation

NotGyro
Copy link
Contributor

@NotGyro NotGyro commented Mar 6, 2024

Add an Option-like type to distinguish "This element is out-of-bounds" from "This element is a Postgres null", as well as genericizing over NullLayout types that mark where null elements are in a container (for example, the null bitslice on arrays).

This is meant to implement the behavior requested in #620 and #1586

For iterators, a separate pull request has been split off: NotGyro#2

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

Apologies, thoughts are incomplete and what is left over after sneezing my brains out all day. Wanted to post what I did have.

pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

I think it would be best to split things up so that we can land the Nullable type and the internal Array refactor and then work on extending the idea further in a followup.

pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
@NotGyro NotGyro marked this pull request as ready for review March 18, 2024 22:28
@workingjubilee workingjubilee changed the title Nullable traits Nullable type Mar 19, 2024
pgrx/src/nullable.rs Outdated Show resolved Hide resolved
pgrx/src/datum/array.rs Outdated Show resolved Hide resolved
@workingjubilee
Copy link
Member

As I mentioned while discussing this offsides, I have some other thoughts about the way the traits carve up the world that I think are better proposed as a refactoring since it's hard to really think them through in the review/talking format.

@workingjubilee workingjubilee merged commit cb67b17 into pgcentralfoundation:develop Mar 20, 2024
11 checks passed
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