Skip to content

feat: explicit implementation for union's required_input_ordering#9867

Closed
NGA-TRAN wants to merge 1 commit intoapache:mainfrom
NGA-TRAN:ntran/union_required_input_ordering
Closed

feat: explicit implementation for union's required_input_ordering#9867
NGA-TRAN wants to merge 1 commit intoapache:mainfrom
NGA-TRAN:ntran/union_required_input_ordering

Conversation

@NGA-TRAN
Copy link
Contributor

@NGA-TRAN NGA-TRAN commented Mar 29, 2024

Which issue does this PR close?

Closes ##9898

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@NGA-TRAN
Copy link
Contributor Author

Here are the failed tests. Will continue investigating and fixing them

failures:
    physical_optimizer::enforce_sorting::tests::test_union_inputs_different_sorted3
    physical_optimizer::enforce_sorting::tests::test_union_inputs_different_sorted5
    physical_optimizer::enforce_sorting::tests::test_union_inputs_different_sorted6
    physical_optimizer::enforce_sorting::tests::test_union_inputs_different_sorted7
    physical_optimizer::enforce_sorting::tests::test_union_inputs_different_sorted_with_limit
    physical_optimizer::enforce_sorting::tests::test_window_multi_path_sort
    physical_optimizer::enforce_sorting::tests::test_window_multi_path_sort2

@alamb
Copy link
Contributor

alamb commented Mar 30, 2024

Thank you @NGA-TRAN -- it might also help when you get a chance to file a ticket explaining the end user visible behavior of this (like it allows the union sortedness to be maintained, for example) 🤔

@alamb alamb marked this pull request as draft March 30, 2024 20:19
@alamb
Copy link
Contributor

alamb commented Mar 30, 2024

Marking as draft as I think this PR is not waiting on feedback. Please mark it as ready for review when it is ready for another look

@github-actions
Copy link

github-actions bot commented Jun 1, 2024

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale PR has not had any activity for some time label Jun 1, 2024
@github-actions github-actions bot closed this Jun 9, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 3, 2025
## Which issue does this PR close?

None, but relates to issue #9898

## Rationale for this change

N/A

## What changes are included in this PR?

This PR adds reproducer tests demonstrating issues with suboptimal
optimizations performed on plans that mix pre-sorted parquets and
`SortExec` under an UNION.

Two sets of tests included:
- Unit tests in
`datafusion/core/tests/physical_optimizer/enforce_sorting.rs`
- E2E-ish tests in `datafusion/core/tests/dataframe/mod.rs`, starting
from logical plans simulating our use-case

> [!NOTE]
> These tests pass with the changes from #9867

## Are these changes tested?

N/A

## Are there any user-facing changes?

N/A

---------

Co-authored-by: Nga Tran <nga-tran@live.com>
codetyri0n pushed a commit to codetyri0n/datafusion that referenced this pull request Nov 11, 2025
…8352)

## Which issue does this PR close?

None, but relates to issue apache#9898

## Rationale for this change

N/A

## What changes are included in this PR?

This PR adds reproducer tests demonstrating issues with suboptimal
optimizations performed on plans that mix pre-sorted parquets and
`SortExec` under an UNION.

Two sets of tests included:
- Unit tests in
`datafusion/core/tests/physical_optimizer/enforce_sorting.rs`
- E2E-ish tests in `datafusion/core/tests/dataframe/mod.rs`, starting
from logical plans simulating our use-case

> [!NOTE]
> These tests pass with the changes from apache#9867

## Are these changes tested?

N/A

## Are there any user-facing changes?

N/A

---------

Co-authored-by: Nga Tran <nga-tran@live.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale PR has not had any activity for some time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants