[py]: delta input snapshot test for deletion vectors#6347
Merged
Conversation
gz
approved these changes
May 29, 2026
f7c48f9 to
7f19065
Compare
mythical-fred
approved these changes
May 30, 2026
mythical-fred
left a comment
There was a problem hiding this comment.
Re-APPROVE on 7f19065 (force-push since my prior APPROVE on 768642e).
Material changes vs the version I approved:
- New test
test_delta_input_cdc_multi_key_order_byexercises the multi-keycdc_order_byparse path with a deliberate write order that flips the winner unless the second key (lsn asc) is honored. Solid construction — same-__feldera_ts, loser-first-on-disk is exactly the right adversarial setup. DeltaTestLocation.stableflag plus the updatedcleanup()docstring correctly avoid wiping the cached DV fixture between runs, and the rewritten "no-op on CI/MinIO" prose addresses gz's "what is a leaked key" thread.- Workflow exposes
CI_K8S_MINIO_*env vars so the platform job can reach the test bucket. Comment explains the mapping; OK.
Soft nits, none blocking:
_dv_fixture_builder.pypinsdelta-spark>=4.2,<5in two places (the docstring and theuv run --withcall). Easy drift target — consider a single_DELTA_SPARK_SPECconstant in the test module passed through to the subprocess.- The new multi-key test relies on parquet row order from
write_deltalakematching the order rows were supplied. If a future deltalake bump starts row-shuffling on write, the test silently becomes tautological. A short comment "if this stops failing without thelsn asctiebreaker, suspect deltalake row ordering" would warn future readers.
APPROVE stands.
7f19065 to
0582fa4
Compare
Adds an end-to-end test that the Feldera delta_table_input connector honors deletion vectors on snapshot reads: stage a Delta table with 200 rows, soft-delete half via a DV-aware DELETE, then assert the connector surfaces only the 100 active rows. PySpark is the only tool that can write DVs (delta-rs and the deltalake wheel can't), so the fixture builder runs in a `uv run --with delta-spark` subprocess. That subprocess only fires on cache miss, so Merge Queue runs that reuse the cached MinIO fixture never install Spark. Bump FIXTURE_VERSION to force a rebuild. Also fixes DeltaTestLocation.cleanup() to leave the directory alone when stable_subpath is set, otherwise the local cache was wiped after every run. if stable_subpath is set, we should not prefix it with pipeline name which contains commit hash, hence we use _fixtures prefix if its set Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
0582fa4 to
3447159
Compare
mythical-fred
approved these changes
Jun 1, 2026
mythical-fred
left a comment
There was a problem hiding this comment.
LGTM. Force-push picks up the gz/swanandx feedback cleanly: DeltaTestLocation.stable preserves the cached fixture across runs, the builder is split out as _dv_fixture_builder.py (underscore prefix keeps pytest from collecting it), the MinIO credentials are surfaced to the OSS platform job, and the staging/upload ordering (data files first, then _delta_log last) keeps a mid-upload reader from observing a log referencing a missing parquet. Fixture-versioned via FIXTURE_VERSION — bump invalidates cached MinIO copies. subprocess.run(..., check=True) is the right call for the Spark builder.
ryzhyk
approved these changes
Jun 1, 2026
gz
approved these changes
Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
check commit message plz
Describe Manual Test Plan
Ran it locally with
FELDERA_HOST=http://localhost:8080 uv run pytest -vv tests/platform/test_delta_input_deletion_vectors.pyChecklist
Breaking Changes?
No, just adding test