hudi-agent commented on code in PR #19505:
URL: https://github.com/apache/hudi/pull/19505#discussion_r3709790438


##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/catalyst/catalog/HoodieCatalogTable.scala:
##########
@@ -290,6 +287,42 @@ class HoodieCatalogTable(val spark: SparkSession, var 
table: CatalogTable) exten
     (finalSchema, tableConfigs)
   }
 
+  /**
+   * Resolves the partition fields to persist in the table config, preferring 
an explicitly configured
+   * partition path field over the columns of the PARTITIONED BY clause.
+   */
+  private def resolvePartitionColumns(tableConfigs: Map[String, String]): 
String = {
+    if (SparkConfigUtils.containsConfigProperty(tableConfigs, 
KeyGeneratorOptions.PARTITIONPATH_FIELD_NAME)) {
+      SparkConfigUtils.getStringWithAltKeys(tableConfigs, 
KeyGeneratorOptions.PARTITIONPATH_FIELD_NAME)
+    } else if (table.partitionColumnNames.isEmpty) {
+      null
+    } else {
+      table.partitionColumnNames.mkString(",")
+    }
+  }
+
+  /**
+   * A non partitioned key generator never produces a partition path, so 
pairing one with partition
+   * columns describes a table that cannot exist. Creating it anyway persists 
a table config that
+   * disagrees with itself, and every subsequent write is rejected for a 
partition path conflict that
+   * names neither the key generator nor the partition columns. Reject it 
while the statement that
+   * introduced it is still in hand.
+   */
+  private def validateKeyGeneratorForPartitionColumns(tableConfigs: 
Map[String, String]): Unit = {
+    val partitionColumns = resolvePartitionColumns(tableConfigs)
+    if (!StringUtils.isNullOrEmpty(partitionColumns)) {
+      val keyGenerator = 
KeyGeneratorType.getKeyGeneratorClassName(tableConfigs.asJava)
+      if (KeyGeneratorType.NON_PARTITION.getClassName.equals(keyGenerator)

Review Comment:
   🤖 nit: could you use Scala's `==` here instead of `.equals()`? 
`KeyGeneratorType.NON_PARTITION.getClassName == keyGenerator || 
KeyGeneratorType.NON_PARTITION_AVRO.getClassName == keyGenerator` reads more 
naturally in Scala and is equivalent for `String`.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



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

Reply via email to