One translation into Arrow, shared with the engine - #31
Merged
Conversation
The seven hundred lines that turned a result into Arrow columns lived here, and another seven hundred of them lived in the JavaScript client, and the two had to agree about what a year-month duration is and what a node names itself. They now both export through zu-arrow in the engine tree, which is the one answer, so there is nowhere left for them to drift apart. What stays here is the three things a shared crate cannot know: which Python exception each refusal is, where this client keeps the table names, and how many rows a caller wanted in a batch. The last of those is new. record_batches takes rows_per_batch, the way DuckDB's fetch_record_batch does, for a consumer with a row group size in mind, and it costs nothing either way because a batch is a slice of an array that is already built rather than a copy of one. A result that matched no rows now says what its columns hold instead of typing them all as null, which is what a query writing Parquet or appending to a table that already exists needs from a run that happened to find nothing. The engine fills the column buffers during the scan now, so the numbers in the README were badly stale: three hundred thousand rows across three columns take 4.8 ms as Arrow against 86 ms as Python objects, where the README still claimed 44 against 67. The conformance runner also gets two fixes it needed to read the corpus at the new pin. The corpus writes a backspace and a form feed in a double quoted scalar and this reader knew neither, and it writes FINISH as a case with an empty columns list, which the reader refused as an unfinished sequence. Both were stopping the whole corpus from loading.
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.
This client had seven hundred lines that turned a result into Arrow columns, and the JavaScript client had another seven hundred, and the two had to agree about what a year-month duration is, what a node names itself and which times Arrow has no type for. They now both export through
zu-arrowin the engine tree, which is the one answer to that question, so there is nowhere left for them to drift apart. The engine pin moves from130f67dto0698a4eto pick it up, which is the same revision the Node client is on.What stays here is the three things a shared crate cannot know. Which Python exception each refusal is, so a value of the wrong type is still a
TypeErrorand one that will not fit is still aValueError. Where this client keeps the table names, which isNames, now offered to the translation through itsTablestrait and borrowed rather than cloned, because a column of a hundred million nodes is a hundred million lookups. And how many rows a caller wanted in a batch.That last one is new.
record_batchestakesrows_per_batch, the way DuckDB'sfetch_record_batchdoes, for a consumer that has a size in mind because its own downstream has one. It costs nothing either way: the arrays are built once and a batch is a slice of them, so the size decides how often a reader is called and nothing else. Zero is refused with the reason. The size cannot ride on__arrow_c_stream__, which takes no argument the protocol did not give it, so what goes toRecordBatchReader.from_streamis a small object holding a stream that was already cut to the size asked for.What changed for a caller
A result that matched no rows now says what its columns hold instead of typing them all as null. The type comes off the column rather than off the rows, so a query that found nothing can still be written to Parquet or appended to a table that already exists, which is what a consumer doing either of those wanted from it.
tests/test_arrow.pysays so under the name it always did.The numbers were stale
The engine fills the column buffers during the scan now rather than transposing the rows afterwards, so the README was claiming a cost that has not been paid for a while. Three hundred thousand rows across three columns take 4.8 ms as Arrow against 86 ms as Python objects, where the README said 44 against 67, and a single integer column takes 0.8 ms against 43 ms, where it said 13.8 against 44.5. A million rows in two columns as numpy take 9.9 ms against 11.9 ms for the Arrow table with
to_numpyon each column and 199 ms for the same rows as tuples. Batch size makes no measurable difference at 4096, 65536 or a million, which is what a slice costing nothing looks like from outside.The conformance runner
Two fixes it needed before it could read the corpus at all, both of which were stopping the whole file from loading rather than failing a case. The corpus writes a backspace and a form feed in a double quoted scalar and this reader's escape table had neither. And it writes
FINISHas a case with an emptycolumns:, which is a query that answers no columns rather than a query whose columns held no rows, and the reader was refusing it as a sequence somebody had left unfinished. With both fixed the corpus runs: 1035 cases, 1030 passed, 0 failed, 5 unsupported, all five being a local time written to the nanosecond that this client holds to the microsecond.Checks
pytestgreen, 737 passed and 5 skipped.ruff checkandruff format --checkclean.cargo fmt --all --checkclean,cargo clippy --all-targets -- -D warningsclean, and both of the other two ABI feature sets check. The conformance runner andtests/test_conformance.pyboth green against the corpus at the new pin.