You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "JingsongLi (via GitHub)" <gi...@apache.org> on 2026/09/14 15:45:48 UTC

[PR] [python] Align native scan planning with Rust [paimon]

JingsongLi opened a new pull request, #9825:
URL: https://github.com/apache/paimon/pull/9825

   ### Purpose
   
   Use the planning APIs from apache/paimon-rust#831 (merged) to support native incremental and distributed scans in PyPaimon. Timestamp windows combine APPEND deltas into one plan so primary-key versions across commits are merged together; the ending snapshot supplies metadata and deletion vectors even when it contributes no APPEND files.
   
   - Add native append slices/shards, primary-key bucket shards, and Data Evolution row-position selection. Assign DE positions before explicit/global-index range pruning, and apply limits after distribution.
   - Preserve snapshot IDs for empty plans and explain output, validate branch resolution, and fall back to Python when the installed binding cannot support the scan.
   - Honor `index-file-in-data-file-dir` for deletion-vector writes, reads, and abort cleanup. Preserve explicit paths and legacy Python index locations, handle partition-directory differences, and retain multiple DV index files in one bucket.
   - Keep overlapping floating-point key bounds together during split planning, including conservative grouping for NaN bounds.
   
   The native planner remains optional and disabled by default. The README documents supported scans, binding requirements, and fallback behavior.
   
   ### Tests
   
   Passed locally with Python 3.11 and a newly built Rust wheel whose core and Python-binding sources match the merged Rust main:
   
   - Planner, adapter compatibility, incremental, distribution, key-bound, and deletion-vector path regressions: 165 passed.
   - Reader, parallel reader, and Data Evolution regressions: 174 passed in each of the Python and native planning modes; 9 Lance cases deselected in each mode. The native runs exercised 147 native plans, including ORC reads.
   - Flake8 and Python 3.6 grammar checks on all 24 changed Python files; `git diff --check`.
   
   The full Python dependency/version matrix and Java integration suite were not run locally.
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [python] Align native scan planning with Rust [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9825:
URL: https://github.com/apache/paimon/pull/9825#issuecomment-5673668105

   Re-open to trigger tests.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [python] Align native scan planning with Rust [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi closed pull request #9825: [python] Align native scan planning with Rust
URL: https://github.com/apache/paimon/pull/9825


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [python] Align native scan planning with Rust [paimon]

Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #9825:
URL: https://github.com/apache/paimon/pull/9825#discussion_r4011411232


##########
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:
   [P2] Use schema-aware formatting for Java bucket-local DV paths
   
   For a Java-written DV with `index-file-in-data-file-dir=true` and no `external_path`, `canonical_partition=True` only escapes Python `str(value)` and special-cases booleans; it does not reproduce Java's typed partition casts. I verified Java's `InternalRowPartitionComputer`/`PartitionPathUtils` output and reproduced read failures for both `FLOAT 0.1` (Java directory `p=0.1`, Python looks under `p=0.10000000149011612`) and `TIMESTAMP(3) '2026-09-15 12:00:00'` (Java uses `p=2026-09-15 12%3A00%3A00.000`, Python omits `.000`). None of the fallback paths locates the existing canonical DV, and reading raises `FileNotFoundError`. Rust also intentionally rejects floating-point partition formatting, so enabling native planning does not rescue the FLOAT case.
   
   Reproduction: extend `test_java_canonical_bucket_dv_without_external_path` to these typed partitions, place the valid DV bytes at the Java-generated canonical bucket path, clear its `external_path`, and remove the old Python-path copy. Both Python reads fail instead of returning the surviving rows. Please pass the partition field types into canonical path construction and use Java-compatible formatting, retaining the current explicit-path and legacy-path precedence, with regressions for Java-produced typed DV paths.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [python] Align native scan planning with Rust [paimon]

Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [python] Align native scan planning with Rust [paimon]

Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #9825:
URL: https://github.com/apache/paimon/pull/9825#discussion_r4012097610


##########
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:
   Verified fixed in `10b5984866bf3e7e23fc2f47fc2dfae1554be7f2`. All four original reproductions now pass. I additionally compared the paths directly with Java in UTC, Asia/Shanghai, and America/New_York, and verified canonical bucket-DV reads, repeated deletion, and historical reads with both Python and native planners using the binding from apache/paimon-rust#834. Resolving this thread.
   



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [python] Align native scan planning with Rust [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi closed pull request #9825: [python] Align native scan planning with Rust
URL: https://github.com/apache/paimon/pull/9825


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [python] Align native scan planning with Rust [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #9825:
URL: https://github.com/apache/paimon/pull/9825


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org