leaves12138 commented on code in PR #8846:
URL: https://github.com/apache/paimon/pull/8846#discussion_r3661932845
##########
paimon-python/pypaimon/read/scanner/chunk_shuffle_split_generator.py:
##########
@@ -225,13 +418,18 @@ def _chunk_to_split(self, chunk: _Chunk) -> Split:
# set_file_path is already done once per unique file in
# ChunkShuffleSplitGeneratorBase.create_splits.
+ data_deletion_files = self._get_deletion_files_for_split(
+ files,
+ chunk.partition,
+ chunk.bucket,
+ )
data_split = DataSplit(
files=files,
partition=chunk.partition,
bucket=chunk.bucket,
raw_convertible=True,
- data_deletion_files=None,
+ data_deletion_files=data_deletion_files,
Review Comment:
`SlicedSplit.merged_row_count()` cannot derive the live count of this
physical slice from the full-file DV cardinality; it proportionally scales and
truncates. For a 10-row file with DV positions `{0, 3, 4, 9}` and
`chunk_size=3`, the two chunks return 3 live rows each, but report merged
counts 3 and 2. Please carry the exact `live_row_count` computed by the planner
into the split and add a metadata assertion.
##########
paimon-python/pypaimon/read/scanner/chunk_shuffle_split_generator.py:
##########
@@ -334,12 +562,17 @@ def _chunk_to_split(self, chunk: _Chunk) -> Split:
row_ranges.append(seg.row_range)
row_ranges.sort(key=lambda r: r.from_)
+ data_deletion_files = self._get_deletion_files_for_split(
+ all_files,
+ chunk.partition,
+ chunk.bucket,
+ )
data_split = DataSplit(
files=all_files,
partition=chunk.partition,
bucket=chunk.bucket,
raw_convertible=False,
- data_deletion_files=None,
+ data_deletion_files=data_deletion_files,
Review Comment:
`IndexedSplit.merged_row_count()` is just the sum of these physical row
ranges, so attaching a DV here makes DE chunks report deleted rows as live.
With a 10-row range and four deletions, two actual 3-row chunks each report 5.
This propagates incorrect counts to explain/Ray metadata. Please preserve the
exact live count for each generated chunk and test it.
##########
paimon-python/pypaimon/read/scanner/chunk_shuffle_split_generator.py:
##########
@@ -86,8 +202,13 @@ def __init__(
super().__init__(table, target_split_size, open_file_cost,
deletion_files_map)
self.seed = seed
self.chunk_size = chunk_size
+ # Planning-only cache. Readers continue to load DVs through their
+ # existing split-local factories.
+ self._deletion_vector_cache = {}
Review Comment:
This cache is unbounded for the lifetime of planning. In a normal
`create_splits` pass each DV descriptor is consumed only once, yet every
decoded bitmap remains reachable until all chunks have been materialized. On a
large DV-enabled training table this makes driver memory scale with the total
DV cardinality and can OOM planning. Please avoid retaining all bitmaps, or use
a bounded/evicting cache; reading a descriptor without retaining it is
sufficient for the common path.
--
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]
