leaves12138 commented on code in PR #9946:
URL: https://github.com/apache/paimon/pull/9946#discussion_r4043394313
##########
paimon-python/pypaimon/globalindex/btree/btree_index_writer.py:
##########
@@ -100,15 +106,23 @@ def finish(self) -> bytes:
class _SstFileWriter:
- def __init__(self, out: PositionOutput, block_size: int):
+ def __init__(
+ self,
+ out: PositionOutput,
+ block_size: int,
+ bloom_filter_enabled: bool,
+ ):
self._out = out
self._block_size = block_size
self._data_block_writer = _BlockWriter()
self._index_block_writer = _BlockWriter()
self._last_key: Optional[bytes] = None
+ self._bloom_hashes = array('i') if bloom_filter_enabled else None
def put(self, key: bytes, value: bytes) -> None:
self._data_block_writer.add(key, value)
+ if self._bloom_hashes is not None:
+ self._bloom_hashes.append(murmur_hash_bytes(key))
Review Comment:
[P2] Canonicalize floating-point NaNs before inserting/probing the Bloom
filter
Hashing the serialized bytes here introduces false negatives for
`FLOAT`/`DOUBLE` NaNs. Python's `FloatSerializer`/`DoubleSerializer` use
`struct.pack`, which preserves the NaN sign/payload, whereas Java's
`Float.floatToIntBits`/`Double.doubleToLongBits` canonicalize all NaNs. The SST
comparators treat these NaN representations as equal, but their Bloom hashes
differ.
I reproduced this with both types: write keys `0.0` through `63.0`, followed
by two rows containing `float('-nan')` (row IDs 128 and 129). With Bloom
disabled, both Java and Python readers return `[128, 129]` for equality/`IN`
lookups using a canonical NaN. With Bloom enabled on the Python writer, both
readers return an empty result. Noncanonical NaN payloads reproduce it too. The
reverse direction also fails: a Python lookup using `float('-nan')` misses the
NaN rows in a Java-written Bloom-enabled file, while the Bloom-disabled control
returns them.
Could we normalize Python FLOAT/DOUBLE serialization to Java's canonical NaN
bit patterns (`0x7fc00000` / `0x7ff8000000000000`) on both writing and lookup,
and add regression coverage for equality/`IN` and cross-language reads? Without
that, enabling this optional optimization changes query results instead of only
reducing I/O.
--
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]
