Add Arrow IPC format for ad-hoc queries.#3791
Merged
Merged
Conversation
Contributor
Author
|
@Karakatiza666 we should switch the web-console to use the arrow-ipc format instead of JSON to really fix #3564. I'll create a separate issue. |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for the Arrow IPC format for ad-hoc queries, along with adding Parquet output support for fda. Key changes include:
- Adding a new ArrowIpc variant to the AdHocResultFormat enum and its Display implementation.
- Extending fda to handle Arrow IPC and Parquet outputs in multiple commands.
- Updating the CLI to include new output format options and adding the Arrow dependency.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/feldera-types/src/query.rs | Added new ArrowIpc variant with corresponding Display mapping. |
| crates/fda/src/shell.rs | Extended output format matching to include Arrow IPC and Parquet. |
| crates/fda/src/main.rs | Updated query/pipeline actions to support Arrow IPC and Parquet. |
| crates/fda/src/cli.rs | Added new CLI output format options with a Display impl. |
| crates/fda/Cargo.toml | Included the new Arrow dependency. |
| crates/adapters/src/adhoc/mod.rs | Integrated StreamWriter to stream Arrow IPC responses. |
Files not reviewed (1)
- openapi.json: Language not supported
Comments suppressed due to low confidence (1)
crates/fda/src/shell.rs:153
- [nitpick] There's an inconsistency in the string representation for the ArrowIpc format across the codebase. In query.rs the variant maps to 'arrow_ipc', so consider aligning the representation to avoid confusion.
OutputFormat::ArrowIpc => "arrow",
8dcdf3c to
3117ee9
Compare
mihaibudiu
approved these changes
Mar 27, 2025
Contributor
|
Hey, how about the automatic review? |
3117ee9 to
4bcd65b
Compare
JSON is unable to represent some datatypes we support like MAP with non-string keys so the serialization to JSON fails (#3546). This adds a new query format for arrow ipc to the backend and fda. Using this formats the queries also work for map types with non-string keys. Also adds support for parquet format to fda which was previously missing. Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
4bcd65b to
105046f
Compare
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.
JSON is unable to represent some datatypes we support like MAP with non-string keys so the serialization to JSON fails (#3546).
This adds a new query format for arrow ipc to the backend and fda. Using this formats the queries also work for map types with non-string keys.
Also adds support for parquet format to fda which was previously missing.