pan3793 commented on code in PR #7622:
URL: https://github.com/apache/kyuubi/pull/7622#discussion_r3765109872
##########
extensions/spark/kyuubi-spark-connector-hive/src/main/scala/org/apache/kyuubi/spark/connector/hive/write/HiveWrite.scala:
##########
@@ -66,8 +67,27 @@ case class HiveWrite(
private val tableLocation = hiveTable.getDataLocation
private val allColumns = info.schema().toAttributes
- private val dataColumns = allColumns.take(allColumns.length -
hiveTable.getPartCols.size())
- private val partColumns = allColumns.takeRight(hiveTable.getPartCols.size())
+ // Split data and partition columns by name rather than by position. Users
can create a
+ // partitioned table whose partition columns are not the trailing columns of
the schema
+ // (e.g. df.write.partitionBy("mid_col")), so a positional take/takeRight
would mis-classify
+ // columns and lead the metastore to reject the written partition spec.
Match names with the
+ // session resolver because the Hive metastore lower-cases partition column
names while the
+ // write schema preserves the user's original case. See KYUUBI #6403.
+ private val resolver = sparkSession.sessionState.conf.resolver
+ private def isPartitionColumn(name: String): Boolean =
+ table.partitionColumnNames.exists(resolver(_, name))
+ // partColumns keeps the query's original column case so requiredOrdering()
resolves against the
+ // query.
+ private val (partColumns, dataColumns) =
+ allColumns.partition(col => isPartitionColumn(col.name))
+
+ // SPARK-28054: the Hive metastore keeps partition columns with lower-cased
names, so the written
+ // partition directories must use lower-cased names; otherwise
loadDynamicPartitions rejects the
+ // spec with "contains non-partition columns" when the schema preserves the
user's original case.
+ // Rename via withName to keep the same exprId, so the value projection
against allColumns is
+ // unaffected. See KYUUBI #6403.
+ private val outputPartColumns: Seq[AttributeReference] =
Review Comment:
Correction on my P1: the mechanism holds, the impact does not.
On ext4, with the reconstruction reverted, the guard resolves
`.../FavoriteColor=blue` (`exists=false`) while the data lives in
`favoritecolor=blue`, so it never fires. But the stale file is still removed:
Hive `moveFile(..., replace=true)` deletes the destination directory before
moving (`Hive.java`, branch-2.3), including when the partition is missing from
the metastore. So this is a dead guard, not data retention - downgrading from
P1. The lower-casing is still the right alignment, keep it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
