rdblue commented on a change in pull request #29339:
URL: https://github.com/apache/spark/pull/29339#discussion_r464541802



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Implicits.scala
##########
@@ -62,4 +74,31 @@ object DataSourceV2Implicits {
       new CaseInsensitiveStringMap(options.asJava)
     }
   }
+
+  def convertPartitionIndentifers(
+      partSpec: TablePartitionSpec,
+      partSchema: StructType): InternalRow = {
+    val partValues = partSchema.map { part =>
+      part.dataType match {
+        case _: ByteType =>
+          partSpec.getOrElse(part.name, "0").toByte

Review comment:
       Conversion to `InternalRow` should not modify the partition values by 
filling in defaults. Filling in a default like this is a correctness bug.
   
   I think this should require that all partition names are present in the map, 
and pass null if a name is present but does not have a value. If the partition 
doesn't allow null partition values, then it should throw an exception.




----------------------------------------------------------------
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.

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



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

Reply via email to