[SPARK-36280][SQL] Remove redundant aliases after RewritePredicateSubquery #33509

wangyum · 2021-07-24T05:02:32Z

What changes were proposed in this pull request?

Remove redundant aliases after RewritePredicateSubquery. For example:

sql("CREATE TABLE t1 USING parquet AS SELECT id AS a, id AS b, id AS c FROM range(10)")
sql("CREATE TABLE t2 USING parquet AS SELECT id AS x, id AS y FROM range(8)")
sql(
  """
    |SELECT *
    |FROM  t1
    |WHERE  a IN (SELECT x
    |  FROM  (SELECT x AS x,
    |           Rank() OVER (partition BY x ORDER BY Sum(y) DESC) AS ranking
    |    FROM   t2
    |    GROUP  BY x) tmp1
    |  WHERE  ranking <= 5)
    |""".stripMargin).explain

Before this PR:

== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- BroadcastHashJoin [a#10L], [x#7L], LeftSemi, BuildRight, false
   :- FileScan parquet default.t1[a#10L,b#11L,c#12L]
   +- BroadcastExchange HashedRelationBroadcastMode(List(input[0, bigint, true]),false), [id=#68]
      +- Project [x#7L]
         +- Filter (ranking#8 <= 5)
            +- Window [rank(_w2#25L) windowspecdefinition(x#15L, _w2#25L DESC NULLS LAST, specifiedwindowframe(RowFrame, unboundedpreceding$(), currentrow$())) AS ranking#8], [x#15L], [_w2#25L DESC NULLS LAST]
               +- Sort [x#15L ASC NULLS FIRST, _w2#25L DESC NULLS LAST], false, 0
                  +- Exchange hashpartitioning(x#15L, 5), ENSURE_REQUIREMENTS, [id=#62]
                     +- HashAggregate(keys=[x#15L], functions=[sum(y#16L)])
                        +- Exchange hashpartitioning(x#15L, 5), ENSURE_REQUIREMENTS, [id=#59]
                           +- HashAggregate(keys=[x#15L], functions=[partial_sum(y#16L)])
                              +- FileScan parquet default.t2[x#15L,y#16L]

After this PR:

== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- BroadcastHashJoin [a#10L], [x#15L], LeftSemi, BuildRight, false
   :- FileScan parquet default.t1[a#10L,b#11L,c#12L]
   +- BroadcastExchange HashedRelationBroadcastMode(List(input[0, bigint, true]),false), [id=#67]
      +- Project [x#15L]
         +- Filter (ranking#8 <= 5)
            +- Window [rank(_w2#25L) windowspecdefinition(x#15L, _w2#25L DESC NULLS LAST, specifiedwindowframe(RowFrame, unboundedpreceding$(), currentrow$())) AS ranking#8], [x#15L], [_w2#25L DESC NULLS LAST]
               +- Sort [x#15L ASC NULLS FIRST, _w2#25L DESC NULLS LAST], false, 0
                  +- HashAggregate(keys=[x#15L], functions=[sum(y#16L)])
                     +- Exchange hashpartitioning(x#15L, 5), ENSURE_REQUIREMENTS, [id=#59]
                        +- HashAggregate(keys=[x#15L], functions=[partial_sum(y#16L)])
                           +- FileScan parquet default.t2[x#15L,y#16L]

Why are the changes needed?

Reduce shuffle to improve query performance. This change can benefit TPC-DS q70.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Unit test.

SparkQA · 2021-07-24T05:54:46Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46113/

SparkQA · 2021-07-24T09:47:31Z

Test build #141596 has finished for PR 33509 at commit 8b41dd0.

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

sql/core/src/test/scala/org/apache/spark/sql/RewritePredicateSubqueryEndToEndSuite.scala

dongjoon-hyun

+1, LGTM. (with one minor comment).

cc @maropu

SparkQA · 2021-07-28T10:19:18Z

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

SparkQA · 2021-07-28T10:52:47Z

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

SparkQA · 2021-07-28T13:09:49Z

Test build #141764 has finished for PR 33509 at commit a631195.

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

allisonwang-db · 2021-07-28T20:24:40Z

sql/core/src/test/scala/org/apache/spark/sql/RewritePredicateSubqueryEndToEndSuite.scala

+import org.apache.spark.sql.execution.exchange.ShuffleExchangeExec
+import org.apache.spark.sql.test.SharedSparkSession
+
+class RewritePredicateSubqueryEndToEndSuite extends QueryTest with SharedSparkSession


Hi @wangyum I am curious why a separate "EndToEndSuite" is needed instead of adding this test to SubquerySuite?

This change focus on PredicateSubquery. If you mind, we can move to SubquerySuite.

It would be great to have all subquery related tests centralized in SubquerySuite :)

SparkQA · 2021-07-31T03:22:55Z

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

SparkQA · 2021-07-31T04:13:44Z

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

SparkQA · 2021-07-31T07:22:43Z

Test build #141919 has finished for PR 33509 at commit 51b0651.

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

dongjoon-hyun · 2021-08-02T09:02:22Z

sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala

+        |                                ORDER BY Sum(y) DESC) AS ranking
+        |                     FROM   t2
+        |                     GROUP  BY x) tmp1
+        |             WHERE  ranking <= 5)


Do we use https://www.dpriver.com/pp/sqlformat.htm style in our other test cases? Is there a reason why you choose this formatter, @wangyum ?

I have used this tool for many years. or do you have a recommended tool?

I don't think we have a standard about the SQL format in Spark right now. This format looks fine, except for the SELECT x ...many spaces... AS x

cloud-fan · 2021-08-02T16:00:42Z

sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala

@@ -1876,4 +1877,29 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
        "ReusedSubqueryExec should reuse an existing subquery")
    }
  }
+
+  test("SPARK-36280: Remove redundant aliases after RewritePredicateSubquery") {
+    sql("CREATE TABLE t1 USING parquet AS SELECT id AS a, id AS b, id AS c FROM range(10)")


nit: wrap with withTable

Fixed it. Sorry. I don't know why I forgot.

SparkQA · 2021-08-03T05:38:23Z

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

SparkQA · 2021-08-03T06:14:52Z

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

SparkQA · 2021-08-03T09:26:24Z

Test build #141980 has finished for PR 33509 at commit 3c89d19.

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

Remove redundant aliases after RewritePredicateSubquery

8b41dd0

github-actions bot added the SQL label Jul 24, 2021

wangyum requested review from cloud-fan and dongjoon-hyun July 26, 2021 04:02

cloud-fan approved these changes Jul 26, 2021

View reviewed changes

dongjoon-hyun reviewed Jul 27, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/RewritePredicateSubqueryEndToEndSuite.scala Outdated Show resolved Hide resolved

dongjoon-hyun approved these changes Jul 27, 2021

View reviewed changes

Format SQL using https://www.dpriver.com/pp/sqlformat.htm

a631195

allisonwang-db reviewed Jul 28, 2021

View reviewed changes

Move test

51b0651

dongjoon-hyun reviewed Aug 2, 2021

View reviewed changes

cloud-fan reviewed Aug 2, 2021

View reviewed changes

Fix test

3c89d19

dongjoon-hyun approved these changes Aug 3, 2021

View reviewed changes

dongjoon-hyun closed this in 4a6afb4 Aug 3, 2021

wangyum deleted the SPARK-36280 branch August 3, 2021 23:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-36280][SQL] Remove redundant aliases after RewritePredicateSubquery #33509

[SPARK-36280][SQL] Remove redundant aliases after RewritePredicateSubquery #33509

wangyum commented Jul 24, 2021

SparkQA commented Jul 24, 2021

SparkQA commented Jul 24, 2021

dongjoon-hyun left a comment

SparkQA commented Jul 28, 2021

SparkQA commented Jul 28, 2021

SparkQA commented Jul 28, 2021

allisonwang-db Jul 28, 2021

wangyum Jul 30, 2021

allisonwang-db Jul 30, 2021

wangyum Jul 31, 2021

SparkQA commented Jul 31, 2021

SparkQA commented Jul 31, 2021

SparkQA commented Jul 31, 2021

dongjoon-hyun Aug 2, 2021

wangyum Aug 2, 2021

cloud-fan Aug 2, 2021

cloud-fan Aug 2, 2021

wangyum Aug 3, 2021

SparkQA commented Aug 3, 2021

SparkQA commented Aug 3, 2021

SparkQA commented Aug 3, 2021

[SPARK-36280][SQL] Remove redundant aliases after RewritePredicateSubquery #33509

[SPARK-36280][SQL] Remove redundant aliases after RewritePredicateSubquery #33509

Conversation

wangyum commented Jul 24, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Jul 24, 2021

SparkQA commented Jul 24, 2021

dongjoon-hyun left a comment

Choose a reason for hiding this comment

SparkQA commented Jul 28, 2021

SparkQA commented Jul 28, 2021

SparkQA commented Jul 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Jul 31, 2021

SparkQA commented Jul 31, 2021

SparkQA commented Jul 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 3, 2021

SparkQA commented Aug 3, 2021

SparkQA commented Aug 3, 2021