maltzsama commented on issue #96:
URL: https://github.com/apache/arrow-swift/issues/96#issuecomment-5085102959
Agreed that Archery is where this needs to end up, and your guessed
interface looks right. Checking
`dev/archery/archery/integration/tester_rust.py`, the Rust tester runs exactly
that shape — `[exe, '--integration', '--arrow=X', '--json=Y',
'--mode=VALIDATE']` — plus `JSON_TO_ARROW` and `ARROW_TO_JSON` modes and two
separate binaries for stream/file conversion.
My hesitation is about sequencing rather than direction. The JSON files
Archery compares against live in `arrow-testing`, in the same directory as the
gold IPC files, so whatever makes that corpus available to the test suite is a
prerequisite for the Archery work rather than a parallel track. Today there is
no such mechanism: the suite generates its own fixtures with a Go program under
`data-generator/`, producing three cases — bool+string, float64+string, and a
two-field struct — which are the only external data the reader is ever checked
against.
That is what #185 addresses. It adds `arrow-testing` as a submodule,
resolves it through `ARROW_TEST_DATA` with a fetch fallback so RC verification
still works from a tarball, and adds conformance tests for both the file and
stream readers. It also removes the Go generator, since nothing depends on it
afterwards. I would rather get that base agreed before building the integration
executable on top of it, because otherwise the executable has no corpus to run
against.
Two corrections to my own comment above, both found while writing those
tests:
- `cpp-21.0.0/generated_binary.arrow_file` is not currently readable — it
fails with `unknownType("Unsupported field type found: fixedsizebinary")`.
Attempting the whole corpus, 142 of 182 files fail, almost all on unsupported
types. The conformance tests in #185 therefore use `generated_primitive` and
`generated_duplicate_fieldnames`, which do read today.
- Comparing `ArrowWriter` output against gold files does not work as I
suggested. The IPC format leaves room for implementation choices in padding and
metadata layout, so a byte comparison would fail spuriously even on correct
output. Validating against the JSON representation, as you describe, is the
right approach — which is a further argument for Archery.
One prerequisite worth flagging while we are on the subject. The integration
JSON always carries a `VALIDITY` array, including for non-nullable fields where
every entry is 1, while the IPC file may omit the validity buffer entirely in
that case. Arrow Swift currently mishandles the omitted case: it reads outside
the allocation and returns different values between consecutive runs of the
same input. I have a fix in flight. Without it, `VALIDATE` would fail
spuriously on every non-nullable field, so it looks worth landing first.
On your other question in this issue, about exposing footer blocks: I lean
towards not adding public API for it if the gold files can cover the same
ground, since that is API surface to maintain either way. But I do not have a
strong view, and it would be useful to know whether the block alignment fix in
#95 is verifiable through a written-then-reread file rather than through direct
footer access.
As a side effect, removing the Go dependency also makes adding a macOS
runner simpler, since the build then needs only a Swift toolchain and the
submodule.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
