Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-16663][SQL] desc table should be consistent between data source and hive serde tables #14302

Closed
wants to merge 3 commits into from

Conversation

cloud-fan
Copy link
Contributor

@cloud-fan cloud-fan commented Jul 21, 2016

What changes were proposed in this pull request?

Currently there are 2 inconsistence:

  1. for data source table, we only print partition names, for hive table, we also print partition schema. After this PR, we will always print schema
  2. if column doesn't have comment, data source table will print empty string, hive table will print null. After this PR, we will always print null

How was this patch tested?

new test in HiveDDLSuite

@cloud-fan
Copy link
Contributor Author

cc @yhuai @liancheng

@SparkQA
Copy link

SparkQA commented Jul 21, 2016

Test build #62679 has finished for PR 14302 at commit 090b109.

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

@SparkQA
Copy link

SparkQA commented Jul 22, 2016

Test build #62699 has finished for PR 14302 at commit 6726676.

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

@SparkQA
Copy link

SparkQA commented Jul 22, 2016

Test build #62709 has finished for PR 14302 at commit 1ffa49d.

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

append(buffer, s"# ${output.head.name}", "", "")
partCols.foreach(col => append(buffer, col, "", ""))
append(buffer, s"# ${output.head.name}", output(1).name, output(2).name)
val partCols = partColNames.map(n => schema.get.find(_.name == n).get)
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about this?

val s = schema.get
s.fieldNames.intersect(partColNames).map(s.apply)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rewrite the whole if as following:

for (s <- schema if partColNames.nonEmpty) {
  append(buffer, "# Partition Information", "", "")
  append(buffer, s"# ${output.head.name}", output(1).name, output(2).name)
  describeSchema(StructType(partColNames.map(s(_))))
}

@SparkQA
Copy link

SparkQA commented Jul 26, 2016

Test build #62859 has finished for PR 14302 at commit 5a5ddba.

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

@SparkQA
Copy link

SparkQA commented Jul 26, 2016

Test build #62865 has finished for PR 14302 at commit 56338b4.

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

@liancheng
Copy link
Contributor

LGTM, merging to master.

@asfgit asfgit closed this in a2abb58 Jul 26, 2016
@gatorsmile
Copy link
Member

@cloud-fan To backport #14531, we need to backport this. Do you want me to do it?

asfgit pushed a commit that referenced this pull request Sep 3, 2016
…e and hive serde tables

Currently there are 2 inconsistence:

1. for data source table, we only print partition names, for hive table, we also print partition schema. After this PR, we will always print schema
2. if column doesn't have comment, data source table will print empty string, hive table will print null. After this PR, we will always print null

new test in `HiveDDLSuite`

Author: Wenchen Fan <wenchen@databricks.com>

Closes #14302 from cloud-fan/minor3.

(cherry picked from commit a2abb58)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan
Copy link
Contributor Author

backported to 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants