leaves12138 commented on code in PR #10461:
URL: https://github.com/apache/paimon/pull/10461#discussion_r4227844806
##########
paimon-python/pypaimon/write/native_write.py:
##########
@@ -39,11 +39,13 @@ def native_write_available() -> bool:
return True
-def _native_partition_types_supported(schema, partition_keys):
+def _native_partition_types_supported(schema, partition_keys,
legacy_partition_name=True):
"""Partition keys which Rust can encode and use to locate existing
files."""
- return not any(
+ # Java's legacy byte[].toString() contains an allocation identity, so
binary
+ # partitions only have portable names with the cast-based option.
+ return not legacy_partition_name or not any(
pa.types.is_binary(data_type) or pa.types.is_large_binary(data_type)
- or pa.types.is_fixed_size_binary(data_type) or
pa.types.is_floating(data_type)
+ or pa.types.is_fixed_size_binary(data_type)
Review Comment:
[P2] Match Java non-finite names before enabling floating partitions
Admitting FLOAT/DOUBLE to native IO exposes a mismatch in
`_floating_partition_string()`: its non-finite branch returns Python `nan`,
`inf`, and `-inf`, whereas both Rust and Java use `NaN`, `Infinity`, and
`-Infinity`. I reproduced this with a DOUBLE-partitioned append table and
`partition.legacy-name=false`: native write/commit succeeds, but a subsequent
Python scan/read fails with `FileNotFoundError`, looking under `p=inf` while
the actual file is under `p=Infinity`. NaN and negative infinity fail the same
way; the finite controls succeed. `from_native_commit_messages()` also
reconstructs the wrong `file_path`. Please handle these three spellings
explicitly and add FLOAT/DOUBLE native-write/Python-read round trips; removing
old directory probing is not the problem.
##########
paimon-python/pypaimon/write/file_store_commit.py:
##########
@@ -1087,31 +1087,18 @@ def _generate_partition_statistics(self,
commit_entries: List[ManifestEntry]) ->
List of PartitionStatistics for each unique partition
"""
partition_stats = {}
+ path_factory = self.table.path_factory()
for entry in commit_entries:
- # Convert partition tuple to dictionary for PartitionStatistics
- partition_value = tuple(entry.partition.values) # Call the method
to get partition value
- if partition_value:
- # Assuming partition is a tuple and we need to convert it to a
dict
- # This may need adjustment based on actual partition format
- if isinstance(partition_value, tuple):
- # Create partition spec from partition tuple and table
partition keys
- partition_spec = {}
- if len(partition_value) == len(self.table.partition_keys):
- for i, key in enumerate(self.table.partition_keys):
- partition_spec[key] = str(partition_value[i])
- else:
- # Fallback: use indices as keys
- for i, value in enumerate(partition_value):
- partition_spec[f"partition_{i}"] = str(value)
- else:
- # If partition is already a dict or other format
- partition_spec = dict(partition_value) if partition_value
else {}
- else:
- # Default partition for unpartitioned tables
- partition_spec = {}
-
- partition_key = tuple(sorted(partition_spec.items()))
+ partition_value = tuple(entry.partition.values)
+ if len(partition_value) != len(self.table.partition_keys):
+ raise ValueError('Partition row does not match the table
partition keys')
+ # Java PartitionEntry.toPartitionStatistics reuses the path value
+ # computer, without path escaping. Group by typed partition values:
+ # NULL and whitespace remain distinct even if their names coincide.
+ partition_spec = dict(zip(self.table.partition_keys,
path_factory._canonical_partition(partition_value)))
+ partition_key = tuple(bytes(value) if isinstance(value,
(bytearray, memoryview)) else value
+ for value in partition_value)
Review Comment:
[P2] Preserve BinaryRow identity when grouping partition statistics
A tuple of ordinary Python values is not equivalent to the Java
`Map<BinaryRow, PartitionEntry>` key. In particular `(0.0,) == (-0.0,)`,
although these rows encode differently and name distinct `p=0.0`/`p=-0.0`
partitions. With two records in each, this head publishes only `{'p': '0.0',
record_count: 4}`; the base correctly produces two specs with count 2 each.
Independently decoded identical NaNs have the opposite problem: Python NaNs
compare unequal, so their statistics are not aggregated even when their
BinaryRow bytes are identical. Both cases reproduce for FLOAT and DOUBLE.
Please use a schema-aware binary partition identity, matching Java and Rust,
rather than Python scalar equality; retain the intended distinction between
NULL and whitespace.
--
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]
