[SPARK-20954][SQL] `DESCRIBE [EXTENDED]` result should be compatible with previous Spark #18203

dongjoon-hyun · 2017-06-05T06:53:18Z

What changes were proposed in this pull request?

After SPARK-20067, DESCRIBE and DESCRIBE EXTENDED shows the following result. This is incompatible with Spark 2.1.1. This PR removes the column header line in case of those command.

MASTER and BRANCH-2.2

scala> sql("desc t").show(false)
+----------+---------+-------+
|col_name  |data_type|comment|
+----------+---------+-------+
|# col_name|data_type|comment|
|a         |int      |null   |
+----------+---------+-------+

SPARK 2.1.1 and this PR

scala> sql("desc t").show(false)
+--------+---------+-------+
|col_name|data_type|comment|
+--------+---------+-------+
|a       |int      |null   |
+--------+---------+-------+

How was this patch tested?

Pass the Jenkins with the updated test suites.

… previous Spark and Hive.

SparkQA · 2017-06-05T06:57:34Z

Test build #77734 has started for PR 18203 at commit a0b8b1d.

dongjoon-hyun · 2017-06-05T07:12:02Z

Retest this please

SparkQA · 2017-06-05T08:27:59Z

Test build #77735 has finished for PR 18203 at commit a0b8b1d.

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

SparkQA · 2017-06-05T11:27:49Z

Test build #77737 has finished for PR 18203 at commit 02913a5.

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

dongjoon-hyun · 2017-06-05T15:49:12Z

Hi, @gatorsmile .
Could you review this PR?

dongjoon-hyun · 2017-06-05T22:48:57Z

Hi, @gatorsmile and @cloud-fan .
Could you review this PR when you have sometime?
This will recover the incompatible changes at Spark 2.2.0.

cloud-fan · 2017-06-07T05:52:55Z

LGTM, cc @gatorsmile for final sign-off

gatorsmile · 2017-06-07T06:23:29Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala

          s"DESC PARTITION is not allowed on a temporary view: ${table.identifier}")
      }
-      describeSchema(catalog.lookupRelation(table).schema, result)
+      describeSchema(catalog.lookupRelation(table).schema, result, isExtended)


When specifying Extend, we should also remove it too, right? @cloud-fan

Thank you Right. This PR considered only simple DESC. I'll update 'DESC EXTENDED', too. It is different from DESC FORMATTED as you pointed.

cloud-fan · 2017-06-08T18:08:11Z

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala

        partitionSpec,
-        ctx.EXTENDED != null || ctx.FORMATTED != null)
+        ctx.EXTENDED != null,
+        ctx.FORMATTED != null)


we deprecated DESC FORMATTED intentionally, and made DESC EXTENDED behave as DESC FORMATTED. I really don't think users will query the result of DESC EXTENDED before, as all the detail table information is inside one column, so it's probably fine to break this behavior just for display.

Oh, I see. Then, you want remove the header completely for all case. Did I understand correctly?

Or, do I simple revert the following two commits after your review?

For DESC t, we should change it back to be compatible with before, for DESC EXTENDED, it should be same as DESC FORMATTED, so we don't need to change it.

This reverts commit 744d166.

This reverts commit 48329a4.

SparkQA · 2017-06-08T18:18:22Z

Test build #77812 has finished for PR 18203 at commit 48329a4.

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

SparkQA · 2017-06-08T19:06:50Z

Test build #77813 has finished for PR 18203 at commit 744d166.

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

dongjoon-hyun · 2017-06-08T21:15:26Z

Oops. I'll ping again. Sorry. I didn't push it.

SparkQA · 2017-06-08T23:41:24Z

Test build #77819 has finished for PR 18203 at commit 1eb712d.

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

gatorsmile · 2017-06-08T23:47:03Z

LGTM

dongjoon-hyun · 2017-06-08T23:47:43Z

Thank you. Please wait for a run. The last style fix is not tested by Jenkins.

cloud-fan · 2017-06-08T23:48:00Z

thanks, merging to master! can you send a new PR for 2.2?

cloud-fan · 2017-06-08T23:48:27Z

oops, I though it has passed tests... Let's see the test result then

dongjoon-hyun · 2017-06-08T23:49:26Z

Sorry for that. I'll start to prepare a new PR for 2.2.

SparkQA · 2017-06-09T02:49:03Z

Test build #77827 has finished for PR 18203 at commit 5d27020.

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

dongjoon-hyun · 2017-06-09T02:58:27Z

It passed. Also, a PR for branch-2.2, #18245, passed, too.

dongjoon-hyun · 2017-06-09T04:24:22Z

Thank you for review and merge both PRs, @gatorsmile and @cloud-fan .

[SPARK-20954][SQL] Simple DESCRIBE result should be compatible with…

a0b8b1d

… previous Spark and Hive.

fix

02913a5

dongjoon-hyun mentioned this pull request Jun 5, 2017

[SPARK-20067] [SQL] Unify and Clean Up Desc Commands Using Catalog Interface #17394

Closed

gatorsmile reviewed Jun 7, 2017

View reviewed changes

Fix DESC EXTENDED, too.

48329a4

dongjoon-hyun changed the title ~~[SPARK-20954][SQL] Simple DESCRIBE result should be compatible with previous Spark~~ [SPARK-20954][SQL] DESCRIBE [EXTENDED] result should be compatible with previous Spark Jun 8, 2017

fix typo.

744d166

cloud-fan reviewed Jun 8, 2017

View reviewed changes

dongjoon-hyun added 2 commits June 8, 2017 11:17

Revert "fix typo."

b8c3b13

This reverts commit 744d166.

Revert "Fix DESC EXTENDED, too."

a589686

This reverts commit 48329a4.

dongjoon-hyun added 2 commits June 8, 2017 14:16

Address comments.

1eb712d

fix style.

5d27020

asfgit closed this in 6e95897 Jun 8, 2017

dongjoon-hyun deleted the SPARK-20954 branch June 10, 2017 16:07

gatorsmile mentioned this pull request Feb 9, 2018

[SPARK-23364][SQL]'desc table' command in spark-sql add column head display #20557

Closed

[SPARK-20954][SQL] DESCRIBE [EXTENDED] result should be compatible with previous Spark #18203

[SPARK-20954][SQL] DESCRIBE [EXTENDED] result should be compatible with previous Spark #18203

Uh oh!

Conversation

dongjoon-hyun commented Jun 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Jun 5, 2017

Uh oh!

dongjoon-hyun commented Jun 5, 2017

Uh oh!

SparkQA commented Jun 5, 2017

Uh oh!

SparkQA commented Jun 5, 2017

Uh oh!

dongjoon-hyun commented Jun 5, 2017

Uh oh!

dongjoon-hyun commented Jun 5, 2017

Uh oh!

cloud-fan commented Jun 7, 2017

Uh oh!

gatorsmile Jun 7, 2017

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 7, 2017

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 8, 2017

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 8, 2017

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 8, 2017

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 8, 2017

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 8, 2017

Uh oh!

SparkQA commented Jun 8, 2017

Uh oh!

dongjoon-hyun commented Jun 8, 2017

Uh oh!

SparkQA commented Jun 8, 2017

Uh oh!

gatorsmile commented Jun 8, 2017

Uh oh!

dongjoon-hyun commented Jun 8, 2017

Uh oh!

cloud-fan commented Jun 8, 2017

Uh oh!

cloud-fan commented Jun 8, 2017

Uh oh!

dongjoon-hyun commented Jun 8, 2017

Uh oh!

SparkQA commented Jun 9, 2017

Uh oh!

dongjoon-hyun commented Jun 9, 2017

Uh oh!

dongjoon-hyun commented Jun 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[SPARK-20954][SQL] `DESCRIBE [EXTENDED]` result should be compatible with previous Spark #18203

[SPARK-20954][SQL] `DESCRIBE [EXTENDED]` result should be compatible with previous Spark #18203

dongjoon-hyun commented Jun 5, 2017 •

edited

Loading