Skip to content

[UUID 4b] Bloom filter support for the logical UUID type - #19182

Merged
xiangfu0 merged 5 commits into
apache:masterfrom
xiangfu0:uuid-split/04b-bloom
Aug 7, 2026
Merged

[UUID 4b] Bloom filter support for the logical UUID type#19182
xiangfu0 merged 5 commits into
apache:masterfrom
xiangfu0:uuid-split/04b-bloom

Conversation

@xiangfu0

@xiangfu0 xiangfu0 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What

Completes Bloom-filter support for logical UUID columns across segment creation, segment reload/index preprocessing, and segment pruning.

Representation

UUID values are stored as fixed-width BYTES. Bloom filters use the existing lowercase dashless hexadecimal representation:

  • query literals normalize to 16 UUID bytes
  • BloomFilterCreator encodes byte-backed stored values with BytesUtils.toHexString
  • ValueBasedSegmentPruner hashes ByteArray.toString, which produces the same representation

No UUID-specific Bloom key rendering is needed.

Changes

  • dispatch BloomFilterCreator on stored type so UUID reuses BYTES handling for SV and MV values
  • dispatch BloomFilterHandler on stored type so Bloom indexes can be added during reload for dictionary and raw UUID columns
  • remove the UUID-specific canonical-dashed conversion from ValueBasedSegmentPruner
  • update the creator test to assert the stored hexadecimal representation
  • replace mocked creator coverage with a real segment build/load/prune component test
  • add a shared-cluster CustomIntegration test that builds and queries a Bloom-indexed UUID segment

Testing

  • BloomFilterCreatorTest
  • BloomFilterSegmentPrunerTest
  • UuidBloomFilterTest
  • component matrix: dictionary/raw × SV/MV × creation-time/reload-time
  • cluster queries with a dashless hexadecimal literal and a canonical string cast to UUID
  • pruning metrics prove present values are processed and an absent in-range value is Bloom-pruned
  • canonical, uppercase, and dashless UUID query literals; EQ and IN; present and absent values
  • multiple values within an MV row

About this PR

Repurposed from pruning-only to the UUID Bloom-filter story.

Part of #18140.

BloomFilterSegmentPruner could not prune UUID columns: it probed the bloom
filter with _comparableValue.toString(), which for a UUID is a ByteArray, so
the probe used ByteArray's identity-ish toString and never matched a real
entry. That is not a correctness bug -- a bloom miss only ever costs a
non-pruned segment -- but it made the pruner useless for UUID predicates.

The fix matches BloomFilterCreator.add(Object, int), which is the contract the
reader has to mirror: it renders a UUID as its canonical dashed string. The
rendering is resolved once, at pruner construction, rather than per
mightBeContained() call.

Deliberately scoped to UUID. BIG_DECIMAL has the same class of divergence
(DataType#toString uses toPlainString(), the creator uses value.toString(), so
they disagree on trailing zeros) but fixing that changes pruning for existing
tables and belongs in its own change.

Split out of apache#18872. Depends only on UuidUtils (apache#18869), already on master.
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Aug 7, 2026
Adds UUID handling to the predicate evaluators, so =, !=, IN, NOT IN and range
predicates work against a UUID column on both the raw and the dictionary path.

UUID follows the pattern TIMESTAMP already uses: a logical type whose stored
type does the work. The literal is parsed to its 16-byte stored form once, when
the evaluator is built, and from there the existing BYTES evaluators apply --
no per-value conversion in the scan loop.

The dictionary path needs no UUID branch: Dictionary#getStoredValue returns hex
for a UUID column and indexOf(String) hex-decodes, so the existing String-keyed
lookup is already correct. PredicateUtils renders the literal to that hex form
for those String-typed lookup APIs.

Split into apache#19181 (CAST), apache#19182 (bloom filter pruning) and apache#19183 (transform
functions); this PR is now just the predicate evaluators.
@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.65%. Comparing base (d3604a5) to head (19a0bed).
⚠️ Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
...t/index/loader/bloomfilter/BloomFilterHandler.java 33.33% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19182      +/-   ##
============================================
+ Coverage     66.62%   66.65%   +0.03%     
  Complexity     1423     1423              
============================================
  Files          3443     3443              
  Lines        218577   218626      +49     
  Branches      34792    34792              
============================================
+ Hits         145624   145730     +106     
+ Misses        61218    61171      -47     
+ Partials      11735    11725      -10     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 66.65% <50.00%> (+0.03%) ⬆️
temurin 66.65% <50.00%> (+0.03%) ⬆️
unittests 66.65% <50.00%> (+0.03%) ⬆️
unittests1 57.21% <50.00%> (+0.08%) ⬆️
unittests2 38.92% <12.50%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Those forms are accepted by UuidUtils.toBytes but must hash to the canonical
key, which is why the probe goes literal -> stored bytes -> canonical string
rather than hashing the raw literal. Verified non-vacuous: hashing _value
directly fails exactly these assertions and no others.

Also uses the UuidUtils.toString(ByteArray) overload instead of unwrapping.
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Aug 7, 2026
Adds UUID handling to the predicate evaluators, so =, !=, IN, NOT IN and range
predicates work against a UUID column on both the raw and the dictionary path.

UUID follows the pattern TIMESTAMP already uses: a logical type whose stored
type does the work. The literal is parsed to its 16-byte stored form once, when
the evaluator is built, and from there the existing BYTES evaluators apply --
no per-value conversion in the scan loop.

The dictionary path needs no UUID branch: Dictionary#getStoredValue returns hex
for a UUID column and indexOf(String) hex-decodes, so the existing String-keyed
lookup is already correct. PredicateUtils renders the literal to that hex form
for those String-typed lookup APIs.

Split into apache#19181 (CAST), apache#19182 (bloom filter pruning) and apache#19183 (transform
functions); this PR is now just the predicate evaluators.
@xiangfu0
xiangfu0 requested a lite review from Copilot August 7, 2026 19:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables BloomFilterSegmentPruner to correctly prune segments for logical UUID columns by hashing the same canonical UUID string representation that BloomFilterCreator writes into the bloom filter, and adds unit coverage to prevent future reader/writer rendering divergence.

Changes:

  • Update bloom-filter probe key rendering to canonical UUID string for UUID-typed predicates (mirrors BloomFilterCreator#add(Object, int) contract).
  • Add UUID-specific pruning tests, including mixed-case and dashless literal normalization.
  • Add a round-trip test that builds bloom filters through BloomFilterCreator to validate reader/writer consistency across multiple data types (including BIG_DECIMAL scientific notation).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java Fix bloom-filter probe key generation for UUID by rendering to canonical UUID string before hashing (memoized per value/type).
pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java Add UUID pruning tests and a creator-round-trip regression test to ensure bloom filter key rendering stays consistent.

@Jackie-Jiang Jackie-Jiang added the feature New functionality label Aug 7, 2026

@Jackie-Jiang Jackie-Jiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the logic of adding UUID into bloom filter? What value do we add?

Use the BYTES stored type's lowercase hex key for UUID Bloom creation and pruning. Cover segment generation and reload across dictionary/raw and SV/MV paths with real generated segments.
@xiangfu0 xiangfu0 changed the title [UUID 4b] Bloom filter segment pruning for the logical UUID type [UUID 4b] Bloom filter support for the logical UUID type Aug 7, 2026
Exercise UUID Bloom filtering through a real custom-cluster query using both dashless hex and canonical CAST literals. Verify present values are not falsely pruned and an absent in-range value is pruned by the Bloom filter.
@xiangfu0
xiangfu0 requested review from Jackie-Jiang and a lite review from Copilot August 7, 2026 22:53
@xiangfu0

xiangfu0 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review question about what is inserted into the Bloom filter:

  • UUID ingestion normalizes values to the 16-byte stored representation.
  • BloomFilterCreator dispatches on getStoredType(); UUID therefore follows the BYTES path.
  • The Bloom key is BytesUtils.toHexString(byte[]), i.e. lowercase dashless hex.
  • Query literals (dashless hex or CAST(canonical AS UUID)) normalize to the same bytes/hex key.

The new UuidBloomFilterTest validates both query forms through a real cluster and asserts that present segments are processed while an absent in-range UUID is Bloom-pruned.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@xiangfu0
xiangfu0 merged commit baaf0a7 into apache:master Aug 7, 2026
12 checks passed
@xiangfu0
xiangfu0 deleted the uuid-split/04b-bloom branch August 7, 2026 23:34
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Aug 8, 2026
Add UUID transform coverage and stored-byte handling for CASE and IN.

- CASE validates bare UUID branches as fixed-width hex and initializes the bytes result buffer for null-aware evaluation; canonical dashed values require CAST(... AS UUID).
- IN parses all-literal UUID lists to the 16-byte stored form so dashed and dashless spellings compare by value.
- Binary comparisons already dispatch through BYTES stored types; add dictionary/raw regression coverage and improve diagnostics and comparator documentation.

Split out of apache#18872. Prerequisite UUID changes apache#19181 and apache#19182 are now on master.
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Aug 8, 2026
Add UUID transform regression coverage and stored-byte handling for CASE.

- CASE validates bare UUID branches as fixed-width hex and initializes the bytes result buffer for null-aware evaluation; canonical dashed values require CAST(... AS UUID).\n- IN already uses the BYTES stored path; cover uppercase hex, explicit UUID casts, and rejection of bare canonical strings.\n- Binary comparisons already dispatch through BYTES stored types; add dictionary/raw regression coverage and improve diagnostics and comparator documentation.

Split out of apache#18872. Prerequisite UUID changes apache#19181 and apache#19182 are now on master.
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Aug 8, 2026
Add UUID transform regression coverage and stored-byte handling for CASE.

- CASE validates bare UUID branches as fixed-width hex and initializes the bytes result buffer for null-aware evaluation; canonical dashed values require CAST(... AS UUID).
- IN already uses the BYTES stored path; cover uppercase hex, explicit UUID casts, and rejection of bare canonical strings.
- Binary comparisons already dispatch through BYTES stored types; add dictionary/raw regression coverage and improve diagnostics and comparator documentation.

Split out of apache#18872. Prerequisite UUID changes apache#19181 and apache#19182 are now on master.
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Aug 8, 2026
Add UUID transform regression coverage and stored-byte handling for CASE.

- CASE validates bare UUID branches as fixed-width hex and initializes the bytes result buffer for null-aware evaluation; canonical dashed values require CAST(... AS UUID).
- IN already uses the BYTES stored path; cover uppercase hex, explicit UUID casts, and rejection of bare canonical strings.
- Binary comparisons already dispatch through BYTES stored types; add dictionary/raw regression coverage and improve diagnostics and comparator documentation.

Split out of apache#18872. Prerequisite UUID changes apache#19181 and apache#19182 are now on master.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants