Skip to content

Conversation

@koertkuipers
Copy link
Contributor

What changes were proposed in this pull request?

Besides spark setting spark.sql.sources.partitionOverwriteMode also allow setting partitionOverWriteMode per write

How was this patch tested?

Added unit test in InsertSuite

Please review http://spark.apache.org/contributing.html before opening a pull request.

@SparkQA
Copy link

SparkQA commented Jul 19, 2018

Test build #93290 has finished for PR 21818 at commit 7dd2eab.

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

@SparkQA
Copy link

SparkQA commented Jul 20, 2018

Test build #93295 has finished for PR 21818 at commit 7d0752b.

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

val enableDynamicOverwrite = parameters.get("partitionOverwriteMode")
.map(mode => PartitionOverwriteMode.withName(mode.toUpperCase))
.getOrElse(sparkSession.sessionState.conf.partitionOverwriteMode) ==
PartitionOverwriteMode.DYNAMIC
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is too long

val partitionOverwriteMode = parameters.get("partitionOverwriteMode")....
val enableDynamicOverwrite = partitionOverwriteMode == PartitionOverwriteMode.DYNAMIC

}
}
}
test("SPARK-24860: dynamic partition overwrite specified per source without catalog table") {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add a blank file before this new test case

}
test("SPARK-24860: dynamic partition overwrite specified per source without catalog table") {
withTempPath { path =>
Seq((1, 1, 1)).toDF("i", "part1", "part2")
Copy link
Contributor

Choose a reason for hiding this comment

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

can we simplify the test? ideally we only need one partition column, and write some initial data to the table. then do an overwrite with partitionOverwriteMode=dynamic, and another overwrite with partitionOverwriteMode=static.

@SparkQA
Copy link

SparkQA commented Jul 20, 2018

Test build #93349 has finished for PR 21818 at commit a4ebf9d.

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

@HyukjinKwon
Copy link
Member

retest this please

"mode to keep the same behavior of Spark prior to 2.3. Note that this config doesn't " +
"affect Hive serde tables, as they are always overwritten with dynamic mode.")
"affect Hive serde tables, as they are always overwritten with dynamic mode. This can " +
"also be set as an output option for a data source using key partitionOverwriteMode, " +
Copy link
Member

Choose a reason for hiding this comment

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

Also need to explain the precedence between the option and sqlconf.

@SparkQA
Copy link

SparkQA commented Jul 25, 2018

Test build #93530 has finished for PR 21818 at commit a4ebf9d.

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

@SparkQA
Copy link

SparkQA commented Jul 25, 2018

Test build #93550 has finished for PR 21818 at commit 03c0926.

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

@gatorsmile
Copy link
Member

LGTM

Thanks! Merged to master

@asfgit asfgit closed this in 17f469b Jul 25, 2018
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