fix: FULL OUTER JOIN and LIMIT produces wrong results

[zhuqi-lucas]

Which issue does this PR close?

• Closes FULL OUTER JOIN and LIMIT produces wrong results #14335
• Closes Add test case for incorrect FULL OUTER JOIN + LIMIT pushdown @alamb #14336

Rationale for this change

LIMITs are incorrectly pushed through FULL OUTER Joins

What changes are included in this PR?

disable full outer join limit push down

Are these changes tested?

Added unit test, also the original unit test show right behaviour now.

Are there any user-facing changes?

Yes, FULL OUTER JOIN and LIMIT produces right results now.

[zhuqi-lucas]

cc @alamb @Dandandan
For the first step, i think we should disable full outer join push down limit, this PR done.
For further improvement, we can investigate if we have some cases which we can support push down limit for full outer join.

Thanks a lot!

And also added the test cases in #14336

It shows right result now.

[zhuqi-lucas]

I am confident now that we can remove the pushdown limit for full outer join, because i also checked the full outer join pushdown filter, we will not push down for full outer join for both side, code reference:

// Get pushable predicates from current optimizer state
let (left_preserved, right_preserved) = lr_is_preserved(join.join_type);

pub(crate) fn lr_is_preserved(join_type: JoinType) -> (bool, bool) {
    match join_type {
        JoinType::Inner => (true, true),
        JoinType::Left => (true, false),
        JoinType::Right => (false, true),
        JoinType::Full => (false, false),
        // No columns from the right side of the join can be referenced in output
        // predicates for semi/anti joins, so whether we specify t/f doesn't matter.
        JoinType::LeftSemi | JoinType::LeftAnti | JoinType::LeftMark => (true, false),
        // No columns from the left side of the join can be referenced in output
        // predicates for semi/anti joins, so whether we specify t/f doesn't matter.
        JoinType::RightSemi | JoinType::RightAnti => (false, true),
    }
}

[xudong963]

LGTM, pushing down limit to full join will introduce the issue of correctness, we should avoid it.

[xudong963]

I think the test will be flaky. (Maybe explain is enough)

[alamb]

I agree in theory the results from this query is undefined (in the sense that any two rows would be valid)

However, given we are limiting to a single core via

set datafusion.execution.target_partitions = 1;

I think this won't be flaky in practice

Of course I am somewhat biased as I wrote these tests

[xudong963]

oh, make sense, I didn't notice the setting

[alamb]

Thank you @zhuqi-lucas and @xudong963

[alamb]

I agree in theory the results from this query is undefined (in the sense that any two rows would be valid)

However, given we are limiting to a single core via

set datafusion.execution.target_partitions = 1;

I think this won't be flaky in practice

Of course I am somewhat biased as I wrote these tests

[xudong963]

Let's go

[zhuqi-lucas]

Thank you @alamb and @xudong963!