[GitHub] [spark] steveloughran commented on a diff in pull request #37468: [SPARK-40034][SQL] PathOutputCommitters to support dynamic partitions

[GitBox <gi...@apache.org>]

steveloughran commented on code in PR #37468:
URL: https://github.com/apache/spark/pull/37468#discussion_r964636364


##########
hadoop-cloud/src/hadoop-3/main/scala/org/apache/spark/internal/io/cloud/PathOutputCommitProtocol.scala:
##########
@@ -114,10 +112,33 @@ class PathOutputCommitProtocol(
         // failures. Warn
         logTrace(s"Committer $committer may not be tolerant of task commit failures")
       }
+    } else {
+      // if required other committers need to be checked for dynamic partition
+      // compatibility through a StreamCapabilities probe.
+      if (dynamicPartitionOverwrite) {
+        if (supportsDynamicPartitions) {
+          logDebug(
+            s"Committer $committer has declared compatibility with dynamic partition overwrite")
+        } else {
+          throw new IOException(PathOutputCommitProtocol.UNSUPPORTED + ": " + committer)
+        }
+      }
     }
     committer
   }
 
+
+  /**
+   * Does the instantiated committer support dynamic partitions?
+   * @return true if the committer declares itself compatible.
+   */
+  private def supportsDynamicPartitions = {
+    committer.isInstanceOf[FileOutputCommitter] ||

Review Comment:
   up until now, if you used file output committer through the factory, it would reject the use of dynamic partitions. now it lets the caller without needing to implement the path capability.
   
   this does ensure that you can use that committer through the factory mechanism even on hadoop versions <= 3.3.4. I didn't add the path capabilities probe to the original FileOutputCommitter because I'm scared of going near that file: old, complex, critical code
   
   + will update the user facing details



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org