Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Feb 18, 2021

What changes were proposed in this pull request?

Rename the following v2 exec nodes:

  • AlterTableAddPartitionExec -> AddPartitionExec
  • AlterTableRenamePartitionExec -> RenamePartitionExec
  • AlterTableDropPartitionExec -> DropPartitionExec

Why are the changes needed?

  • To be consistent with v2 exec node added before: ALTER TABLE .. RENAME TO` -> RenameTableExec.
  • For simplicity and readability of the execution plans.

Does this PR introduce any user-facing change?

Should not since this is internal API.

How was this patch tested?

By running the existing test suites:

$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *AlterTableAddPartitionSuite"
$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *AlterTableDropPartitionSuite"
$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *AlterTableRenamePartitionSuite"

@SparkQA
Copy link

SparkQA commented Feb 18, 2021

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

@SparkQA
Copy link

SparkQA commented Feb 18, 2021

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

@SparkQA
Copy link

SparkQA commented Feb 18, 2021

Test build #135238 has finished for PR 31584 at commit dabc422.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Merged to master.

* Physical plan node for adding partitions of table.
*/
case class AlterTableAddPartitionExec(
case class AddPartitionExec(
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 rename the logical plan as well? The logical plan for REAME is RenameTable as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here is the PR #31596 @cloud-fan @imback82 @dongjoon-hyun Could you take a look at it.

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