This is an automated email from the ASF dual-hosted git repository.

JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git


The following commit(s) were added to refs/heads/master by this push:
     new d208b698f7 [python] Skip non-overlapping manifest entries before 
DataFileMeta construction (#8387)
d208b698f7 is described below

commit d208b698f794816693876ce2e81fc0a3937a4a80
Author: XiaoHongbo <[email protected]>
AuthorDate: Tue Jun 30 17:20:32 2026 +0800

    [python] Skip non-overlapping manifest entries before DataFileMeta 
construction (#8387)
---
 .../pypaimon/manifest/manifest_file_manager.py     |  23 +--
 .../pypaimon/read/scanner/file_scanner.py          |  48 +++++-
 .../pypaimon/tests/test_early_row_range_filter.py  | 185 +++++++++++++++++++++
 3 files changed, 243 insertions(+), 13 deletions(-)

diff --git a/paimon-python/pypaimon/manifest/manifest_file_manager.py 
b/paimon-python/pypaimon/manifest/manifest_file_manager.py
index 3bd6dbb226..3e0ae9a866 100644
--- a/paimon-python/pypaimon/manifest/manifest_file_manager.py
+++ b/paimon-python/pypaimon/manifest/manifest_file_manager.py
@@ -54,12 +54,14 @@ class ManifestFileManager:
 
     def read_entries_parallel(self, manifest_files: List[ManifestFileMeta], 
manifest_entry_filter=None,
                               drop_stats=True, max_workers=8,
-                              early_entry_filter: Optional[Callable[[int, 
int], bool]] = None
+                              early_entry_filter: Optional[Callable[[int, 
int], bool]] = None,
+                              early_record_filter: Optional[Callable[[dict], 
bool]] = None
                               ) -> List[ManifestEntry]:
 
         def _process_single_manifest(manifest_file: ManifestFileMeta) -> 
List[ManifestEntry]:
             return self.read(manifest_file.file_name, manifest_entry_filter, 
drop_stats,
-                             early_entry_filter=early_entry_filter)
+                             early_entry_filter=early_entry_filter,
+                             early_record_filter=early_record_filter)
 
         def _entry_identifier(e: ManifestEntry) -> tuple:
             return (
@@ -90,17 +92,14 @@ class ManifestFileManager:
         return final_entries
 
     def read(self, manifest_file_name: str, manifest_entry_filter=None, 
drop_stats=True,
-             early_entry_filter: Optional[Callable[[int, int], bool]] = None
+             early_entry_filter: Optional[Callable[[int, int], bool]] = None,
+             early_record_filter: Optional[Callable[[dict], bool]] = None
              ) -> List[ManifestEntry]:
         """
-        early_entry_filter: optional ``(bucket, total_buckets) -> bool``
-        called immediately after the avro record is parsed. Mirrors
-        Java ``BucketFilter`` applied at the InternalRow stage in
-        ``ManifestEntryCache``: when it returns False, the entry's
-        ``_FILE`` block / partition / stats are never deserialized.
-        Caller is responsible for soundness (any non-pruning rule must
-        return True). The full ``manifest_entry_filter`` still runs on
-        the survivors.
+        early_entry_filter: ``(bucket, total_buckets) -> bool``, skip before 
deserializing _FILE.
+        early_record_filter: ``(fastavro record dict) -> bool``, skip before 
constructing
+            DataFileMeta. Separate from early_entry_filter because it operates 
on the full
+            record (can inspect _FILE sub-fields) rather than just 
bucket/total_buckets.
         """
         manifest_file_path = f"{self.manifest_path}/{manifest_file_name}"
 
@@ -120,6 +119,8 @@ class ManifestFileManager:
                 else:
                     if not early_entry_filter(bucket, total_buckets):
                         continue
+            if early_record_filter is not None and not 
early_record_filter(record):
+                continue
             file_dict = dict(record['_FILE'])
             key_dict = dict(file_dict['_KEY_STATS'])
             key_stats = SimpleStats(
diff --git a/paimon-python/pypaimon/read/scanner/file_scanner.py 
b/paimon-python/pypaimon/read/scanner/file_scanner.py
index 3facb2f7d5..fe46abd67b 100755
--- a/paimon-python/pypaimon/read/scanner/file_scanner.py
+++ b/paimon-python/pypaimon/read/scanner/file_scanner.py
@@ -101,6 +101,42 @@ def _row_ranges_from_predicate(predicate: 
Optional[Predicate]) -> Optional[List]
     return visit(predicate)
 
 
+def _build_early_row_range_filter(row_ranges):
+    """Skip entries whose row-id range doesn't intersect ``row_ranges``.
+
+    Runs on the raw fastavro record (OrderedDict) before the expensive
+    Python object construction (BinaryRow, GenericRow, SimpleStats,
+    DataFileMeta). fastavro has already parsed the Avro bytes; this
+    filter avoids the construction cost, not I/O or parsing.
+
+    Safe for DELETE entries because ADD and DELETE for the same file
+    share the same ``_FIRST_ROW_ID``.
+    """
+    if row_ranges is None or not row_ranges:
+        return None
+
+    from pypaimon.utils.range import Range
+
+    def _filter(record):
+        file_dict = record.get('_FILE')
+        if file_dict is None:
+            return True
+        first_row_id = file_dict.get('_FIRST_ROW_ID')
+        if first_row_id is None:
+            return True
+        row_count = file_dict.get('_ROW_COUNT')
+        if row_count is None:
+            return True
+        file_start = int(first_row_id)
+        file_end = file_start + int(row_count) - 1
+        for r in row_ranges:
+            if Range.intersect(file_start, file_end, r.from_, r.to):
+                return True
+        return False
+
+    return _filter
+
+
 def _filter_manifest_files_by_row_ranges(
         manifest_files: List[ManifestFileMeta],
         row_ranges: List) -> List[ManifestFileMeta]:
@@ -341,8 +377,9 @@ class FileScanner:
         if row_ranges is not None:
             manifest_files = 
_filter_manifest_files_by_row_ranges(manifest_files, row_ranges)
 
-        entries = self.read_manifest_entries(manifest_files)
+        entries = self.read_manifest_entries(manifest_files, 
row_ranges=row_ranges)
 
+        # Redundant when early_record_filter ran; kept for explain mode and as 
safety net.
         if row_ranges is not None:
             entries = _filter_manifest_entries_by_row_ranges(entries, 
row_ranges)
 
@@ -391,7 +428,8 @@ class FileScanner:
         except Exception:
             return None
 
-    def read_manifest_entries(self, manifest_files: List[ManifestFileMeta]) -> 
List[ManifestEntry]:
+    def read_manifest_entries(self, manifest_files: List[ManifestFileMeta],
+                              row_ranges=None) -> List[ManifestEntry]:
         max_workers = 
self.table.options.scan_manifest_parallelism(os.cpu_count() or 8)
         if self.scan_stats is not None:
             self.scan_stats.manifest_files_total += len(manifest_files)
@@ -407,11 +445,17 @@ class FileScanner:
             self.scan_stats.manifest_files_after_partition += 
len(manifest_files)
             # Force single-threaded so we can mutate stats without locking.
             max_workers = 1
+        # Disable early_record_filter when scan_stats is active (explain mode)
+        # so that all entries flow through _filter_manifest_entry for accurate
+        # funnel counting.
+        early_row_filter = None if self.scan_stats is not None \
+            else _build_early_row_range_filter(row_ranges)
         return self.manifest_file_manager.read_entries_parallel(
             manifest_files,
             self._filter_manifest_entry,
             max_workers=max_workers,
             early_entry_filter=self._build_early_bucket_filter(),
+            early_record_filter=early_row_filter,
         )
 
     def _build_early_bucket_filter(self):
diff --git a/paimon-python/pypaimon/tests/test_early_row_range_filter.py 
b/paimon-python/pypaimon/tests/test_early_row_range_filter.py
new file mode 100644
index 0000000000..2253518a9f
--- /dev/null
+++ b/paimon-python/pypaimon/tests/test_early_row_range_filter.py
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+import tempfile
+import unittest
+from unittest.mock import patch
+
+import pyarrow as pa
+
+from pypaimon import CatalogFactory, Schema
+from pypaimon.common.predicate import Predicate
+from pypaimon.manifest.schema.data_file_meta import DataFileMeta
+
+
+class TestManifestReadRowRangePerformance(unittest.TestCase):
+    """Reproduce: scan planning constructs ALL manifest entries even when
+    row_ranges only match a few files."""
+
+    NUM_COMMITS = 20
+    ROWS_PER_COMMIT = 10
+
+    @classmethod
+    def setUpClass(cls):
+        cls.tempdir = tempfile.mkdtemp()
+        cls.warehouse = os.path.join(cls.tempdir, 'warehouse')
+        cls.catalog = CatalogFactory.create({'warehouse': cls.warehouse})
+        cls.catalog.create_database('default', True)
+
+        pa_schema = pa.schema([
+            ('id', pa.int32()),
+            ('value', pa.string()),
+        ])
+        schema = Schema.from_pyarrow_schema(pa_schema, options={
+            'row-tracking.enabled': 'true',
+            'data-evolution.enabled': 'true',
+            'manifest.merge-min-count': '1',
+        })
+        cls.catalog.create_table('default.test_row_range_perf', schema, False)
+        cls.table = cls.catalog.get_table('default.test_row_range_perf')
+        cls.pa_schema = pa_schema
+
+        write_builder = cls.table.new_batch_write_builder()
+        for i in range(cls.NUM_COMMITS):
+            tw = write_builder.new_write()
+            tc = write_builder.new_commit()
+            start_id = i * cls.ROWS_PER_COMMIT
+            data = pa.Table.from_pydict({
+                'id': list(range(start_id, start_id + cls.ROWS_PER_COMMIT)),
+                'value': [f'v{j}' for j in range(cls.ROWS_PER_COMMIT)],
+            }, schema=pa_schema)
+            tw.write_arrow(data)
+            cmts = tw.prepare_commit()
+            for msg in cmts:
+                for nf in msg.new_files:
+                    nf.first_row_id = start_id
+            tc.commit(cmts)
+            tw.close()
+            tc.close()
+
+    @classmethod
+    def tearDownClass(cls):
+        shutil.rmtree(cls.tempdir, ignore_errors=True)
+
+    def test_scan_constructs_all_entries_without_early_row_range_filter(self):
+        """With manifest.merge-min-count=1, all entries are in one manifest.
+        Querying with _ROW_ID BETWEEN 5 AND 14 should return 2 files, but
+        the current code constructs DataFileMeta for ALL 20 entries because
+        the row-range filter runs after full object construction."""
+        construction_count = [0]
+        original_init = DataFileMeta.__init__
+
+        def counting_init(self_meta, *args, **kwargs):
+            construction_count[0] += 1
+            original_init(self_meta, *args, **kwargs)
+
+        predicate = Predicate(method='between', index=None,
+                              field='_ROW_ID', literals=[5, 14])
+
+        with patch.object(DataFileMeta, '__init__', counting_init):
+            rb = self.table.new_read_builder().with_filter(predicate)
+            splits = rb.new_scan().plan().splits()
+
+        total_files = sum(len(s.files) for s in splits)
+        self.assertEqual(total_files, 2)
+
+        actual = self.table.new_read_builder().with_filter(predicate) \
+            .new_read().to_arrow(splits)
+        self.assertEqual(sorted(actual.column('id').to_pylist()),
+                         list(range(5, 15)))
+
+        # 2 matching files × 2 (ADD + DELETE from manifest merge) = 4
+        self.assertLessEqual(
+            construction_count[0], 2 * total_files,
+            f"Expected at most {2 * total_files} DataFileMeta constructions, "
+            f"got {construction_count[0]}")
+
+    def test_add_delete_pair_not_broken_by_early_filter(self):
+        """Guard ADD/DELETE merge semantics: when a file is overwritten,
+        its ADD and DELETE entries share the same _FIRST_ROW_ID. The early
+        filter must either keep both or drop both — never keep ADD but
+        drop DELETE (which would resurrect a deleted file)."""
+        pa_schema = self.pa_schema
+        self.catalog.create_table('default.test_add_delete_pair',
+                                  Schema.from_pyarrow_schema(pa_schema, 
options={
+                                      'row-tracking.enabled': 'true',
+                                      'data-evolution.enabled': 'true',
+                                      'manifest.merge-min-count': '1',
+                                  }), False)
+        table = self.catalog.get_table('default.test_add_delete_pair')
+        wb = table.new_batch_write_builder()
+
+        # Commit 1: write file at row_id 0-9
+        tw, tc = wb.new_write(), wb.new_commit()
+        tw.write_arrow(pa.Table.from_pydict(
+            {'id': list(range(10)), 'value': ['old'] * 10}, schema=pa_schema))
+        cmts = tw.prepare_commit()
+        for m in cmts:
+            for nf in m.new_files:
+                nf.first_row_id = 0
+        tc.commit(cmts)
+        tw.close()
+        tc.close()
+
+        # Commit 2: overwrite same row_id 0-9 with new data
+        # This creates DELETE(old_file, row_id=0) + ADD(new_file, row_id=0)
+        tw = wb.new_write().with_write_type(['id', 'value'])
+        tc = wb.new_commit()
+        tw.write_arrow(pa.Table.from_pydict(
+            {'id': list(range(10)), 'value': ['new'] * 10}, schema=pa_schema))
+        cmts = tw.prepare_commit()
+        for m in cmts:
+            for nf in m.new_files:
+                nf.first_row_id = 0
+        tc.commit(cmts)
+        tw.close()
+        tc.close()
+
+        # Commit 3: write file at row_id 100-109 (unrelated, outside query 
range)
+        tw, tc = wb.new_write(), wb.new_commit()
+        tw.write_arrow(pa.Table.from_pydict(
+            {'id': list(range(100, 110)), 'value': ['other'] * 10}, 
schema=pa_schema))
+        cmts = tw.prepare_commit()
+        for m in cmts:
+            for nf in m.new_files:
+                nf.first_row_id = 100
+        tc.commit(cmts)
+        tw.close()
+        tc.close()
+
+        # Query row_id [0, 9]: should return the NEW data, not the old
+        predicate = Predicate(method='between', index=None,
+                              field='_ROW_ID', literals=[0, 9])
+        rb = table.new_read_builder().with_filter(predicate)
+        splits = rb.new_scan().plan().splits()
+        actual = rb.new_read().to_arrow(splits)
+
+        values = actual.column('value').to_pylist()
+        self.assertTrue(all(v == 'new' for v in values),
+                        f"Expected all 'new' but got {values}. "
+                        "Early filter may have dropped DELETE without its 
ADD.")
+
+        # Also verify row_id [100, 109] is NOT in the result
+        ids = actual.column('id').to_pylist()
+        self.assertTrue(all(i < 100 for i in ids),
+                        f"row_id [100,109] should be filtered out but got 
ids={ids}")
+
+
+if __name__ == '__main__':
+    unittest.main()

Reply via email to