Skip to content

Conversation

@AngersZhuuuu
Copy link
Contributor

@AngersZhuuuu AngersZhuuuu commented Mar 2, 2021

What changes were proposed in this pull request?

In the PR, I propose to generate "stable" output attributes per the logical node of the DESCRIBE COLUMN command.

Why are the changes needed?

This fixes the issue demonstrated by the example:

val tbl = "testcat.ns1.ns2.tbl"
sql(s"CREATE TABLE $tbl (c0 INT) USING _")
val description = sql(s"DESCRIBE TABLE $tbl c0")
description.drop("info_name")
[info]   org.apache.spark.sql.AnalysisException: Resolved attribute(s) info_name#74 missing from info_name#25,info_value#26 in operator !Project [info_name#74]. Attribute(s) with the same name appear in the operation: info_name. Please check if the right attribute(s) are used.;
[info] !Project [info_name#74]
[info] +- LocalRelation [info_name#25, info_value#26]

Does this PR introduce any user-facing change?

After this change user drop()/add() works well.

How was this patch tested?

Added UT

@github-actions github-actions bot added the SQL label Mar 2, 2021
@SparkQA
Copy link

SparkQA commented Mar 2, 2021

Test build #135622 has started for PR 31696 at commit 1b03c95.

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

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

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

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

@AngersZhuuuu
Copy link
Contributor Author

FYI @cloud-fan @HyukjinKwon

ResolvedViewIdentifier(ident), column: UnresolvedAttribute, isExtended, _) =>
// For views, the column will not be resolved by `ResolveReferences` because
// `ResolvedView` stores only the identifier.
DescribeColumnCommand(ident.asTableIdentifier, column.nameParts, isExtended)
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we pass the output to the v1 command as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

shall we pass the output to the v1 command as well?

You mean unify V2 and v1 DESC command's output? I think v2's DESC TABLE need to do this too.

Copy link
Contributor

Choose a reason for hiding this comment

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

yea, you can fix it here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, you can fix it here as well.

Done

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

Test build #135632 has finished for PR 31696 at commit 2dc21a0.

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

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

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

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

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

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

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

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

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

@AngersZhuuuu
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Mar 2, 2021

Test build #135660 has finished for PR 31696 at commit 50b9e0c.

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

@cloud-fan
Copy link
Contributor

cloud-fan commented Mar 3, 2021

thanks, merging to master!

@cloud-fan cloud-fan closed this in 17f0e70 Mar 3, 2021
@cloud-fan
Copy link
Contributor

@AngersZhuuuu can you open a backport PR for 3.1?

@AngersZhuuuu
Copy link
Contributor Author

@AngersZhuuuu can you open a backport PR for 3.1?

sure

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

Can this PR be ported to 3.0?

@cloud-fan
Copy link
Contributor

#31676 doesn't go to 3.0 either. Shall we backport both?

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.

4 participants