Repository navigation
Conversation
Author
|
The changes followed your style (both commits style and testing), however if you still don't like any AI usage, feel free to close the PR, I'll use different approach in my own client. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hey, I'm working on a BigQuery client that reads typed rows through the Storage Read API. It sends
BIGNUMERICasDecimal256(76, 38)andINTERVALasInterval(MonthDayNano), andfrom_record_batchfails on both today (Unsupported arrow data type Decimal256(76, 38)). The error is for the whole batch, even if the struct doesn't name these columns.This PR adds:
Decimal256, handled the same way asDecimal128. marrow gets the data type, array and view with ani256element type in the Arrow layout. serde_arrow has no arrow dependency outside the conversions, so the 256 bit parsing and formatting is a small helper next to the existing decimal code;Interval(MonthDayNano), mapped tojiff::Spanstrings as discussed in Add support for Interval types #178, plus structs withmonths,days,nanosecondsfields. A span has one sign, so intervals with mixed signs (BigQuery allows them) need the struct form.YearMonthandDayTimeare left out to keep it small.I also fixed the
filedtypo in the decimal builder error annotation, the builder is shared withDecimal256now. It touches the same files as #331 in different places, I checked that it merges cleanly and the serde_arrow tests pass with it.Full disclosure, I used AI to make these changes, in a responsible way and under my own specs and control. I do write my own code for a lot of things, but for features like this one, just adding new types and more mappings, it's easier to use AI nowadays. I'm not an antagonist of AI and use it responsibly.
Checklist
uv run python x.py precommitChanges.mdforserde_arrow,marrow/Changes.mdformarrow)