xiangfu0 commented on PR #18229: URL: https://github.com/apache/pinot/pull/18229#issuecomment-4637064165
## Codec chaining now supported (revisits the earlier single-transform constraint) Following up on the earlier discussion where we restricted pipelines to a single transform: the pipeline now supports **full chaining** of the form > **N value-preserving transforms → at most one packing transform → N compressions** Examples that are now valid and round-trip end-to-end (covered by `CodecPipelineForwardIndexTest`): - `CODEC(DELTA, DELTADELTA, LZ4)` — chained value transforms + compression - `CODEC(DELTA, T64, LZ4)` — delta → frame-of-reference bit-pack → compress - `CODEC(DELTA, LZ4, ZSTD(3))` — multiple chained compressions ### Why multi-transform is now safe (not the earlier corruption case) The original concern was that `CODEC(DELTA, DELTA)` would silently corrupt, because each transform embedded a per-frame `[flag][count]` header and assumed raw column-typed input — so feeding one transform's headered output into the next was undefined. That's fixed at the root: **`DELTA`/`DELTADELTA` are now header-less, value-preserving passthrough transforms** — they map a typed value array to a same-width typed value array, with the element type taken from the column context and the value count from the buffer length. So a following transform receives exactly what it expects, and the chain composes correctly. This is enforced, not just hoped for: - New SPI contract `CodecDefinition.isValuePreserving()` — `true` for `DELTA`/`DELTADELTA`, `false` for packing transforms (`T64`/`GORILLA`) and compressions. - `CodecPipelineValidator` walks the stages tracking a "typed-value domain". A `TRANSFORM` may only appear while still in that domain; a **packing** transform (`T64`/`GORILLA`, whose bit-packed output is no longer a typed value array) or any compression ends it. So a packing transform must be the **last** transform, and transforms must precede compressions. ### Still rejected (by necessity) - `CODEC(T64, DELTA, …)` / `CODEC(T64, GORILLA)` — a packing transform's output isn't a typed value array, so nothing typed may follow it. - `CODEC(ZSTD(3), DELTA)` — a transform can't consume compressed bytes. Wire formats: `DELTA`/`DELTADELTA` are now header-less (passthrough); `T64`/`GORILLA` keep their self-framed formats and remain terminal among transforms. Design doc (§2, §5, §12) updated. Happy to adjust the model if you'd prefer to keep the stricter constraint. -- 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]
