[SPARK-32743][SQL] Add distinct info at UnresolvedFunction toString #29586

ulysses-you · 2020-08-31T00:06:34Z

What changes were proposed in this pull request?

Add distinct info at UnresolvedFunction.toString.

Why are the changes needed?

Make UnresolvedFunction info complete.

create table test (c1 int, c2 int);
explain extended select sum(distinct c1) from test;

-- before this pr
== Parsed Logical Plan ==
'Project [unresolvedalias('sum('c1), None)]
+- 'UnresolvedRelation [test]

-- after this pr
== Parsed Logical Plan ==
'Project [unresolvedalias('sum(distinct 'c1), None)]
+- 'UnresolvedRelation [test]

Does this PR introduce any user-facing change?

Yes, get distinct info during sql parse.

How was this patch tested?

manual test.

ulysses-you · 2020-08-31T00:08:10Z

@maropu sorry for the late pr.

maropu · 2020-08-31T00:41:16Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala

-  override def toString: String = s"'$name(${children.mkString(", ")})"
+  override def toString: String = {
+    val distinct = if (isDistinct) "distinct " else ""
+    s"'$name($distinct${children.mkString(", ")})"


We need to handle filter, too?

filter as a child already exists.

Ah, I see. But, its better to reformat the output, 'Project [unresolvedalias('sum('c1), None)] into 'Project [unresolvedalias('sum('c1))] because None looks ambigous for users.

The none is introduced by unresolvedalias, shall we reformat it in this pr ?

Seems we can check none in Expression.flatArguments to avoid these case.

protected def flatArguments: Iterator[Any] = stringArgs.flatMap { case t: Iterable[_] => t case single => single :: Nil case None => Nil }

Oh, ok. ~~Yea, can you?~~ NVM, on second thought, the current one looks okay.

Does this like ok ? please tell me if need to do anything else.

Could you add tests somewhere?

How about add test in explain.sql ? like this

-- distinct func EXPLAIN EXTENDED SELECT sum(distinct val) FROM explain_temp1

SparkQA · 2020-08-31T05:19:04Z

Test build #128056 has finished for PR 29586 at commit d4e578c.

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

SparkQA · 2020-09-08T02:47:12Z

Test build #128373 has finished for PR 29586 at commit 745de51.

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

SparkQA · 2020-09-08T07:05:01Z

Test build #128379 has finished for PR 29586 at commit 7f112c9.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-08T10:12:15Z

Test build #128392 has finished for PR 29586 at commit a784a9b.

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

SparkQA · 2020-09-08T11:59:55Z

Test build #128402 has finished for PR 29586 at commit 0eeafdc.

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

SparkQA · 2020-09-08T16:19:27Z

Test build #128411 has finished for PR 29586 at commit b8c438a.

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

…2743

SparkQA · 2020-09-16T01:55:49Z

Test build #128726 has finished for PR 29586 at commit 458d744.

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

SparkQA · 2020-09-16T03:09:41Z

Test build #128733 has finished for PR 29586 at commit c340253.

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

SparkQA · 2020-09-16T07:05:02Z

Test build #128741 has finished for PR 29586 at commit 95a3e02.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-09-30T23:19:16Z

retest this please

maropu · 2020-09-30T23:19:25Z

cc: @HyukjinKwon @viirya

SparkQA · 2020-10-01T00:08:59Z

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

SparkQA · 2020-10-01T00:26:53Z

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

maropu · 2020-10-01T01:17:53Z

@ulysses-you could you fix the test failures?

HyukjinKwon · 2020-10-02T05:06:35Z

retest this please

SparkQA · 2020-10-02T07:05:02Z

Test build #129337 has finished for PR 29586 at commit 95a3e02.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-10-02T07:59:00Z

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

SparkQA · 2020-10-02T08:23:29Z

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

ulysses-you · 2020-10-06T13:16:50Z

thanks @maropu, can you trigger an another test ? I'm not sure if it is a related test failure.

maropu · 2020-10-06T14:49:16Z

thanks @maropu, can you trigger an another test ? I'm not sure if it is a related test failure.

You cannot do so?

maropu · 2020-10-06T14:49:24Z

retest this please

SparkQA · 2020-10-06T15:56:44Z

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

SparkQA · 2020-10-06T16:14:10Z

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

SparkQA · 2020-10-06T16:30:17Z

Test build #129453 has finished for PR 29586 at commit 95a3e02.

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

maropu · 2020-10-06T23:23:55Z

The test failure looks valid.

…2743

ulysses-you · 2020-10-08T09:51:55Z

thanks @maropu , this failure is introduced after #29756. Updated the test.

SparkQA · 2020-10-08T10:36:14Z

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

SparkQA · 2020-10-08T10:54:04Z

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

SparkQA · 2020-10-08T12:36:25Z

Test build #129555 has finished for PR 29586 at commit 4e10394.

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

SparkQA · 2020-10-08T14:13:40Z

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

SparkQA · 2020-10-08T14:32:56Z

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

SparkQA · 2020-10-08T17:49:29Z

Test build #129556 has finished for PR 29586 at commit 75d0b58.

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

maropu · 2020-10-09T00:25:48Z

Thanks! Merged to master.

ulysses-you · 2020-10-09T00:31:10Z

thanks for merging !

### What changes were proposed in this pull request? Add distinct info at `UnresolvedFunction.toString`. ### Why are the changes needed? Make `UnresolvedFunction` info complete. ``` create table test (c1 int, c2 int); explain extended select sum(distinct c1) from test; -- before this pr == Parsed Logical Plan == 'Project [unresolvedalias('sum('c1), None)] +- 'UnresolvedRelation [test] -- after this pr == Parsed Logical Plan == 'Project [unresolvedalias('sum(distinct 'c1), None)] +- 'UnresolvedRelation [test] ``` ### Does this PR introduce _any_ user-facing change? Yes, get distinct info during sql parse. ### How was this patch tested? manual test. Closes apache#29586 from ulysses-you/SPARK-32743. Authored-by: ulysses <[email protected]> Signed-off-by: Takeshi Yamamuro <[email protected]> (cherry picked from commit a907729) # Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala # sql/core/src/test/resources/sql-tests/results/explain-aqe.sql.out # sql/core/src/test/resources/sql-tests/results/explain.sql.out

init

d4e578c

probot-autolabeler bot added the SQL label Aug 31, 2020

maropu reviewed Aug 31, 2020

View reviewed changes

add test

745de51

fix

7f112c9

miss ;

a784a9b

fix

0eeafdc

fi

b8c438a

fix location

befcc9e

ulysses-you mentioned this pull request Sep 13, 2020

[SPARK-32827][SQL] Add spark.sql.maxMetadataStringLength config #29688

Closed

ulysses-you added 2 commits September 16, 2020 07:59

Merge branch 'master' of https://github.com/apache/spark into SPARK-3…

3021d15

…2743

add config

458d744

remove :

c340253

ulysses-you added 2 commits September 16, 2020 13:06

fix UnresolvedRelation

197f41f

fix explain in aqe

95a3e02

maropu approved these changes Sep 16, 2020

View reviewed changes

viirya approved these changes Oct 1, 2020

View reviewed changes

HyukjinKwon approved these changes Oct 2, 2020

View reviewed changes

ulysses-you added 2 commits October 8, 2020 17:44

Merge branch 'master' of https://github.com/apache/spark into SPARK-3…

7e11593

…2743

fix

4e10394

fix

75d0b58

maropu closed this in a907729 Oct 9, 2020

ulysses-you deleted the SPARK-32743 branch October 18, 2023 09:13

[SPARK-32743][SQL] Add distinct info at UnresolvedFunction toString #29586

[SPARK-32743][SQL] Add distinct info at UnresolvedFunction toString #29586

Uh oh!

Conversation

ulysses-you commented Aug 31, 2020

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?

Uh oh!

ulysses-you commented Aug 31, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 31, 2020

Uh oh!

SparkQA commented Sep 8, 2020

Uh oh!

SparkQA commented Sep 8, 2020

Uh oh!

SparkQA commented Sep 8, 2020

Uh oh!

SparkQA commented Sep 8, 2020

Uh oh!

SparkQA commented Sep 8, 2020

Uh oh!

SparkQA commented Sep 16, 2020

Uh oh!

SparkQA commented Sep 16, 2020

Uh oh!

SparkQA commented Sep 16, 2020

Uh oh!

maropu commented Sep 30, 2020

Uh oh!

maropu commented Sep 30, 2020

Uh oh!

SparkQA commented Oct 1, 2020

Uh oh!

SparkQA commented Oct 1, 2020

Uh oh!

maropu commented Oct 1, 2020

Uh oh!

HyukjinKwon commented Oct 2, 2020

Uh oh!

SparkQA commented Oct 2, 2020

Uh oh!

SparkQA commented Oct 2, 2020

Uh oh!

SparkQA commented Oct 2, 2020

Uh oh!

ulysses-you commented Oct 6, 2020

Uh oh!

maropu commented Oct 6, 2020

Uh oh!

maropu commented Oct 6, 2020

Uh oh!

SparkQA commented Oct 6, 2020

Uh oh!

maropu Aug 31, 2020 •

edited

Loading

maropu Aug 31, 2020 •

edited

Loading