Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20824#discussion_r174988745
--- Diff:
core/src/main/scala/org/apache/spark/internal/io/FileCommitProtocol.scala ---
@@ -145,15 +146,23 @@ object FileCommitProtocol {
jobId: String,
outputPath: String,
dynamicPartitionOverwrite: Boolean = false): FileCommitProtocol = {
+
+ logDebug(s"Creating committer $className; job $jobId;
output=$outputPath;" +
+ s" dynamic=$dynamicPartitionOverwrite")
val clazz =
Utils.classForName(className).asInstanceOf[Class[FileCommitProtocol]]
// First try the constructor with arguments (jobId: String,
outputPath: String,
// dynamicPartitionOverwrite: Boolean).
// If that doesn't exist, try the one with (jobId: string, outputPath:
String).
try {
val ctor = clazz.getDeclaredConstructor(classOf[String],
classOf[String], classOf[Boolean])
+ logDebug("Using (String, String, Boolean) constructor")
ctor.newInstance(jobId, outputPath,
dynamicPartitionOverwrite.asInstanceOf[java.lang.Boolean])
} catch {
case _: NoSuchMethodException =>
+ logDebug("Falling back to (String, String) constructor")
+ require(!dynamicPartitionOverwrite,
+ "Dynamic Partition Overwrite is enabled but" +
+ s" the committer ${className} does not have the appropriate
constructor")
--- End diff --
Hm .. actually we could do .. for example ..
```scala
abstract class FileCommitProtocol {
...
def dynamicPartitionOverwrite: Boolean = false
}
```
```scala
class HadoopMapReduceCommitProtocol(
jobId: String,
path: String,
override val dynamicPartitionOverwrite: Boolean = false)
```
(^ it's not double checked closely, for example, if the signature is safe
or not. Was just an idea)
and use `committer.dynamicPartitionOverwrite` in
`InsertIntoHadoopFsRelationCommand` to respect if the commit protocol supports
or not, if I understood all correctly, and then produce a warning saying
dynamic partition overwrite will be ignored.
_However_, sure. I think this case is kind of a made-up case and should be
a corner case I guess. I don't want to suggest an overkill (maybe) and I think
we don't have to make this complicated too much for now.
I am okay as is. Just wanted to make sure that we considered and checked
other possible stories.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
