deepakpanda93 commented on code in PR #19505:
URL: https://github.com/apache/hudi/pull/19505#discussion_r3869494125
##########
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:
Applied, thanks — good catch. Those two were the only `.equals(` calls in
the whole file, while its existing string comparisons (the `table.provider`
checks near the top) already use `==`, so they were the odd ones out.
Equivalent here as you say, and worth spelling out why it is safe to take:
`keyGenerator` genuinely can be null —
`KeyGeneratorType.getKeyGeneratorClassName` returns null when neither the class
name nor the type is configured, and the `HoodieConfig` overload is annotated
`@Nullable`. Both forms handle that identically, because the receiver is the
non-null constant: `CONSTANT.equals(null)` is `false` and `CONSTANT == null` is
`false`, so neither throws and the guard stays quiet. Pure style change, no
behaviour difference.
```scala
if (KeyGeneratorType.NON_PARTITION.getClassName == keyGenerator
|| KeyGeneratorType.NON_PARTITION_AVRO.getClassName == keyGenerator) {
```
Rebased onto latest master and re-ran `TestCreateTable` (spark3.5): 56/56
pass, including this PR's two cases — the one asserting the create is rejected,
and the negative control confirming the guard stays quiet when the table
genuinely has no partition columns.
--
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]
