Skip to content

[SPARK-34014][SQL] Ignore Distinct if it is the right child of a left semi or anti join#31045

Closed
tanelk wants to merge 3 commits into
apache:masterfrom
tanelk:SPARK-34014_ignore_distinct
Closed

[SPARK-34014][SQL] Ignore Distinct if it is the right child of a left semi or anti join#31045
tanelk wants to merge 3 commits into
apache:masterfrom
tanelk:SPARK-34014_ignore_distinct

Conversation

@tanelk

@tanelk tanelk commented Jan 5, 2021

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Remove Distinct from the right child of left semi and anti joins.

Why are the changes needed?

Left semi and anti joins ignore duplicates in the right child. Finding distinct values there will only add overhead.

This pattern is present in some TPCDS queries.

Does this PR introduce any user-facing change?

No

How was this patch tested?

UT

@SparkQA

SparkQA commented Jan 5, 2021

Copy link
Copy Markdown

Test build #133688 has finished for PR 31045 at commit 1708e56.

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

@tanelk

tanelk commented Jan 5, 2021

Copy link
Copy Markdown
Contributor Author

retest this please

@github-actions github-actions Bot added the SQL label Jan 5, 2021
@SparkQA

SparkQA commented Jan 5, 2021

Copy link
Copy Markdown

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38279/

@SparkQA

SparkQA commented Jan 5, 2021

Copy link
Copy Markdown

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38279/

* SELECT DISTINCT f1, f2 FROM t ==> SELECT f1, f2 FROM t GROUP BY f1, f2
* }}}
*
* The [[Distinct]] can be ignored if it is the right child of a left semi or anti join.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: could you move this comment between L1619/L1620?

// The [[Distinct]] can be ignored if it is the right child of a left semi or anti join.
case j @ Join(_, Distinct(right), LeftSemiOrAnti(_), _, _) => j.copy(right = right)

object ReplaceDistinctWithAggregate extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = plan transform {
def apply(plan: LogicalPlan): LogicalPlan = plan transformDown {
case j @ Join(_, Distinct(right), LeftSemiOrAnti(_), _, _) => j.copy(right = right)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just out of curiosity, this rule can always make performance better? For example, how about the case where a right table has too many duplicates?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I benchmarked a bit and you are right - this can cause a performance regression. For example q87 took a 10% hit on sf100. I think it's best to close this PR and issue.

@SparkQA

SparkQA commented Jan 6, 2021

Copy link
Copy Markdown

Test build #133690 has finished for PR 31045 at commit 1708e56.

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

@tanelk tanelk closed this Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants