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-18146] [SQL] Avoid using Union to chain together create table and repair partition commands #15665

Closed
wants to merge 5 commits into from

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Oct 27, 2016

What changes were proposed in this pull request?

The behavior of union is not well defined here. It is safer to explicitly execute these commands in order. The other use of Union in this way will be removed by #15633

How was this patch tested?

Existing tests.

cc @yhuai @cloud-fan

@SparkQA
Copy link

SparkQA commented Oct 28, 2016

Test build #67677 has finished for PR 15665 at commit 11fc8c4.

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

val createCmd = CreateTable(tableDesc, mode, Some(df.logicalPlan))
val cmd = if (tableDesc.partitionColumnNames.nonEmpty &&
val result = df.sparkSession.sessionState.executePlan(
CreateTable(tableDesc, mode, Some(df.logicalPlan)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the execution of this command triggered at here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you want to call result.toRdd at here to trigger the execution of a command? Then inside the loop to use toRdd again.

Copy link
Contributor

Choose a reason for hiding this comment

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

yea, we don't need the result variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@SparkQA
Copy link

SparkQA commented Oct 28, 2016

Test build #67689 has finished for PR 15665 at commit 310c5cb.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -254,7 +254,8 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) {
)
}

df.sparkSession.sessionState.executePlan(
df.sparkSession.sessionState.
Copy link
Contributor

Choose a reason for hiding this comment

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

what's going on here? a dot ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some github text editing gone wrong... fixed it

@SparkQA
Copy link

SparkQA commented Oct 28, 2016

Test build #67712 has finished for PR 15665 at commit dd01f3a.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 28, 2016

Test build #67714 has finished for PR 15665 at commit e8d28e1.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 28, 2016

Test build #67716 has finished for PR 15665 at commit da00002.

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

@rxin
Copy link
Contributor

rxin commented Oct 29, 2016

cc @hvanhovell @cloud-fan did you have anything to do with this in the past?

@ericl
Copy link
Contributor Author

ericl commented Oct 29, 2016

This was a discussed follow-up the pr on datasource partition management: #15515

@cloud-fan
Copy link
Contributor

LGTM, merging to master!

@asfgit asfgit closed this in 3ad99f1 Oct 30, 2016
robert3005 pushed a commit to palantir/spark that referenced this pull request Nov 1, 2016
…nd repair partition commands

## What changes were proposed in this pull request?

The behavior of union is not well defined here. It is safer to explicitly execute these commands in order. The other use of `Union` in this way will be removed by apache#15633

## How was this patch tested?

Existing tests.

cc yhuai cloud-fan

Author: Eric Liang <ekhliang@gmail.com>
Author: Eric Liang <ekl@databricks.com>

Closes apache#15665 from ericl/spark-18146.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…nd repair partition commands

## What changes were proposed in this pull request?

The behavior of union is not well defined here. It is safer to explicitly execute these commands in order. The other use of `Union` in this way will be removed by apache#15633

## How was this patch tested?

Existing tests.

cc yhuai cloud-fan

Author: Eric Liang <ekhliang@gmail.com>
Author: Eric Liang <ekl@databricks.com>

Closes apache#15665 from ericl/spark-18146.
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.

5 participants