Jackie-Jiang commented on code in PR #19182:
URL: https://github.com/apache/pinot/pull/19182#discussion_r3738525197
##########
pinot-core/src/main/java/org/apache/pinot/core/query/pruner/ValueBasedSegmentPruner.java:
##########
@@ -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)
Review Comment:
We need a canonical format. When query comes, we also convert the value into
canonical format. Should we just use hex as canonical format (i.e. use stored
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
