You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "raghavyadav01 (via GitHub)" <gi...@apache.org> on 2026/09/25 18:30:42 UTC
[PR] Let a partition function report that it cannot place a value [pinot]
raghavyadav01 opened a new pull request, #19673:
URL: https://github.com/apache/pinot/pull/19673
`PartitionFunction.getPartition` returns a primitive `int`, so a function facing a value outside its column's domain has no way to say so — it has to return *some* id.
Returning a real one lets a pruner drop a segment that does hold matching rows, so the only safe answer available today is to pick an id and arrange for every segment to also claim it. That works, but it costs pruning on every query whose value genuinely belongs to that partition: at N buckets, roughly 1/N of the key space stops pruning entirely. It also leaves each segment carrying more than one partition id, which disqualifies it from partition-aware placement and from `TablePartitionInfo`.
This adds `UNKNOWN_PARTITION = -1` to the contract and teaches the readers to treat it as *"this value says nothing about which segments can match"*:
- `SinglePartitionColumnSegmentPruner` and `MultiPartitionColumnsSegmentPruner` keep the segment
- `ColumnValueSegmentPruner` (server side) keeps the segment
- `AbstractColumnStatisticsCollector` does not record it as a partition of the segment
- `MutableSegmentImpl` does not count an unplaceable row as a partition mismatch — the stream already decided which partition the row belongs to
### Compatibility
Nothing returns `-1` today. Every `PartitionIdNormalizer` (`POSITIVE_MODULO`, `ABS`, `MASK`, `PRE_MODULO_ABS`) yields a value in `[0, numPartitions)`, so existing partition functions, existing segment metadata, and existing pruning behaviour are all unaffected. The sentinel is opt-in for implementations that choose to return it.
### Testing
Two tests in `SegmentPrunerTest` covering both broker pruners, backed by a test-only `UnknownPartitionFunction`. Both fail without the change (verified by reverting the two pruners: 2 failures) and pass with it. `PartitionFunctionTest`, `PartitionFunctionFactoryTest` and `PartitionIdNormalizerTest` stay green (28 tests).
--
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] Let a partition function report that it cannot place a value [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19673:
URL: https://github.com/apache/pinot/pull/19673#issuecomment-5838150484
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19673?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 `50.00000%` with `3 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 58.29%. Comparing base ([`36a3253`](https://app.codecov.io/gh/apache/pinot/commit/36a3253a2d683c67ba10a189da322ec8d88f407a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9db3452`](https://app.codecov.io/gh/apache/pinot/commit/9db3452f411859e02be8c3fca41d8fd5b3ce5056?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 7 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/19673?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ot/core/query/pruner/ColumnValueSegmentPruner.java](https://app.codecov.io/gh/apache/pinot/pull/19673?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fquery%2Fpruner%2FColumnValueSegmentPruner.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvQ29sdW1uVmFsdWVTZWdtZW50UHJ1bmVyLmphdmE=) | 50.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19673?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...local/indexsegment/mutable/MutableSegmentImpl.java](https://app.codecov.io/gh/apache/pinot/pull/19673?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Findexsegment%2Fmutable%2FMutableSegmentImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9NdXRhYmxlU2VnbWVudEltcGwuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19673?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../impl/stats/AbstractColumnStatisticsCollector.java](https://app.codecov.io/gh/apache/pinot/pull/19673?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Fcreator%2Fimpl%2Fstats%2FAbstractColumnStatisticsCollector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9zdGF0cy9BYnN0cmFjdENvbHVtblN0YXRpc3RpY3NDb2xsZWN0b3IuamF2YQ==) | 66.66% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19673?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 (36a3253) and HEAD (9db3452). Click for more details.
>
> <details><summary>HEAD has 24 uploads less than BASE</summary>
>
>| Flag | BASE (36a3253) | HEAD (9db3452) |
>|------|------|------|
>|lane-a|2|0|
>|integration|4|0|
>|java-25|6|1|
>|integration1|2|0|
>|temurin|6|1|
>|lane-b|2|0|
>|unittests|2|1|
>|unittests2|1|0|
>|integration2|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19673 +/- ##
============================================
- Coverage 67.95% 58.29% -9.67%
+ Complexity 1450 1 -1449
============================================
Files 3511 2700 -811
Lines 227477 166169 -61308
Branches 35994 27073 -8921
============================================
- Hits 154592 96865 -57727
- Misses 60739 61167 +428
+ Partials 12146 8137 -4009
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19673/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/19673/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/19673/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/19673/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/19673/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.29% <50.00%> (-9.67%)` | :arrow_down: |
| [lane-a](https://app.codecov.io/gh/apache/pinot/pull/19673/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/19673/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/19673/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.29% <50.00%> (-9.67%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19673/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.29% <50.00%> (-9.67%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19673/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.29% <50.00%> (+0.10%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19673/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/19673?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] Let a partition function report that it cannot place a value [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on PR #19673:
URL: https://github.com/apache/pinot/pull/19673#issuecomment-5897830939
Closing this for now. The cases it was written for are all handled where the value is parsed, so nothing in the tree needs the sentinel yet. Worth reopening if a partition function lands that genuinely cannot place a legal value of its column's type.
--
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] Let a partition function report that it cannot place a value [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 closed pull request #19673: Let a partition function report that it cannot place a value
URL: https://github.com/apache/pinot/pull/19673
--
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