From 002582585bc2ffa6310881002d4aa1084b71cdcd Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Thu, 6 Aug 2026 23:15:28 -0700 Subject: [PATCH 1/5] [UUID 4b] Bloom filter segment pruning for the logical UUID type 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 #18872. Depends only on UuidUtils (#18869), already on master. --- .../query/pruner/ValueBasedSegmentPruner.java | 19 +++- .../pruner/BloomFilterSegmentPrunerTest.java | 105 +++++++++++++++++- 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java index 0f0b768f878e..c23c319b43a6 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java @@ -37,7 +37,9 @@ import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.env.PinotConfiguration; import org.apache.pinot.spi.exception.BadQueryRequestException; +import org.apache.pinot.spi.utils.ByteArray; import org.apache.pinot.spi.utils.CommonConstants.Server; +import org.apache.pinot.spi.utils.UuidUtils; /// The `ValueBasedSegmentPruner` prunes segments based on values inside the filter and segment metadata and data. @@ -230,15 +232,30 @@ public void ensureDataType(DataType dt) { } public boolean mightBeContained(BloomFilterReader bloomFilter) { + // The rendering and hashing below run once per (value, data type): the resulting hashes are memoized and + // every subsequent segment in the query reuses them. Deliberately not precomputed in ensureDataType, so a + // query that only reaches min/max pruning never pays for it. if (!_hashed) { GuavaBloomFilterReaderUtils.Hash128AsLongs hash128AsLongs = - GuavaBloomFilterReaderUtils.hashAsLongs(_comparableValue.toString()); + GuavaBloomFilterReaderUtils.hashAsLongs(bloomFilterKey()); _hash1 = hash128AsLongs.getHash1(); _hash2 = hash128AsLongs.getHash2(); _hashed = true; } return bloomFilter.mightContain(_hash1, _hash2); } + + /// Renders the value exactly as `BloomFilterCreator#add(Object, int)` did when the index was built. If the + /// two disagree the lookup silently misses and the segment is wrongly pruned, dropping matching rows with no + /// error. That creator special-cases UUID to the canonical string and renders everything else with + /// `value.toString()` -- which for BYTES is already hex via [ByteArray]. + /// + /// Deliberately NOT routed through `DataType#toString`: that renders BIG_DECIMAL with + /// `toPlainString()`, which the creator does not, so every BIG_DECIMAL bloom filter would start missing. + private String bloomFilterKey() { + return _dt == DataType.UUID ? UuidUtils.toString(((ByteArray) _comparableValue).getBytes()) + : _comparableValue.toString(); + } } } } diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java index d9b30d2bb82a..f50ae6037072 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java @@ -25,6 +25,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.math.BigDecimal; import java.nio.file.Files; import java.util.ArrayList; import java.util.HashMap; @@ -38,11 +39,14 @@ import org.apache.pinot.segment.spi.SegmentMetadata; import org.apache.pinot.segment.spi.datasource.DataSource; import org.apache.pinot.segment.spi.datasource.DataSourceMetadata; +import org.apache.pinot.segment.spi.index.creator.BloomFilterCreator; import org.apache.pinot.segment.spi.index.reader.BloomFilterReader; import org.apache.pinot.segment.spi.memory.PinotDataBuffer; import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.env.PinotConfiguration; +import org.apache.pinot.spi.utils.UuidUtils; import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import static org.mockito.Mockito.mock; @@ -95,6 +99,100 @@ public void testBloomFilterPruning() assertTrue(runPruner(indexSegment, "SELECT COUNT(*) FROM testTable WHERE column = 21.0 AND column = 30.0")); } + @Test + public void testUuidBloomFilterPruning() + throws IOException { + IndexSegment indexSegment = mockIndexSegment(new String[]{ + "550e8400-e29b-41d4-a716-446655440000", + "550e8400-e29b-41d4-a716-446655440001" + }, DataType.UUID); + + assertFalse(runPruner(indexSegment, + "SELECT COUNT(*) FROM testTable WHERE column = '550e8400-e29b-41d4-a716-446655440000'")); + assertFalse(runPruner(indexSegment, + "SELECT COUNT(*) FROM testTable WHERE column IN ('550e8400-e29b-41d4-a716-446655440001')")); + assertTrue(runPruner(indexSegment, + "SELECT COUNT(*) FROM testTable WHERE column = '550e8400-e29b-41d4-a716-44665544ffff'")); + } + + /// The pruner hashes the query literal and looks it up in a bloom filter that was written by + /// [BloomFilterCreator]. If the two sides render the same value differently, the lookup misses, the segment is + /// pruned, and matching rows silently disappear. This builds the filter through the real creator so any divergence + /// shows up as a wrongly-pruned segment. + /// + /// BIG_DECIMAL is the sharp case: `BigDecimal.toString()` yields `1.0E-7` while + /// `toPlainString()` yields `0.00000010`, so rendering the reader side via + /// `FieldSpec.DataType#toString` would break every BIG_DECIMAL bloom filter. + @Test(dataProvider = "bloomFilterRoundTripValues") + public void testBloomFilterRoundTripsThroughCreatorRendering(DataType dataType, Object indexedValue, + String queryLiteral) + throws IOException { + IndexSegment indexSegment = mockIndexSegmentIndexedViaCreator(dataType, indexedValue); + assertFalse(runPruner(indexSegment, "SELECT COUNT(*) FROM testTable WHERE column = " + queryLiteral), + dataType + " literal " + queryLiteral + " was pruned despite being present in the bloom filter"); + } + + @DataProvider(name = "bloomFilterRoundTripValues") + public Object[][] bloomFilterRoundTripValues() { + byte[] uuidBytes = UuidUtils.toBytes("550e8400-e29b-41d4-a716-446655440000"); + return new Object[][]{ + {DataType.INT, 42, "42"}, + {DataType.LONG, 42L, "42"}, + {DataType.DOUBLE, 21.0d, "21.0"}, + // Scientific notation is the case that breaks if the reader renders via toPlainString(). + {DataType.BIG_DECIMAL, new BigDecimal("1.0E-7"), "1.0E-7"}, + // NOTE: a BIG_DECIMAL literal carrying trailing zeros (e.g. 123.4500) is still wrongly pruned, because the + // query literal loses them before it reaches the pruner while the indexed value keeps its scale. That is a + // pre-existing gap on master, independent of the rendering fixed here, so it is deliberately not asserted. + + {DataType.STRING, "hello", "'hello'"}, + {DataType.BYTES, new byte[]{0x0a, 0x1b, 0x2c}, "'0a1b2c'"}, + {DataType.UUID, uuidBytes, "'550e8400-e29b-41d4-a716-446655440000'"} + }; + } + + private IndexSegment mockIndexSegmentIndexedViaCreator(DataType dataType, Object indexedValue) + throws IOException { + IndexSegment indexSegment = mock(IndexSegment.class); + when(indexSegment.getColumnNames()).thenReturn(ImmutableSet.of("column")); + SegmentMetadata segmentMetadata = mock(SegmentMetadata.class); + when(segmentMetadata.getTotalDocs()).thenReturn(20); + when(indexSegment.getSegmentMetadata()).thenReturn(segmentMetadata); + + DataSource dataSource = mock(DataSource.class); + when(indexSegment.getDataSourceNullable("column")).thenReturn(dataSource); + DataSourceMetadata dataSourceMetadata = mock(DataSourceMetadata.class); + when(dataSourceMetadata.getDataType()).thenReturn(dataType); + when(dataSource.getDataSourceMetadata()).thenReturn(dataSourceMetadata); + + // Route the value through BloomFilterCreator's own default add(Object, int) so the writer-side rendering under + // test is the production one, not a copy of it. + BloomFilterReaderBuilder builder = new BloomFilterReaderBuilder(); + BloomFilterCreator creator = new BloomFilterCreator() { + @Override + public DataType getDataType() { + return dataType; + } + + @Override + public void add(String value) { + builder.put(value); + } + + @Override + public void seal() { + } + + @Override + public void close() { + } + }; + creator.add(indexedValue, -1); + when(dataSource.getBloomFilter()).thenReturn(builder.build()); + + return indexSegment; + } + @Test(expectedExceptions = RuntimeException.class) public void testQueryTimeoutOnPruning() throws IOException { @@ -161,6 +259,11 @@ public void testIsApplicableTo() { private IndexSegment mockIndexSegment(String[] values) throws IOException { + return mockIndexSegment(values, DataType.DOUBLE); + } + + private IndexSegment mockIndexSegment(String[] values, DataType dataType) + throws IOException { IndexSegment indexSegment = mock(IndexSegment.class); when(indexSegment.getColumnNames()).thenReturn(ImmutableSet.of("column")); SegmentMetadata segmentMetadata = mock(SegmentMetadata.class); @@ -175,7 +278,7 @@ private IndexSegment mockIndexSegment(String[] values) for (String v : values) { builder.put(v); } - when(dataSourceMetadata.getDataType()).thenReturn(DataType.DOUBLE); + when(dataSourceMetadata.getDataType()).thenReturn(dataType); when(dataSource.getDataSourceMetadata()).thenReturn(dataSourceMetadata); when(dataSource.getBloomFilter()).thenReturn(builder.build()); From 60d246b14a05d43bb7f19ea09d21dbe435c46153 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Fri, 7 Aug 2026 00:28:10 -0700 Subject: [PATCH 2/5] Cover uppercase and dashless UUID literals in bloom filter pruning 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. --- .../core/query/pruner/ValueBasedSegmentPruner.java | 2 +- .../query/pruner/BloomFilterSegmentPrunerTest.java | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java index c23c319b43a6..19669fb11c50 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java @@ -253,7 +253,7 @@ public boolean mightBeContained(BloomFilterReader bloomFilter) { /// Deliberately NOT routed through `DataType#toString`: that renders BIG_DECIMAL with /// `toPlainString()`, which the creator does not, so every BIG_DECIMAL bloom filter would start missing. private String bloomFilterKey() { - return _dt == DataType.UUID ? UuidUtils.toString(((ByteArray) _comparableValue).getBytes()) + return _dt == DataType.UUID ? UuidUtils.toString((ByteArray) _comparableValue) : _comparableValue.toString(); } } diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java index f50ae6037072..20306445073e 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java @@ -113,6 +113,17 @@ public void testUuidBloomFilterPruning() "SELECT COUNT(*) FROM testTable WHERE column IN ('550e8400-e29b-41d4-a716-446655440001')")); assertTrue(runPruner(indexSegment, "SELECT COUNT(*) FROM testTable WHERE column = '550e8400-e29b-41d4-a716-44665544ffff'")); + + // The bloom filter is keyed on the canonical lowercase dashed rendering, but UuidUtils.toBytes also accepts + // uppercase and dashless literals. Those must reach the same key, which is the whole reason the probe goes + // literal -> stored bytes -> canonical string rather than hashing the raw literal: hashing the literal directly + // would miss here and silently prune a segment that holds the row. + assertFalse(runPruner(indexSegment, + "SELECT COUNT(*) FROM testTable WHERE column = '550E8400-E29B-41D4-A716-446655440000'")); + assertFalse(runPruner(indexSegment, + "SELECT COUNT(*) FROM testTable WHERE column = '550e8400e29b41d4a716446655440000'")); + assertFalse(runPruner(indexSegment, + "SELECT COUNT(*) FROM testTable WHERE column IN ('550E8400E29B41D4A716446655440001')")); } /// The pruner hashes the query literal and looks it up in a bloom filter that was written by From 40a349917bfae192f2ca656438d624445e87b64c Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Fri, 7 Aug 2026 14:48:31 -0700 Subject: [PATCH 3/5] [UUID 4b] Use stored representation for Bloom filters 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. --- .../query/pruner/ValueBasedSegmentPruner.java | 19 +- .../pruner/BloomFilterSegmentPrunerTest.java | 213 +++++++++--------- .../bloomfilter/BloomFilterHandler.java | 7 +- .../index/creator/BloomFilterCreatorTest.java | 14 +- .../spi/index/creator/BloomFilterCreator.java | 19 +- 5 files changed, 124 insertions(+), 148 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java index 19669fb11c50..0f0b768f878e 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java @@ -37,9 +37,7 @@ import org.apache.pinot.spi.data.FieldSpec.DataType; import org.apache.pinot.spi.env.PinotConfiguration; import org.apache.pinot.spi.exception.BadQueryRequestException; -import org.apache.pinot.spi.utils.ByteArray; import org.apache.pinot.spi.utils.CommonConstants.Server; -import org.apache.pinot.spi.utils.UuidUtils; /// The `ValueBasedSegmentPruner` prunes segments based on values inside the filter and segment metadata and data. @@ -232,30 +230,15 @@ public void ensureDataType(DataType dt) { } public boolean mightBeContained(BloomFilterReader bloomFilter) { - // The rendering and hashing below run once per (value, data type): the resulting hashes are memoized and - // every subsequent segment in the query reuses them. Deliberately not precomputed in ensureDataType, so a - // query that only reaches min/max pruning never pays for it. if (!_hashed) { GuavaBloomFilterReaderUtils.Hash128AsLongs hash128AsLongs = - GuavaBloomFilterReaderUtils.hashAsLongs(bloomFilterKey()); + GuavaBloomFilterReaderUtils.hashAsLongs(_comparableValue.toString()); _hash1 = hash128AsLongs.getHash1(); _hash2 = hash128AsLongs.getHash2(); _hashed = true; } return bloomFilter.mightContain(_hash1, _hash2); } - - /// Renders the value exactly as `BloomFilterCreator#add(Object, int)` did when the index was built. If the - /// two disagree the lookup silently misses and the segment is wrongly pruned, dropping matching rows with no - /// error. That creator special-cases UUID to the canonical string and renders everything else with - /// `value.toString()` -- which for BYTES is already hex via [ByteArray]. - /// - /// Deliberately NOT routed through `DataType#toString`: that renders BIG_DECIMAL with - /// `toPlainString()`, which the creator does not, so every BIG_DECIMAL bloom filter would start missing. - private String bloomFilterKey() { - return _dt == DataType.UUID ? UuidUtils.toString((ByteArray) _comparableValue) - : _comparableValue.toString(); - } } } } diff --git a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java index 20306445073e..5d02f6a339d1 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/BloomFilterSegmentPrunerTest.java @@ -25,26 +25,37 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; -import java.math.BigDecimal; import java.nio.file.Files; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.Executors; +import org.apache.commons.io.FileUtils; import org.apache.pinot.core.query.request.context.QueryContext; import org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils; +import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader; +import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl; +import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig; import org.apache.pinot.segment.local.segment.index.readers.bloom.OnHeapGuavaBloomFilterReader; +import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader; +import org.apache.pinot.segment.spi.ImmutableSegment; import org.apache.pinot.segment.spi.IndexSegment; import org.apache.pinot.segment.spi.SegmentMetadata; +import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig; import org.apache.pinot.segment.spi.datasource.DataSource; import org.apache.pinot.segment.spi.datasource.DataSourceMetadata; -import org.apache.pinot.segment.spi.index.creator.BloomFilterCreator; import org.apache.pinot.segment.spi.index.reader.BloomFilterReader; import org.apache.pinot.segment.spi.memory.PinotDataBuffer; +import org.apache.pinot.spi.config.table.BloomFilterConfig; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.TableType; import org.apache.pinot.spi.data.FieldSpec.DataType; +import org.apache.pinot.spi.data.Schema; +import org.apache.pinot.spi.data.readers.GenericRow; import org.apache.pinot.spi.env.PinotConfiguration; -import org.apache.pinot.spi.utils.UuidUtils; +import org.apache.pinot.spi.utils.ReadMode; +import org.apache.pinot.spi.utils.builder.TableConfigBuilder; import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -53,11 +64,17 @@ import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; public class BloomFilterSegmentPrunerTest { private static final BloomFilterSegmentPruner PRUNER = new BloomFilterSegmentPruner(); + private static final String UUID_COLUMN = "uuidColumn"; + private static final String UUID_0 = "550e8400-e29b-41d4-a716-446655440000"; + private static final String ABSENT_UUID = "550e8400-e29b-41d4-a716-446655440001"; + private static final String UUID_2 = "550e8400-e29b-41d4-a716-446655440002"; + private static final String UUID_3 = "550e8400-e29b-41d4-a716-446655440003"; @BeforeClass public void setUp() { @@ -99,109 +116,102 @@ public void testBloomFilterPruning() assertTrue(runPruner(indexSegment, "SELECT COUNT(*) FROM testTable WHERE column = 21.0 AND column = 30.0")); } - @Test - public void testUuidBloomFilterPruning() - throws IOException { - IndexSegment indexSegment = mockIndexSegment(new String[]{ - "550e8400-e29b-41d4-a716-446655440000", - "550e8400-e29b-41d4-a716-446655440001" - }, DataType.UUID); - - assertFalse(runPruner(indexSegment, - "SELECT COUNT(*) FROM testTable WHERE column = '550e8400-e29b-41d4-a716-446655440000'")); - assertFalse(runPruner(indexSegment, - "SELECT COUNT(*) FROM testTable WHERE column IN ('550e8400-e29b-41d4-a716-446655440001')")); - assertTrue(runPruner(indexSegment, - "SELECT COUNT(*) FROM testTable WHERE column = '550e8400-e29b-41d4-a716-44665544ffff'")); - - // The bloom filter is keyed on the canonical lowercase dashed rendering, but UuidUtils.toBytes also accepts - // uppercase and dashless literals. Those must reach the same key, which is the whole reason the probe goes - // literal -> stored bytes -> canonical string rather than hashing the raw literal: hashing the literal directly - // would miss here and silently prune a segment that holds the row. - assertFalse(runPruner(indexSegment, - "SELECT COUNT(*) FROM testTable WHERE column = '550E8400-E29B-41D4-A716-446655440000'")); - assertFalse(runPruner(indexSegment, - "SELECT COUNT(*) FROM testTable WHERE column = '550e8400e29b41d4a716446655440000'")); - assertFalse(runPruner(indexSegment, - "SELECT COUNT(*) FROM testTable WHERE column IN ('550E8400E29B41D4A716446655440001')")); - } - - /// The pruner hashes the query literal and looks it up in a bloom filter that was written by - /// [BloomFilterCreator]. If the two sides render the same value differently, the lookup misses, the segment is - /// pruned, and matching rows silently disappear. This builds the filter through the real creator so any divergence - /// shows up as a wrongly-pruned segment. - /// - /// BIG_DECIMAL is the sharp case: `BigDecimal.toString()` yields `1.0E-7` while - /// `toPlainString()` yields `0.00000010`, so rendering the reader side via - /// `FieldSpec.DataType#toString` would break every BIG_DECIMAL bloom filter. - @Test(dataProvider = "bloomFilterRoundTripValues") - public void testBloomFilterRoundTripsThroughCreatorRendering(DataType dataType, Object indexedValue, - String queryLiteral) - throws IOException { - IndexSegment indexSegment = mockIndexSegmentIndexedViaCreator(dataType, indexedValue); - assertFalse(runPruner(indexSegment, "SELECT COUNT(*) FROM testTable WHERE column = " + queryLiteral), - dataType + " literal " + queryLiteral + " was pruned despite being present in the bloom filter"); + @Test(dataProvider = "uuidBloomFilterCreationModes") + public void testUuidBloomFilterPruningEndToEnd(boolean noDictionary, boolean multiValue, boolean createOnLoad) + throws Exception { + File indexDir = Files.createTempDirectory("uuidBloomFilterSegment").toFile(); + ImmutableSegment segment = null; + try { + TableConfig tableConfig = createTableConfig(noDictionary, !createOnLoad); + Schema schema = createSchema(multiValue); + + List rows = new ArrayList<>(); + rows.add(row(UUID_0, multiValue)); + rows.add(row(UUID_2, multiValue)); + + String segmentName = (noDictionary ? "raw" : "dictionary") + (multiValue ? "Mv" : "Sv") + "UuidSegment"; + SegmentGeneratorConfig generatorConfig = new SegmentGeneratorConfig(tableConfig, schema); + generatorConfig.setSegmentName(segmentName); + generatorConfig.setOutDir(indexDir.getAbsolutePath()); + SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl(); + driver.init(generatorConfig, new GenericRowRecordReader(rows)); + driver.build(); + + File segmentDir = new File(indexDir, segmentName); + if (createOnLoad) { + segment = ImmutableSegmentLoader.load(segmentDir, + new IndexLoadingConfig(createTableConfig(noDictionary, true), schema)); + } else { + segment = ImmutableSegmentLoader.load(segmentDir, ReadMode.mmap); + } + assertEquals(segment.getSegmentMetadata().getColumnMetadataFor(UUID_COLUMN).hasDictionary(), !noDictionary); + assertEquals(segment.getSegmentMetadata().getColumnMetadataFor(UUID_COLUMN).isSingleValue(), !multiValue); + assertEquals(segment.getDataSource(UUID_COLUMN).getDataSourceMetadata().getDataType(), DataType.UUID); + assertNotNull(segment.getDataSource(UUID_COLUMN).getBloomFilter()); + + assertFalse(runPruner(segment, + "SELECT COUNT(*) FROM testTable WHERE uuidColumn = '" + UUID_0 + "'")); + assertFalse(runPruner(segment, + "SELECT COUNT(*) FROM testTable WHERE uuidColumn = '550E8400-E29B-41D4-A716-446655440000'")); + assertFalse(runPruner(segment, + "SELECT COUNT(*) FROM testTable WHERE uuidColumn = '550e8400e29b41d4a716446655440000'")); + if (multiValue) { + assertFalse(runPruner(segment, + "SELECT COUNT(*) FROM testTable WHERE uuidColumn = '" + UUID_3 + "'")); + } + assertFalse(runPruner(segment, + "SELECT COUNT(*) FROM testTable WHERE uuidColumn IN ('" + ABSENT_UUID + "', '" + UUID_2 + "')")); + assertTrue(runPruner(segment, + "SELECT COUNT(*) FROM testTable WHERE uuidColumn = '" + ABSENT_UUID + "'")); + } finally { + if (segment != null) { + segment.destroy(); + } + FileUtils.deleteDirectory(indexDir); + } } - @DataProvider(name = "bloomFilterRoundTripValues") - public Object[][] bloomFilterRoundTripValues() { - byte[] uuidBytes = UuidUtils.toBytes("550e8400-e29b-41d4-a716-446655440000"); + @DataProvider(name = "uuidBloomFilterCreationModes") + public Object[][] uuidBloomFilterCreationModes() { return new Object[][]{ - {DataType.INT, 42, "42"}, - {DataType.LONG, 42L, "42"}, - {DataType.DOUBLE, 21.0d, "21.0"}, - // Scientific notation is the case that breaks if the reader renders via toPlainString(). - {DataType.BIG_DECIMAL, new BigDecimal("1.0E-7"), "1.0E-7"}, - // NOTE: a BIG_DECIMAL literal carrying trailing zeros (e.g. 123.4500) is still wrongly pruned, because the - // query literal loses them before it reaches the pruner while the indexed value keeps its scale. That is a - // pre-existing gap on master, independent of the rendering fixed here, so it is deliberately not asserted. - - {DataType.STRING, "hello", "'hello'"}, - {DataType.BYTES, new byte[]{0x0a, 0x1b, 0x2c}, "'0a1b2c'"}, - {DataType.UUID, uuidBytes, "'550e8400-e29b-41d4-a716-446655440000'"} + {false, false, false}, + {true, false, false}, + {false, true, false}, + {true, true, false}, + {false, false, true}, + {true, false, true}, + {false, true, true}, + {true, true, true} }; } - private IndexSegment mockIndexSegmentIndexedViaCreator(DataType dataType, Object indexedValue) - throws IOException { - IndexSegment indexSegment = mock(IndexSegment.class); - when(indexSegment.getColumnNames()).thenReturn(ImmutableSet.of("column")); - SegmentMetadata segmentMetadata = mock(SegmentMetadata.class); - when(segmentMetadata.getTotalDocs()).thenReturn(20); - when(indexSegment.getSegmentMetadata()).thenReturn(segmentMetadata); - - DataSource dataSource = mock(DataSource.class); - when(indexSegment.getDataSourceNullable("column")).thenReturn(dataSource); - DataSourceMetadata dataSourceMetadata = mock(DataSourceMetadata.class); - when(dataSourceMetadata.getDataType()).thenReturn(dataType); - when(dataSource.getDataSourceMetadata()).thenReturn(dataSourceMetadata); - - // Route the value through BloomFilterCreator's own default add(Object, int) so the writer-side rendering under - // test is the production one, not a copy of it. - BloomFilterReaderBuilder builder = new BloomFilterReaderBuilder(); - BloomFilterCreator creator = new BloomFilterCreator() { - @Override - public DataType getDataType() { - return dataType; - } - - @Override - public void add(String value) { - builder.put(value); - } - - @Override - public void seal() { - } + private static TableConfig createTableConfig(boolean noDictionary, boolean enableBloomFilter) { + TableConfigBuilder builder = new TableConfigBuilder(TableType.OFFLINE).setTableName("testTable"); + if (noDictionary) { + builder.setNoDictionaryColumns(List.of(UUID_COLUMN)); + } + TableConfig tableConfig = builder.build(); + if (enableBloomFilter) { + tableConfig.getIndexingConfig().setBloomFilterConfigs( + Map.of(UUID_COLUMN, new BloomFilterConfig(1e-9, 0, false))); + } + return tableConfig; + } - @Override - public void close() { - } - }; - creator.add(indexedValue, -1); - when(dataSource.getBloomFilter()).thenReturn(builder.build()); + private static Schema createSchema(boolean multiValue) { + Schema.SchemaBuilder builder = new Schema.SchemaBuilder(); + if (multiValue) { + builder.addMultiValueDimension(UUID_COLUMN, DataType.UUID); + } else { + builder.addSingleValueDimension(UUID_COLUMN, DataType.UUID); + } + return builder.build(); + } - return indexSegment; + private static GenericRow row(String uuid, boolean multiValue) { + GenericRow row = new GenericRow(); + row.putValue(UUID_COLUMN, multiValue ? new String[]{uuid, UUID_3} : uuid); + return row; } @Test(expectedExceptions = RuntimeException.class) @@ -270,11 +280,6 @@ public void testIsApplicableTo() { private IndexSegment mockIndexSegment(String[] values) throws IOException { - return mockIndexSegment(values, DataType.DOUBLE); - } - - private IndexSegment mockIndexSegment(String[] values, DataType dataType) - throws IOException { IndexSegment indexSegment = mock(IndexSegment.class); when(indexSegment.getColumnNames()).thenReturn(ImmutableSet.of("column")); SegmentMetadata segmentMetadata = mock(SegmentMetadata.class); @@ -289,7 +294,7 @@ private IndexSegment mockIndexSegment(String[] values, DataType dataType) for (String v : values) { builder.put(v); } - when(dataSourceMetadata.getDataType()).thenReturn(dataType); + when(dataSourceMetadata.getDataType()).thenReturn(DataType.DOUBLE); when(dataSource.getDataSourceMetadata()).thenReturn(dataSourceMetadata); when(dataSource.getBloomFilter()).thenReturn(builder.build()); diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java index d95f509ef325..f59cce2bbf87 100644 --- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java +++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java @@ -130,6 +130,7 @@ private void createAndSealBloomFilterForNonDictionaryColumn(File indexDir, Colum BloomFilterConfig bloomFilterConfig, SegmentDirectory.Writer segmentWriter) throws Exception { int numDocs = columnMetadata.getTotalDocs(); + DataType storedType = columnMetadata.getDataType().getStoredType(); IndexCreationContext context = new IndexCreationContext.Builder(indexDir, _tableConfig, columnMetadata).build(); IndexReaderFactory readerFactory = StandardIndexes.forward().getReaderFactory(); try (BloomFilterCreator bloomFilterCreator = StandardIndexes.bloomFilter() @@ -139,7 +140,7 @@ private void createAndSealBloomFilterForNonDictionaryColumn(File indexDir, Colum ForwardIndexReaderContext readerContext = forwardIndexReader.createContext()) { if (columnMetadata.isSingleValue()) { // SV - switch (columnMetadata.getDataType()) { + switch (storedType) { case INT: for (int i = 0; i < numDocs; i++) { bloomFilterCreator.add(Integer.toString(forwardIndexReader.getInt(i, readerContext))); @@ -177,7 +178,7 @@ private void createAndSealBloomFilterForNonDictionaryColumn(File indexDir, Colum bloomFilterCreator.seal(); } else { // MV - switch (columnMetadata.getDataType()) { + switch (storedType) { case INT: for (int i = 0; i < numDocs; i++) { int[] buffer = new int[columnMetadata.getMaxNumberOfMultiValues()]; @@ -286,7 +287,7 @@ private void createBloomFilterForColumn(SegmentDirectory.Writer segmentWriter, C private Dictionary getDictionaryReader(ColumnMetadata columnMetadata, SegmentDirectory.Writer segmentWriter) throws IOException { - DataType dataType = columnMetadata.getDataType(); + DataType dataType = columnMetadata.getDataType().getStoredType(); switch (dataType) { case INT: diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/BloomFilterCreatorTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/BloomFilterCreatorTest.java index d81ef7fe1e9b..0ca62632fc40 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/BloomFilterCreatorTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/BloomFilterCreatorTest.java @@ -83,6 +83,8 @@ public void testUuidBloomFilterCreatorWithBytesValues() String columnName = "uuidColumn"; String uuid0 = "550e8400-e29b-41d4-a716-446655440000"; String uuid1 = "550e8400-e29b-41d4-a716-446655440001"; + String uuid0Hex = "550e8400e29b41d4a716446655440000"; + String uuid1Hex = "550e8400e29b41d4a716446655440001"; try (BloomFilterCreator bloomFilterCreator = new OnHeapGuavaBloomFilterCreator(TEMP_DIR, columnName, cardinality, new BloomFilterConfig(BloomFilterConfig.DEFAULT_FPP, 0, false), FieldSpec.DataType.UUID)) { bloomFilterCreator.add(UuidUtils.toBytes(uuid0), -1); @@ -94,12 +96,12 @@ public void testUuidBloomFilterCreatorWithBytesValues() try (PinotDataBuffer dataBuffer = PinotDataBuffer.mapReadOnlyBigEndianFile(bloomFilterFile); BloomFilterReader onHeapBloomFilter = BloomFilterReaderFactory.getBloomFilterReader(dataBuffer, true); BloomFilterReader offHeapBloomFilter = BloomFilterReaderFactory.getBloomFilterReader(dataBuffer, false)) { - Assert.assertTrue(onHeapBloomFilter.mightContain(uuid0)); - Assert.assertTrue(onHeapBloomFilter.mightContain(uuid1)); - Assert.assertFalse(onHeapBloomFilter.mightContain("550e8400-e29b-41d4-a716-4466554400ff")); - Assert.assertTrue(offHeapBloomFilter.mightContain(uuid0)); - Assert.assertTrue(offHeapBloomFilter.mightContain(uuid1)); - Assert.assertFalse(offHeapBloomFilter.mightContain("550e8400-e29b-41d4-a716-4466554400ff")); + Assert.assertTrue(onHeapBloomFilter.mightContain(uuid0Hex)); + Assert.assertTrue(onHeapBloomFilter.mightContain(uuid1Hex)); + Assert.assertFalse(onHeapBloomFilter.mightContain("550e8400e29b41d4a7164466554400ff")); + Assert.assertTrue(offHeapBloomFilter.mightContain(uuid0Hex)); + Assert.assertTrue(offHeapBloomFilter.mightContain(uuid1Hex)); + Assert.assertFalse(offHeapBloomFilter.mightContain("550e8400e29b41d4a7164466554400ff")); } } diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/BloomFilterCreator.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/BloomFilterCreator.java index 88970bb7703f..944ed8896a6b 100644 --- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/BloomFilterCreator.java +++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/BloomFilterCreator.java @@ -23,7 +23,6 @@ import org.apache.pinot.segment.spi.index.IndexCreator; import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.utils.BytesUtils; -import org.apache.pinot.spi.utils.UuidUtils; public interface BloomFilterCreator extends IndexCreator { @@ -32,10 +31,8 @@ public interface BloomFilterCreator extends IndexCreator { @Override default void add(Object value, int dictId) { - if (getDataType() == FieldSpec.DataType.BYTES) { + if (getDataType().getStoredType() == FieldSpec.DataType.BYTES) { add(BytesUtils.toHexString((byte[]) value)); - } else if (getDataType() == FieldSpec.DataType.UUID) { - add(uuidToCanonicalString(value)); } else { add(value.toString()); } @@ -43,14 +40,10 @@ default void add(Object value, int dictId) { @Override default void add(Object[] values, @Nullable int[] dictIds) { - if (getDataType() == FieldSpec.DataType.BYTES) { + if (getDataType().getStoredType() == FieldSpec.DataType.BYTES) { for (Object value : values) { add(BytesUtils.toHexString((byte[]) value)); } - } else if (getDataType() == FieldSpec.DataType.UUID) { - for (Object value : values) { - add(uuidToCanonicalString(value)); - } } else { for (Object value : values) { add(value.toString()); @@ -58,14 +51,6 @@ default void add(Object[] values, @Nullable int[] dictIds) { } } - /// Renders a UUID value (typically a 16-byte big-endian `byte[]` from segment ingest) as its canonical - /// lowercase RFC 4122 string. The `byte[]` fast path just skips the type dispatch of `UuidUtils.toBytes(Object)`; - /// neither path copies the buffer (`UuidUtils.toBytes(byte[])` validates the width and returns it as-is). - private static String uuidToCanonicalString(Object value) { - return value instanceof byte[] - ? UuidUtils.toString((byte[]) value) - : UuidUtils.toString(UuidUtils.toBytes(value)); - } /// Adds a value to the bloom filter. void add(String value); From 7be0d8165662ab0fe9c078412f2caea11bcafe3b Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Fri, 7 Aug 2026 15:09:26 -0700 Subject: [PATCH 4/5] [UUID 4b] Add Bloom filter query integration test 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. --- .../tests/custom/UuidBloomFilterTest.java | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidBloomFilterTest.java diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidBloomFilterTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidBloomFilterTest.java new file mode 100644 index 000000000000..bc201ed5e6e4 --- /dev/null +++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/UuidBloomFilterTest.java @@ -0,0 +1,124 @@ +/** + * 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.integration.tests.custom; + +import com.fasterxml.jackson.databind.JsonNode; +import java.io.File; +import java.util.List; +import java.util.Map; +import org.apache.avro.file.DataFileWriter; +import org.apache.avro.generic.GenericData; +import org.apache.pinot.spi.config.table.BloomFilterConfig; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.TableType; +import org.apache.pinot.spi.data.FieldSpec.DataType; +import org.apache.pinot.spi.data.Schema; +import org.apache.pinot.spi.utils.builder.TableConfigBuilder; +import org.testng.annotations.Test; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + + +/// End-to-end coverage for querying a UUID column backed by a Bloom filter. A single segment contains UUIDs ending in +/// `0000` and `0002`; the absent `0001` value is inside the segment min/max range, and the pruning metrics verify that +/// the Bloom value pruner eliminated it. +@Test(suiteName = "CustomClusterIntegrationTest") +public class UuidBloomFilterTest extends CustomDataQueryClusterIntegrationTest { + private static final String TABLE_NAME = "UuidBloomFilterTest"; + private static final String UUID_COLUMN = "uuidColumn"; + private static final String UUID_0 = "550e8400-e29b-41d4-a716-446655440000"; + private static final String UUID_0_HEX = "550e8400e29b41d4a716446655440000"; + private static final String UUID_1_HEX = "550e8400e29b41d4a716446655440001"; + private static final String UUID_2 = "550e8400-e29b-41d4-a716-446655440002"; + + @Override + public String getTableName() { + return TABLE_NAME; + } + + @Override + protected long getCountStarResult() { + return 2; + } + + @Override + public int getNumAvroFiles() { + return 1; + } + + @Override + public TableConfig createOfflineTableConfig() { + TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(getTableName()).build(); + tableConfig.getIndexingConfig().setBloomFilterConfigs( + Map.of(UUID_COLUMN, new BloomFilterConfig(1e-9, 0, false))); + return tableConfig; + } + + @Override + public Schema createSchema() { + return new Schema.SchemaBuilder().setSchemaName(getTableName()) + .addSingleValueDimension(UUID_COLUMN, DataType.UUID) + .build(); + } + + @Override + public List createAvroFiles() + throws Exception { + org.apache.avro.Schema avroSchema = org.apache.avro.Schema.createRecord("uuidRecord", null, null, false); + avroSchema.setFields(List.of(new org.apache.avro.Schema.Field(UUID_COLUMN, + org.apache.avro.Schema.create(org.apache.avro.Schema.Type.STRING), null, null))); + + try (AvroFilesAndWriters avroFilesAndWriters = createAvroFilesAndWriters(avroSchema)) { + DataFileWriter writer = avroFilesAndWriters.getWriters().get(0); + for (String uuid : List.of(UUID_0, UUID_2)) { + GenericData.Record record = new GenericData.Record(avroSchema); + record.put(UUID_COLUMN, uuid); + writer.append(record); + } + return avroFilesAndWriters.getAvroFiles(); + } + } + + @Test + public void testUuidBloomFilterQueries() + throws Exception { + setUseMultiStageQueryEngine(false); + + assertCountAndPrunedSegments( + String.format("SELECT COUNT(*) FROM %s WHERE %s = '%s'", getTableName(), UUID_COLUMN, UUID_0_HEX), 1, 1, 0); + assertCountAndPrunedSegments(String.format( + "SELECT COUNT(*) FROM %s WHERE %s = CAST('%s' AS UUID)", getTableName(), UUID_COLUMN, UUID_2), 1, 1, 0); + assertCountAndPrunedSegments( + String.format("SELECT COUNT(*) FROM %s WHERE %s = '%s'", getTableName(), UUID_COLUMN, UUID_1_HEX), 0, 0, 1); + } + + private void assertCountAndPrunedSegments(String query, long expectedCount, int expectedProcessedSegments, + int expectedPrunedSegments) + throws Exception { + JsonNode response = postQuery(query); + assertTrue(response.path("exceptions").isEmpty(), response.toPrettyString()); + assertEquals(response.path("resultTable").path("rows").path(0).path(0).asLong(), expectedCount, + response.toPrettyString()); + assertEquals(response.path("numSegmentsQueried").asInt(), 1, response.toPrettyString()); + assertEquals(response.path("numSegmentsProcessed").asInt(), expectedProcessedSegments, response.toPrettyString()); + assertEquals(response.path("numSegmentsPrunedByValue").asInt(), expectedPrunedSegments, + response.toPrettyString()); + } +} From 19a0bedd3d66614d8ee3d32c6d8b65d261f786a5 Mon Sep 17 00:00:00 2001 From: Xiang Fu Date: Fri, 7 Aug 2026 16:33:31 -0700 Subject: [PATCH 5/5] Rename bloom filter stored type variable --- .../index/loader/bloomfilter/BloomFilterHandler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java index f59cce2bbf87..8d5aa39e33a7 100644 --- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java +++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java @@ -287,9 +287,9 @@ private void createBloomFilterForColumn(SegmentDirectory.Writer segmentWriter, C private Dictionary getDictionaryReader(ColumnMetadata columnMetadata, SegmentDirectory.Writer segmentWriter) throws IOException { - DataType dataType = columnMetadata.getDataType().getStoredType(); + DataType storedType = columnMetadata.getDataType().getStoredType(); - switch (dataType) { + switch (storedType) { case INT: case LONG: case FLOAT: @@ -300,7 +300,7 @@ private Dictionary getDictionaryReader(ColumnMetadata columnMetadata, SegmentDir return DictionaryIndexType.read(buf, columnMetadata, DictionaryIndexConfig.DEFAULT); default: throw new IllegalStateException( - "Unsupported data type: " + dataType + " for column: " + columnMetadata.getColumnName()); + "Unsupported data type: " + storedType + " for column: " + columnMetadata.getColumnName()); } } }