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

FlatArray and Text #1856

Draft
wants to merge 54 commits into
base: develop
Choose a base branch
from

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented Sep 11, 2024

Introduce a new Rust type that serves as a definition of what Postgres ArrayTypes "really are", called FlatArray. This is meant to be manipulated via &FlatArray<'_, T>... it is an "unsized type". It uses BorrowDatum to power its simplicity in iteration, compared to the incredibly jank implementation in pgrx/src/datum/array.rs! I should know it's jank, because I wrote it!

The cost is that it is much more nitpicky about what it can work with, as for a given FlatArray<'_, T>, we also require BorrowDatum in order to do anything useful with it! This means FlatArray can't be, say, FlatArray<'mcx, String>, and implicitly unbox elements into allocated types. You can still just use .iter().map(), of course, and the good news is this means all FlatArrays must truly be "zero-copy", as the type definition doesn't allow anything else.

As part of this compromise, we introduce a new Text type so that it is still possible to work with common array needs like strings.

Together, these are effectively the first varlena types that allow directly interacting with the data in their varlena header instead of "forgetting" it.

@workingjubilee workingjubilee force-pushed the add-borrowing-datums-force branch 4 times, most recently from d1ba6a9 to 65d503d Compare September 28, 2024 21:02
@workingjubilee
Copy link
Member Author

workingjubilee commented Sep 28, 2024

This needs more tests, and I'll probably split it into components so that it can be reviewed in smaller chunks, but I'm satisfied with the fundamentals of the implementation here.

@workingjubilee workingjubilee marked this pull request as ready for review September 28, 2024 21:06
@workingjubilee workingjubilee changed the title BorrowDatum, FlatArray, and Text? BorrowDatum, FlatArray, and Text Sep 28, 2024
@workingjubilee workingjubilee changed the title BorrowDatum, FlatArray, and Text FlatArray and Text Oct 1, 2024
@workingjubilee workingjubilee marked this pull request as draft October 2, 2024 02:43
@workingjubilee
Copy link
Member Author

workingjubilee commented Oct 2, 2024

Details of Text's impl are unsound (or at least, hard to use soundly?). I am going to PR it separately with sufficient tests.

@workingjubilee
Copy link
Member Author

I think it mostly needs to be as pedantic about alignment as I was afraid of.

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.

1 participant