Skip to content

[python] supports reading data evolution tables with deletionVectorEnabled#8404

Merged
JingsongLi merged 5 commits into
apache:masterfrom
steFaiz:py_dv_for_de
Jul 1, 2026
Merged

[python] supports reading data evolution tables with deletionVectorEnabled#8404
JingsongLi merged 5 commits into
apache:masterfrom
steFaiz:py_dv_for_de

Conversation

@steFaiz

@steFaiz steFaiz commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Purpose

Parts of #8322

This PR supports reading path in python module.

Note:
Python is significantly different with java about the returned position of iterators.

  1. For java, each returned position is the absolute position of underlying file. It can be directly used to calculate row id
  2. For python, each returned position is just the absolute index of the return batch.

For exmaple, if select row id = 0, 10, 20, 30 from a file with range [0, 100]

  1. In java, the returned positions: 0, 10, 20, 30
  2. In python, the returned positions: 0, 1, 2, 3

Tests

UnitTests and E2ETests

@steFaiz steFaiz marked this pull request as draft July 1, 2026 03:40
@steFaiz steFaiz marked this pull request as ready for review July 1, 2026 06:52
i for i, position in enumerate(range(start, end))
if not self._deletion_vector.is_deleted(position)
]
return arrow_batch.take(pyarrow.array(keep_indices, type=pyarrow.int32()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can return a zero-row RecordBatch when every row in the current physical batch is deleted. In the data-evolution path, DataEvolutionMergeReader treats min_rows == 0 as EOF, so later non-empty batches from the same file are never read.

This differs from the Java implementation: ApplyDeletionVectorReader.readBatch() still returns an iterator, and deletion filtering happens inside next(), so an all-deleted batch does not terminate the reader. Could we keep reading here until the filtered batch is non-empty or the wrapped reader returns None, and add a multi-batch test where the first batch is fully deleted but the second batch still has live rows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is a bug! I've wrapped the related code with While loop. Also added a test.

for row_id in range(row_range.from_, row_range.to + 1)
]

def _is_deleted(self, row_id: int) -> bool:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all-deleted data-evolution groups can crash. This PR applies DV before MergeAllBatchReader; if every row in a field bunch is deleted, ApplyDeletionVectorReader.read_arrow_batch() returns None, all_batches stays empty, then ds.InMemoryDataset(self.merged_batch) receives None and raises TypeError. I reproduced it locally with a one-batch reader whose two rows are both deleted. Fix should return None before constructing InMemoryDataset when all_batches is empty, and add a test for “all rows in one data-evolution row-id group are deleted”.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Added:

  1. Python tests covering whole-range deleted
  2. Java e2e tests also add this case
  3. Add a test covering blobupdate + dv + row indices pushed

@JingsongLi

Copy link
Copy Markdown
Contributor

+1

@JingsongLi JingsongLi merged commit b31acd0 into apache:master Jul 1, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants