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

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

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


##########
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:
   I wonder how the tests are succeeding? bcoz, w/ latest master, delete partitions are lazy. only after cleaner gets a chance to clean up, the deleted partition may not show up when we make getAllPartitions(). Can you check the assertions in tests. Or are we asserting for records in the deleted partitions = 0 ? 



##########
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:
   yeah. not sure if he is confusing w/ hive style partitioning. Don't we need to consider both? i.e. url encode and hive style partitioning ?
   
   snippet from KeyGenUtils
   ```
       if (encodePartitionPath) {
         partitionPath = PartitionPathEncodeUtils.escapePathName(partitionPath);
       }
       if (hiveStylePartitioning) {
         partitionPath = partitionPathField + "=" + partitionPath;
       }
   ```
   



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