Re: [PR] HIVE-29637: Iceberg: Incorrect Results for NULL Predicate on Partitio… [hive]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #6515:
URL: https://github.com/apache/hive/pull/6515#discussion_r3331973976


##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/IcebergTableUtil.java:
##########
@@ -452,6 +452,25 @@ public static void performMetadataDelete(Table icebergTable, String branchName,
     deleteFiles.deleteFromRowFilter(exp).commit();
   }
 
+  /**
+   * Parses an Iceberg partition path into a Hive-compatible spec map.
+   * Unlike {@link Warehouse#makeSpecFromName}, this correctly represents null partition values
+   * as {@code null} instead of the literal string "null".
+   */

Review Comment:
   The new helper returns a spec map containing Java null values for null partitions. That works for predicate evaluation, but it is not generally “Hive-compatible”: `Warehouse.makePartNameUtil` throws `MetaException` when any spec value is null/empty. Please clarify this in the Javadoc to avoid future misuse (e.g., generating a partition name from the returned map).



##########
iceberg/iceberg-handler/src/test/queries/positive/iceberg_isnull_partition_pruning.q:
##########
@@ -0,0 +1,38 @@
+CREATE TABLE sample01 (
+  index INT,
+  date_col DATE,
+  timestamp_col TIMESTAMP,
+  str_col VARCHAR(24),
+  string_col STRING,
+  double_col DOUBLE,
+  float_col FLOAT,
+  decimal_col DECIMAL(9,3),
+  tinyint_col TINYINT,
+  smallint_col SMALLINT,
+  int_col INT,
+  bigint_col BIGINT,
+  boolean_col BOOLEAN
+);
+
+INSERT INTO sample01 VALUES
+(1003,"1969-10-27","1993-05-17 07:39:58.375409",NULL,"sloppy bronze hare",-181.01933598375618,-181.019336,-999999.999,-128,-32768,-2147483648,-9223372036854775808,false);

Review Comment:
   This new .q test uses double quotes for date/timestamp/string literals. Hive’s recommended and most commonly used style in qfiles is single quotes; using double quotes can become ambiguous if `hive.support.quoted.identifiers` is changed to `standard` (where double quotes are identifiers). Consider switching these literals to single quotes and regenerating the .q.out accordingly.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org