xiangfu0 commented on PR #19309: URL: https://github.com/apache/pinot/pull/19309#issuecomment-5875488150
Rebased onto master now that #19307 and #19308 are merged, and simplified. The PR went from 3 files / +1128 to **2 files / +764**, and it is one commit again. The "conflict" was an artifact, not real: this branch still carried #19308's commits, whose code is now in master under a different SHA from the squash-merge, so GitHub was showing 20 files / +1556. Replaying only this PR's own commit onto master resolved it with zero conflicts. **Dropped `docs/design/codec-pipeline-v7.md` (523 lines).** `docs/` does not exist anywhere in master, so this would have created a new top-level documentation tree, and Pinot's user docs live in `pinot-docs`. I kept the content and am happy to open it there instead — the format reference is genuinely useful, just not here. Its non-goals are summarised below since they answer your earlier question directly. **Cut the codec matrix from 14 specs to 4.** Each spec previously got its own INT *and* LONG column (28 extra columns) crossed with both engines over three per-codec tests — about 84 invocations on the shared cluster, now 8. The codecs' arithmetic is already unit-tested in master (per-definition tests, `CodecPipelineExecutorTest` including the seven-stage chain, validator, corrupt-input, and the V7 round-trips in `ForwardIndexCreatorFactoryTest`); what integration uniquely proves is ingestion → segment build → query. The four kept columns cover one pipeline shape each: `LZ4` (single-stage compression), `DELTA,ZSTD(3)` (transform + compression with an argument, which also pins that the explicit level survives into the V7 header), `T64` (packing transform with no compression stage), `DELTA,T64,LZ4` (three-stage). I also added `intLegacyLz4` — a raw column with legacy `compressionCodec` and no `codecSpec` — as a negative control, so `testGeneratedSegmentsUseConfiguredForwar dIndexFormats` would catch a regression that routed every raw column through V7. **The realtime test no longer re-runs the whole matrix.** It used to extend `CodecPipelineIntegrationTest`, inheriting every per-codec test and roughly doubling the cluster cost; it now extends `CustomDataQueryClusterIntegrationTest` with its own minimal fixture and covers only the table-type-specific behavior — that `codecSpec` is applied when a consuming segment is committed to an immutable segment (the mutable index ignores it by design). 3 invocations instead of ~92. **Non-goals for v1, from the removed doc** (enforced in code on master, so it is not just documentation): `codecSpec` is supported only for single-value INT/LONG raw columns — `ForwardIndexType` rejects it on multi-value columns and on any other stored type, and the V7 writer only accepts a 4- or 8-byte entry. Variable-width and multi-value V7 storage, consuming-index compression, a SQL-visible codec function, and plugin-registered codecs are all deliberate follow-ups. So there is no variable-length reader in this series because there is no variable-length V7 writer yet. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_ -- 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]
