parthchandra opened a new issue, #7275:
URL: https://github.com/apache/datafusion-comet/issues/7275
Part of #7267.
## Problem
All the overflows in this epic come from storing values in `Utf8`/`Binary`,
which
put every value of an array in one buffer indexed by i32 offsets.
`Utf8View`/`BinaryView` arrays store values across several data buffers, so
no
single buffer has to hold the whole column. arrow's `BatchCoalescer` already
handles
view types with `InProgressByteViewArray`, which adds new buffers instead of
concatenating into one (`arrow-select/src/coalesce.rs:699` for Utf8View,
`:701` for
BinaryView).
## Where Comet stands today
- **Comet never produces view types.** The planner casts DataFusion UDF view
results
back to Utf8/Binary (`planner.rs:2197-2215`, "Comet does not yet support
view
types"), and `jni_api.rs:2989` notes view encodings never come out of a
Comet scan.
- **Native Parquet scan:** DataFusion's `schema_force_view_types` (default
true) only
affects schema inference (`ParquetFormat::infer_schema`), which Comet
doesn't use;
Comet passes its own table schema. The Parquet opener already reads
columns as
views when the table schema declares Utf8View/BinaryView
(`apply_file_schema_type_coercions`, `opener/mod.rs:965`). So the scan
change is
mostly mapping String/Bytes to view types in `serde.rs:80-81`, behind a
config.
- **Native Iceberg scan:** goes through iceberg-rust, then Comet's per-file
adapter
casts to Comet's output schema (`iceberg_scan.rs:240-248`). Both the
output schema
and the adapter need changing. iceberg-rust maps Iceberg binary to
LargeBinary in
its schema conversion (`crates/iceberg/src/arrow/schema.rs:723`); unsure
whether
that type reaches the reader output.
- **JVM boundary:** no view support. `CometPlainVector` only handles
`BaseVariableWidthVector` and `BaseLargeVariableWidthVector` offsets
(`CometPlainVector.java:66-77`), so a view vector would be misread. Arrow
Java
18.3.0 has view vectors; unsure whether its C Data import supports them.
Until it
does, views must be cast back to Utf8/Binary at the plan root before
export, which
puts the i32 limit back on each exported batch (at batch-size scale only).
- **Shuffle:** the writer already knows about views (`contains_view_type`,
`shuffle_writer.rs:422`, avoids sliced view arrays), and
`remote_schema.rs:373-374`
treats Utf8View vs Utf8 as a schema mismatch.
## Proposal
- Map String/Bytes to view types in the native scans, behind a config,
possibly only
for columns whose statistics show large values.
- Make sure every downstream native operator and expression handles view
types, or
casts at the boundary.
- Add view support at the JVM boundary, or cast to Utf8/Binary before export.
## Open questions
- Expression and operator coverage for view types. Not checked.
- Performance cost of view types for small strings, the common case.
- Whether Arrow Java C Data import handles view types.
## Notes
- Separate feature with a larger blast radius. Sub-issues 1-7 and 9-12 fix
the
immediate failures without it.
- `LargeUtf8`/`LargeBinary` (i64 offsets) is the other alternative, at the
cost of
wider offsets and more casts.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
