You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/08/19 05:38:57 UTC
[PR] Add DELTA and DELTADELTA transform codecs [pinot]
xiangfu0 opened a new pull request, #19305:
URL: https://github.com/apache/pinot/pull/19305
Stacked on #19285 (`xiangfu0/codex/codec-stack/02-runtime`). Part of the split of #18229.
## Summary
Adds the first two transform codecs to the package-private codec runtime in
`org.apache.pinot.segment.local.io.codec`:
- `BaseDeltaCodecDefinition` — shared structure for the delta family. Both codecs are
`CodecKind.TRANSFORM` stages restricted to `INT`/`LONG` stored types, and both are
**typed-layout-preserving** (`preservesTypedValueLayout() == true`): they map `count`
column-typed values to the same `count` same-width values with no per-frame header, so they can
be chained ahead of another transform (or each other) and `maxEncodedSize` is the identity.
Two's-complement wrap-around on encode/decode is an intentional, test-locked contract:
overflow-adjacent sequences round-trip bit-for-bit through the wrapping `-`/`+` arithmetic.
- `DeltaCodecDefinition` (`DELTA`) — first value verbatim, then successive deltas.
- `DeltaDeltaCodecDefinition` (`DELTADELTA`) — first value, first delta, then delta-of-deltas
(good for near-regular timestamp intervals).
Both are registered in `CodecRegistry.DEFAULT`, so specs like `DELTA,LZ4`, `DELTADELTA,ZSTD(3)`,
and `DELTA,DELTADELTA,LZ4` now parse, validate, and execute through `CodecPipelineExecutor`.
The names `DELTA`/`DELTADELTA` are frozen on-disk contracts (stored verbatim in V7 segment
headers by a later PR in the stack) and must never be renamed.
## Tests
- `DeltaCodecRoundTripTest` (new) — executor-driven round trips for `DELTA`, `DELTADELTA`,
`DELTA,LZ4`, `DELTADELTA,LZ4` over INT and LONG datasets: empty, single-element, two-element
(DELTADELTA boundary), constant, monotonic, negative/sign-changing, and overflow-adjacent
(`MIN_VALUE` next to `MAX_VALUE`) sequences; both the allocating decode path and the bounded
decode-into (segment-reader) path. Also a fail-closed corrupt-input case: a frame whose byte
length is not a whole number of elements is rejected on both encode and bounded decode.
- `CodecPipelineValidatorTest` — real-transform scenarios against `CodecRegistry.DEFAULT`:
valid delta pipelines, typed-layout chaining (including `DELTA,DELTA` and mixed order),
transform-after-compression rejected, INT/LONG-only type check, argument rejection, and the
typed-value-layout contract assertions for both codecs.
- `CodecRegistryTest` — DELTA/DELTADELTA now resolve from `DEFAULT` (case-insensitive); the
parent PR's absence assertion for unregistered codec names is left untouched.
- `CompressionCodecCorruptInputTest` — the multi-stage inner-frame-guard test now uses
`DELTA,SNAPPY`, tightening the scratch bound to exactly the four-byte declared chunk size.
- `TableConfigUtilsTest` — the codecSpec feature gate still rejects a well-formed
`DELTADELTA,LZ4` config on a supported LONG column with
`"codecSpec is not supported yet for column: longCol"`, proving registration of the runtime
codecs did not open the config surface.
## Deliberately excluded (carried by later PRs in the stack)
- **T64 and GORILLA packing transforms** — next slice (04). They are the codecs with
`preservesTypedValueLayout() == false`; until then the packing-placement rules remain covered
by the synthetic `PACKING` handler in `CodecPipelineValidatorTest`.
- **V7 raw forward index format** (writer/reader that persists the canonical spec in segment
headers) — slice 05.
- **Opening the feature gate** (config/creator/reader/reload wiring, reload change-detection via
`CodecPipelineExecutor.getCanonicalSpec()`) — final slice.
## Why master stays safe with only this merged
The codec runtime remains package-private and unreachable from production paths: the only public
entry point is `CodecPipelineExecutor`, and every config surface still throws
`"codecSpec is not supported yet for column: %s"` (re-asserted in this PR's `TableConfigUtilsTest`
addition). Registering DELTA/DELTADELTA in the closed registry adds dead-until-enabled code with
full unit coverage; no segment format, wire protocol, or config behavior changes.
## Verification
```
./mvnw -q -T 1C install -DskipTests -Ppinot-fastdev -pl pinot-segment-local -am
./mvnw -q test -Ppinot-fastdev -pl pinot-segment-local \
-Dtest=DeltaCodecRoundTripTest,CodecPipelineValidatorTest,CodecRegistryTest,CodecPipelineExecutorTest,CompressionCodecCorruptInputTest \
-Dsurefire.failIfNoSpecifiedTests=false
./mvnw -q test -Ppinot-fastdev -pl pinot-segment-local -Dtest=TableConfigUtilsTest \
-Dsurefire.failIfNoSpecifiedTests=false
./mvnw spotless:apply license:format -pl pinot-segment-local
./mvnw checkstyle:check license:check -pl pinot-segment-local
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add DELTA and DELTADELTA transform codecs [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19305:
URL: https://github.com/apache/pinot/pull/19305#issuecomment-5404947727
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19305?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `205 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.64%. Comparing base ([`4824a5f`](https://app.codecov.io/gh/apache/pinot/commit/4824a5fd1f793b55a02de69f00395497830cf7a7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b81db76`](https://app.codecov.io/gh/apache/pinot/commit/b81db7680fbbd31b8819fcab0fe149e353c40a86?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/19305?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ment/local/io/codec/DeltaDeltaCodecDefinition.java](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fio%2Fcodec%2FDeltaDeltaCodecDefinition.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb2RlYy9EZWx0YURlbHRhQ29kZWNEZWZpbml0aW9uLmphdmE=) | 0.00% | [99 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...t/segment/local/io/codec/DeltaCodecDefinition.java](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fio%2Fcodec%2FDeltaCodecDefinition.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb2RlYy9EZWx0YUNvZGVjRGVmaW5pdGlvbi5qYXZh) | 0.00% | [63 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...gment/local/io/codec/BaseDeltaCodecDefinition.java](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fio%2Fcodec%2FBaseDeltaCodecDefinition.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb2RlYy9CYXNlRGVsdGFDb2RlY0RlZmluaXRpb24uamF2YQ==) | 0.00% | [41 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...he/pinot/segment/local/io/codec/CodecRegistry.java](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fio%2Fcodec%2FCodecRegistry.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb2RlYy9Db2RlY1JlZ2lzdHJ5LmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19305?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (4824a5f) and HEAD (b81db76). Click for more details.
>
> <details><summary>HEAD has 14 uploads less than BASE</summary>
>
>| Flag | BASE (4824a5f) | HEAD (b81db76) |
>|------|------|------|
>|unittests|2|1|
>|java-25|4|1|
>|temurin|4|1|
>|lane-a|1|0|
>|integration|2|0|
>|integration2|2|0|
>|lane-b|1|0|
>|unittests2|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19305 +/- ##
============================================
- Coverage 67.20% 57.64% -9.57%
+ Complexity 1418 1 -1417
============================================
Files 3479 2679 -800
Lines 223075 163172 -59903
Branches 35135 26423 -8712
============================================
- Hits 149920 94060 -55860
+ Misses 61184 61139 -45
+ Partials 11971 7973 -3998
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.64% <0.00%> (-9.57%)` | :arrow_down: |
| [lane-a](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [lane-b](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.64% <0.00%> (-9.57%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.64% <0.00%> (-9.57%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.64% <0.00%> (-0.06%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19305?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add DELTA and DELTADELTA transform codecs [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #19305:
URL: https://github.com/apache/pinot/pull/19305#discussion_r3856033680
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/codec/DeltaDeltaCodecDefinition.java:
##########
@@ -0,0 +1,209 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.io.codec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+
+
+/// Transform codec that computes delta-of-delta values between successive values before the
+/// compression stage (if any). Useful for data where the differences between consecutive values
+/// are approximately constant (e.g. timestamps with regular intervals).
+///
+/// DSL form: `DELTADELTA` (no arguments)
+///
+/// Supported stored types: `INT`, `LONG`.
+///
+/// Stateless and thread-safe; the [#INSTANCE] singleton is shared across all columns.
+///
+/// Wire format (header-less passthrough — element type from the column context, value count from
+/// the buffer length, so the output is a same-width typed value array a following transform can
+/// consume):
+/// ```
+/// [element_size bytes: first value verbatim]
+/// [element_size bytes: first delta (second - first), if count > 1]
+/// [(count-2) * element_size bytes: delta-of-deltas, if count > 2]
+/// ```
+final class DeltaDeltaCodecDefinition
Review Comment:
Do we need DELTADELTA, or just chain 2 DELTA?
What is the pros and cons of both approaches?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add DELTA and DELTADELTA transform codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #19305:
URL: https://github.com/apache/pinot/pull/19305#discussion_r3856049591
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/codec/DeltaDeltaCodecDefinition.java:
##########
@@ -0,0 +1,209 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.io.codec;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+
+
+/// Transform codec that computes delta-of-delta values between successive values before the
+/// compression stage (if any). Useful for data where the differences between consecutive values
+/// are approximately constant (e.g. timestamps with regular intervals).
+///
+/// DSL form: `DELTADELTA` (no arguments)
+///
+/// Supported stored types: `INT`, `LONG`.
+///
+/// Stateless and thread-safe; the [#INSTANCE] singleton is shared across all columns.
+///
+/// Wire format (header-less passthrough — element type from the column context, value count from
+/// the buffer length, so the output is a same-width typed value array a following transform can
+/// consume):
+/// ```
+/// [element_size bytes: first value verbatim]
+/// [element_size bytes: first delta (second - first), if count > 1]
+/// [(count-2) * element_size bytes: delta-of-deltas, if count > 2]
+/// ```
+final class DeltaDeltaCodecDefinition
Review Comment:
They are close but not interchangeable, so we kept both — `DELTA,DELTA` stays legal in the DSL (the validator accepts chained value-preserving transforms), while `DELTADELTA` is the dedicated form. Trade-offs:
**Not the same encoding at index 1.** `DELTADELTA` emits `[v0, v1−v0, second differences…]` (the classic Gorilla-style form). Composing `DELTA` twice emits `[v0, (v1−v0)−v0, second differences…]` — the second slot carries an epoch-magnitude outlier for timestamp columns. Both round-trip exactly, but when a frame-of-reference packing stage follows (`DELTADELTA,T64,…`), that one outlier forces max bit-width for the whole first 64-value block; the dedicated codec keeps every post-`v0` value small.
**Cost per stage.** The executor runs each stage as a full pass with its own intermediate buffer, so `DELTA,DELTA` is two passes + one extra scratch buffer per chunk on encode *and* decode; `DELTADELTA` does the second difference in a single pass.
**Frozen-name surface (the real argument for dropping it).** Codec names are permanent on-disk contracts once V7 headers ship. Dropping `DELTADELTA` later would be a format break, whereas adding it later would have been compatible — so keeping it is a commitment. We think it earns it as the canonical transform for the headline use case (near-regular timestamps), but if you prefer to minimize the frozen surface, removing it from the registry in this PR and re-adding it later is fully backward-compatible — happy to do that instead.
_🤖 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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add DELTA and DELTADELTA transform codecs [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #19305:
URL: https://github.com/apache/pinot/pull/19305
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org