Skip to content

[SkipRecovery] Re-enable mixed-delete tests on Iceberg 1.10.2+ [reduced-it] [databricks] - #15964

Open
wjxiz1992 wants to merge 5 commits into
NVIDIA:mainfrom
wjxiz1992:codex/skip12885-iceberg111
Open

[SkipRecovery] Re-enable mixed-delete tests on Iceberg 1.10.2+ [reduced-it] [databricks]#15964
wjxiz1992 wants to merge 5 commits into
NVIDIA:mainfrom
wjxiz1992:codex/skip12885-iceberg111

Conversation

@wjxiz1992

@wjxiz1992 wjxiz1992 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

JaCoCo production line coverage: not fully measurable locally — no compatible Scala 2.13 / shim 411 nightly baseline is published.

Contributes to #12885.

Description

This test-only recovery restores enforced CPU/GPU result checks for mixed Iceberg position and equality deletes on Iceberg 1.10.2 and newer. Before this change, the format-v2 and format-v3 cases were permanent expected failures with fixed data seeds even on runtimes containing the upstream equality-delete cache fix. The affected older releases are now skipped instead of spending test time on known failures; production behavior is unchanged.

Apache Iceberg #15514, released in 1.11.0 and backported to 1.10.2, fixes equality-delete records being cached in a query-dependent field order. This PR:

  • Skips both mixed-delete tests on Iceberg versions older than 1.10.2, removes their expected-failure markers and permanent seed overrides on fixed versions, and fails pytest configuration when --iceberg lacks an expected runtime version.
  • Passes the expected Iceberg version through the Spark 4.0 extra-classpath pre-merge launcher and documents the required environment variable.
  • Keeps every count, full-row, and GPU-scan assertion. For the format-v3 count query only, disables adaptive query execution so plan validation sees the completed scan, then resets session configuration before fixture cleanup.

Older supported dependencies are not fixed by this PR, so #12885 should remain open. Iceberg 1.10.2 was policy-checked but was not GPU-runtime-tested locally. Shared pytest configuration and pre-merge launcher paths changed, so Databricks remains in CI scope.

Validation

  • Spark 4.1.1 / Iceberg 1.11.0 / Python 3.10: all six focused format-v2/format-v3 CPU/GPU cases passed across the three reader modes (6 passed, 45 deselected in 219.85 seconds). Full-row equality and the format-v3 GpuBatchScanExec assertion were exercised.
  • Spark 4.0.0 / Iceberg 1.10.1 / Python 3.10: all six focused cases were selected and skipped (6 skipped, 45 deselected in 1.73 seconds).
  • Seven release-boundary checks and two decorator-policy checks passed. The checked cases cover missing version, 1.6.1, 1.9.2, 1.10.0, 1.10.1, 1.10.2, and 1.11.0.
  • Unset EXPECTED_ICEBERG_VERSION with --iceberg failed during pytest configuration with exit 4 and the expected error. bash -n and a focused launcher probe confirmed that the Spark 4.0 extra-classpath path forwards Iceberg 1.10.1.
  • Python 3.10 compilation and git diff --check passed.

No production performance change is expected.

AI assistance: The change and PR description were prepared with Codex assistance.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (Please provide the names of the existing tests in the PR description.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Signed-off-by: Allen Xu <allxu@nvidia.com>
Copilot AI lite review requested due to automatic review settings September 11, 2026 04:00
@wjxiz1992 wjxiz1992 added the test Only impacts tests label Sep 11, 2026
@wjxiz1992 wjxiz1992 self-assigned this Sep 11, 2026

Copilot AI left a comment

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.

🟢 Approval recommended

No unresolved blocking issues were identified.

Pull request overview

Re-enables mixed Iceberg delete validation for Iceberg 1.11+ while preserving quarantine behavior for older or unknown versions.

Changes:

  • Gates xfail and fixed-seed markers by Iceberg version.
  • Disables AQE for the v3 count-plan assertion.
  • Resets Spark configuration before cleanup.
File summaries
File Description
integration_tests/src/main/python/iceberg/iceberg_merge_on_read_test.py Updates mixed-delete test gating and session cleanup.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness or repository-rule violations remain.

Findings

  1. P1 Pre-merge Iceberg run fails

Summary

  • Requires an explicit expected Iceberg version when Iceberg tests are enabled.
  • Propagates that version through the Spark 4.0 extra-classpath pre-merge launcher.
  • Skips affected mixed-delete cases before Iceberg 1.10.2 and restores normal assertions on fixed versions.
  • Disables AQE for the v3 count-plan assertion and resets session configuration before fixture cleanup.

Reviews (5) · Last reviewed commit: "Skip mixed-delete tests on affected Iceb..."

@wjxiz1992

Copy link
Copy Markdown
Collaborator Author

build


pytestmark = iceberg_unsupported_mark

# Iceberg 1.11.0 fixes cached equality-delete records being interpreted in the wrong field order:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can the quarantine gate account for Iceberg 1.10.2 as a fixed runtime? Apache backported the equality-delete schema-ordering fix to the 1.10.x branch and released it in 1.10.2.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

Signed-off-by: Allen Xu <allxu@nvidia.com>
@wjxiz1992 wjxiz1992 changed the title [SkipRecovery] Re-enable mixed-delete tests on Iceberg 1.11+ [reduced-it] [SkipRecovery] Re-enable mixed-delete tests on Iceberg 1.10.2+ [reduced-it] Sep 11, 2026
@wjxiz1992

Copy link
Copy Markdown
Collaborator Author

build

firestarman
firestarman previously approved these changes Sep 11, 2026
@wjxiz1992

Copy link
Copy Markdown
Collaborator Author

build

Comment thread integration_tests/src/main/python/iceberg/iceberg_merge_on_read_test.py Outdated
# field order: https://github.com/apache/iceberg/pull/15514 (backport: #15605).
# Keep the quarantine for older/unknown runtimes.
_iceberg_eq_delete_cache_bug = (
runtime_iceberg_version is None or

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should throw exception if runtime_iceberg_version not detected?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

Signed-off-by: Allen Xu <allxu@nvidia.com>
Comment on lines +398 to +400
if config.getoption('iceberg') and not os.environ.get('EXPECTED_ICEBERG_VERSION'):
raise pytest.UsageError(
"EXPECTED_ICEBERG_VERSION must be set when running Iceberg tests")

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.

P1 Pre-merge Iceberg run fails

The new check rejects every --iceberg invocation that does not set EXPECTED_ICEBERG_VERSION. The existing run_iceberg_extra_classpath_tests path passes --iceberg without exporting that variable, even though it already receives the exact Iceberg version. As a result, the Spark 4.0.x extra-classpath pre-merge run exits with UsageError during pytest configuration before collecting any tests. Please export the function's iceberg_version argument as EXPECTED_ICEBERG_VERSION in that launcher.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

@wjxiz1992

Copy link
Copy Markdown
Collaborator Author

build

Signed-off-by: Allen Xu <allxu@nvidia.com>
@wjxiz1992
wjxiz1992 requested a review from a team as a code owner September 14, 2026 04:16
Signed-off-by: Allen Xu <allxu@nvidia.com>
@wjxiz1992 wjxiz1992 changed the title [SkipRecovery] Re-enable mixed-delete tests on Iceberg 1.10.2+ [reduced-it] [SkipRecovery] Re-enable mixed-delete tests on Iceberg 1.10.2+ [reduced-it] [databricks] Sep 14, 2026
@wjxiz1992

Copy link
Copy Markdown
Collaborator Author

build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Only impacts tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants