[SPARK-19712][SQL][FOLLOW-UP] Don't do partial pushdown when pushing down LeftAnti joins below Aggregate or Window operators.

[dilipbiswal]

What changes were proposed in this pull request?

After 23750, we may pushdown left anti joins below aggregate and window operators with a partial join condition. This is not correct and was pointed out by @hvanhovell and @cloud-fan here. This pr addresses their comments.

How was this patch tested?

Added two new tests to verify the behaviour.

[SparkQA]

Test build #104118 has finished for PR 24253 at commit 508a39c.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[dilipbiswal]

@cloud-fan @hvanhovell

[cloud-fan]

shall we move the part of the comments about diffrent join type to

joinType match {
  case LeftSemi => Filter(stayUp.reduce(And), newAgg)
  case _ => join
}

[dilipbiswal]

@cloud-fan Ok.. sounds good.

[cloud-fan]

how about

joinType match {
  // In case of left-semi join, ...
  case LeftSemi => Filter(stayUp.reduce(And), newPlan)
  // In case of left-anti join, ...
  case _ => join

[dilipbiswal]

@cloud-fan I think it would look a bit crowded :-). The actual code may look buried within the comment. Let me give it a try and we can a take a call after ..

[dilipbiswal]

@cloud-fan Just made the change... Let me know if looks okay to you.

[SparkQA]

Test build #104157 has finished for PR 24253 at commit 736710c.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #104158 has finished for PR 24253 at commit 5037d51.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

can we restore this comment now? I think the old version is pretty good.

[cloud-fan]

ditto

[SparkQA]

Test build #104164 has finished for PR 24253 at commit 4851daf.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

@dilipbiswal Did we cover all the places? in pushDownJoin seems we still push down partial join?

[dilipbiswal]

@cloud-fan Thanks a lot. I did miss it. Let me look into it.

[SparkQA]

Test build #104179 has finished for PR 24253 at commit 82112a8.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[dongjoon-hyun]

Oh, the root cause of UT failures seems to be another commit on the master branch.

[dilipbiswal]

@dongjoon-hyun Thank you. Will the current run fail as well ?

[dongjoon-hyun]

I guess so.

[dilipbiswal]

@dongjoon-hyun ok.. thanks a lot for letting me know :-)

[dongjoon-hyun]

Retest this please.

[SparkQA]

Test build #104192 has finished for PR 24253 at commit 72a4552.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #104196 has finished for PR 24253 at commit 72a4552.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

thanks, merging to master!