Skip to content

test: add skipped UPDATE ... FROM coverage for issue #19950#20800

Open
kosiew wants to merge 2 commits intoapache:mainfrom
kosiew:update-tests-19950
Open

test: add skipped UPDATE ... FROM coverage for issue #19950#20800
kosiew wants to merge 2 commits intoapache:mainfrom
kosiew:update-tests-19950

Conversation

@kosiew
Copy link
Contributor

@kosiew kosiew commented Mar 8, 2026

Which issue does this PR close?


Rationale for this change

#20745 contains behavioral changes with a substantial amount of new test coverage to close #19950, but has more than 1000 lines, which makes review difficult as a single PR. This PR isolates the test additions into a tests-only precursor so reviewers can evaluate the expected behavior separately from the implementation.

The intent is to land the coverage scaffolding first without changing current behavior. The newly extracted tests are intentionally skipped and annotated with TODOs referencing #19950 so they can be enabled when the implementation PR lands.


What changes are included in this PR?

This PR contains only test-side changes and the smallest compile-time scaffolding needed to carry those tests on this branch:

#[ignore = "TODO(19950): enable once the implementation PR lands"]
  • Adds TODO(19950) comments to each skipped Rust test.

  • Preserves the extracted SQLLogicTest coverage in:

datafusion/sqllogictest/test_files/update.slt

as commented-out skipped cases with TODO(19950) markers so the intended scenarios are present in the branch without being executed.

  • Adds only minimal test-support scaffolding:

    • shared test schema/helpers used by the extracted tests
    • test-only helper functions in physical_planner.rs
    • a private helper signature adjustment in physical_planner.rs so the extracted planner tests compile against the current branch shape

No implementation fix for UPDATE ... FROM is included here.


Are these changes tested?

Yes, the branch was verified to build with the newly added skipped tests in place using no-run test builds:

cargo test -p datafusion-sql --test sql_integration --no-run
cargo test -p datafusion --lib physical_planner::tests::test_create_not --no-run
cargo test -p datafusion --test core_integration --no-run
cargo test -p datafusion-sqllogictest --test sqllogictests --no-run

The newly extracted tests are intentionally skipped in this PR, so they compile but do not run until the implementation PR lands.


Are there any user-facing changes?

No. This PR is tests-only in intent and does not change existing runtime behavior.

LLM-generated code disclosure

This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed and tested.

Extract test cases from the original apache#20745 change set into a tests-only PR.

- add skipped Rust tests for UPDATE ... FROM planning and filter extraction
- add skipped SQL integration coverage for original target-row projection
- preserve SQLLogicTest cases as commented-out TODO(19950) coverage
- add only minimal compile-time test scaffolding, without the implementation fix
@github-actions github-actions bot added sql SQL Planner core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Mar 8, 2026
@kosiew kosiew marked this pull request as ready for review March 8, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UPDATE ...FROM bug

1 participant