[GitHub] [hudi] xushiyan commented on a diff in pull request #5272: [HUDI-3826] Commands deleting partitions do so incorrectly

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

xushiyan commented on code in PR #5272:
URL: https://github.com/apache/hudi/pull/5272#discussion_r848724898


##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/HoodieSqlCommonUtils.scala:
##########
@@ -372,4 +376,32 @@ object HoodieSqlCommonUtils extends SparkAdapterSupport {
     }.mkString(",")
     partitionsToDrop
   }
+
+  def getPartitionPathToTruncate(hoodieCatalogTable: HoodieCatalogTable,

Review Comment:
   can you add a javadoc to illustrate a sample return value? easier for other to use this method



##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/HoodieSqlCommonUtils.scala:
##########
@@ -372,4 +376,32 @@ object HoodieSqlCommonUtils extends SparkAdapterSupport {
     }.mkString(",")
     partitionsToDrop
   }
+
+  def getPartitionPathToTruncate(hoodieCatalogTable: HoodieCatalogTable,
+                                 table: CatalogTable,
+                                 partitionSpec: Option[TablePartitionSpec],
+                                 resolver: Resolver): String = {
+    val partCols = table.partitionColumnNames
+    val normalizedSpec: Seq[Map[String, String]] = Seq(partitionSpec.map { spec =>
+      normalizePartitionSpec(
+        spec,
+        partCols,
+        table.identifier.quotedString,
+        resolver)
+    }.get)
+
+    val allPartitionPaths = hoodieCatalogTable.getPartitionPaths
+    val enableEncodeUrl = isUrlEncodeEnabled(allPartitionPaths, table)
+
+    val partitionsToTruncate = normalizedSpec.map { spec =>
+      hoodieCatalogTable.partitionFields.map { partitionColumn =>
+        if (enableEncodeUrl) {
+          partitionColumn + "=" + "\"" + spec(partitionColumn) + "\""

Review Comment:
   this encode case did not call `PartitionPathEncodeUtils.escapePathName`?



##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/ProvidesHoodieConfig.scala:
##########
@@ -21,7 +21,7 @@ import org.apache.hudi.DataSourceWriteOptions._
 import org.apache.hudi.common.config.TypedProperties
 import org.apache.hudi.common.model.OverwriteWithLatestAvroPayload
 import org.apache.hudi.common.table.HoodieTableConfig
-import org.apache.hudi.config.HoodieWriteConfig.TBL_NAME
+import org.apache.hudi.config.HoodieWriteConfig.{AVRO_SCHEMA_VALIDATE_ENABLE, TBL_NAME}

Review Comment:
   this is unused import? can we avoid changing unrelated files?



##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/HoodieSqlCommonUtils.scala:
##########
@@ -372,4 +376,32 @@ object HoodieSqlCommonUtils extends SparkAdapterSupport {
     }.mkString(",")
     partitionsToDrop
   }
+
+  def getPartitionPathToTruncate(hoodieCatalogTable: HoodieCatalogTable,

Review Comment:
   similarly can do it for `getPartitionPathToDrop` above



##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/TestTruncateTable.scala:
##########
@@ -69,7 +69,7 @@ class TestTruncateTable extends TestHoodieSqlBase {
 
         df.write.format("hudi")
           .option(HoodieWriteConfig.TBL_NAME.key, tableName)
-          .option(TABLE_TYPE.key, MOR_TABLE_TYPE_OPT_VAL)
+          .option(TABLE_TYPE.key, COW_TABLE_TYPE_OPT_VAL)

Review Comment:
   why this test case change?



-- 
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: commits-unsubscribe@hudi.apache.org

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