leaves12138 commented on code in PR #9825:
URL: https://github.com/apache/paimon/pull/9825#discussion_r4011803826
##########
paimon-python/pypaimon/utils/file_store_path_factory.py:
##########
@@ -149,6 +160,44 @@ def global_index_path_factory(self) -> 'IndexPathFactory':
self.global_index_external_path is not None,
)
+ def new_bucket_index_path(self, partition: Tuple, bucket: int, file_name:
str) -> Tuple[str, bool]:
+ """Return a new bucket index's path and whether to persist its
external location."""
+ if self.index_file_in_data_file_dir:
+ external = self.create_external_path_provider(partition, bucket)
+ if external is not None:
+ return external.get_next_external_data_path(file_name), True
+ # Python data directories historically use str(value) without
+ # escaping. Record the actual location when Java renders it
+ # differently, so its readers can find the DV beside those files.
+ return (f"{self.bucket_path(partition, bucket)}/{file_name}",
+ self._partition_path_requires_explicit_location(partition))
+ factory = self.global_index_path_factory()
+ return factory.to_path(file_name), factory.is_external_path()
+
+ def _partition_path_requires_explicit_location(self, partition: Tuple) ->
bool:
+ # FLOAT/DOUBLE formatting can differ from Python's float repr, and
+ # this factory does not carry field types to distinguish the two.
+ return (any(isinstance(value, float) for value in partition)
+ or self.relative_bucket_path(partition, 0) !=
self.relative_bucket_path(partition, 0, True))
+
+ def bucket_index_path(self, partition: Tuple, bucket: int, index_file,
file_io=None) -> str:
+ """Resolve an existing bucket index, including the legacy Python DV
layout."""
+ if index_file.external_path:
+ return index_file.external_path
+ legacy_path = f"{self.index_path()}/{index_file.file_name}"
+ if not self.index_file_in_data_file_dir:
+ return legacy_path
+ path = f"{self.bucket_path(partition, bucket,
True)}/{index_file.file_name}"
Review Comment:
Verified fixed at ead0e87: the FLOAT and ordinary TIMESTAMP cases now
resolve their Java canonical DV paths, and the expanded typed-path regressions
pass. Resolving this thread. I found a separate remaining case for
TIMESTAMP_LTZ, which is reported in a new inline comment.
##########
paimon-python/pypaimon/utils/file_store_path_factory.py:
##########
@@ -98,7 +146,39 @@ def data_file_path(self) -> str:
return f"{self._root}/{self.data_file_path_directory}"
return self._root
- def relative_bucket_path(self, partition: Tuple, bucket: int) -> str:
+ def relative_bucket_path(self, partition: Tuple, bucket: int,
canonical_partition: bool = False) -> str:
+ if canonical_partition and partition:
+ partition = self._canonical_partition(partition)
+ return self._relative_bucket_path(partition, bucket,
canonical_partition)
+
+ def _canonical_partition(self, partition: Tuple) -> Tuple[str, ...]:
+ values = []
+ for i, value in enumerate(partition):
+ data_type = self.partition_types[i] if self.partition_types is not
None else None
+ type_name = str(data_type).split('(', 1)[0].split()[0]
+ if _is_null_or_whitespace_only(value):
+ text = self.default_part_value
+ elif type_name in ('FLOAT', 'REAL', 'DOUBLE'):
+ text = _floating_partition_string(value, type_name != 'DOUBLE')
+ elif self.legacy_partition_name and type_name == 'DATE':
+ text = str((value - date(1970, 1, 1)).days)
+ elif self.legacy_partition_name and
type_name.startswith('TIMESTAMP'):
+ text = value.isoformat(timespec='minutes')
+ if value.second or value.microsecond:
+ text = value.isoformat(timespec='microseconds' if
value.microsecond else 'seconds')
+ if value.microsecond and value.microsecond % 1000 == 0:
+ text = text[:-3]
Review Comment:
[P2] Normalize TIMESTAMP_LTZ before formatting the Java partition path
This branch also matches `TIMESTAMP_LTZ`, but
`GenericRowDeserializer._parse_timestamp()` returns UTC-aware datetimes for
that type. `isoformat()` therefore appends `+00:00`, whereas Java's legacy
`Timestamp.toString()` has no offset suffix. For `TIMESTAMP_LTZ(3)` at
`2026-09-15 12:00:00 UTC`, Java's default legacy directory is
`p=2026-09-15T12%3A00/`, but this resolver searches
`p=2026-09-15T12%3A00+00%3A00/`. The fractional branch also slices characters
off the timezone rather than the fraction: `.120000+00:00` becomes `.120000+00`.
The non-legacy mode has the same issue: `_is_unsupported()` excludes LTZ, so
it reaches `str(value)` and retains the offset instead of producing Java's
local-time/precision-based string. With both JVM and Python timezone set to
UTC, I verified Java's paths and reproduced four DV-read failures
(legacy/non-legacy, whole seconds/120 ms). The canonical DV exists, has no
`external_path`, and the old Python copy is absent, but `to_arrow()` raises
`FileNotFoundError`.
Please handle LTZ explicitly according to Java's legacy/non-legacy timezone
semantics before formatting the fraction, without appending an offset suffix.
Extend the Java canonical DV tests to `pa.timestamp('ms', tz='UTC')` in both
modes.
--
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]
