Fix more review comments

Author: Kontinuation

api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java

@@ -34,6 +34,7 @@
 import org.apache.iceberg.Schema;
 import org.apache.iceberg.Table;
 import org.apache.iceberg.expressions.Literals.BoundingBoxLiteral;
+import org.apache.iceberg.geospatial.BoundingBox;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.transforms.Transforms;
@@ -549,7 +550,8 @@ private static String sanitize(Literal<?> literal, long now, int today) {
     } else if (literal instanceof Literals.VariantLiteral) {
       return sanitizeVariant(((Literals.VariantLiteral) literal).value(), now, today);
     } else if (literal instanceof BoundingBoxLiteral) {
-      return "(boundingbox)";
+      BoundingBox bbox = BoundingBox.fromByteBuffer(((BoundingBoxLiteral) literal).value());
+      return sanitizeBoundingBox(bbox);
     } else {
       // for uuid, decimal, fixed and binary, match the string result
       return sanitizeSimpleString(literal.value().toString());
@@ -628,6 +630,20 @@ private static String sanitizeSimpleString(CharSequence value) {
     return String.format(Locale.ROOT, "(hash-%08x)", HASH_FUNC.apply(value));
   }
 
+  private static String sanitizeBoundingBox(BoundingBox bbox) {
+    boolean hasZ = bbox.min().hasZ() && bbox.max().hasZ();
+    boolean hasM = bbox.min().hasM() && bbox.max().hasM();
+    if (hasZ && hasM) {
+      return "(boundingbox-xyzm)";
+    } else if (hasZ) {
+      return "(boundingbox-xyz)";
+    } else if (hasM) {
+      return "(boundingbox-xym)";
+    } else {
+      return "(boundingbox-xy)";
+    }
+  }
+
   private static String sanitizeVariant(Variant value, long now, int today) {
     return sanitizeVariant(value.value(), now, today);
   }

api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java

@@ -19,7 +19,6 @@
 package org.apache.iceberg.expressions;
 
 import java.util.List;
-import java.util.Locale;
 import java.util.Set;
 import org.apache.iceberg.exceptions.ValidationException;
 import org.apache.iceberg.relocated.com.google.common.base.Joiner;
@@ -286,11 +285,9 @@ public String toString() {
       case NOT_STARTS_WITH:
         return term() + " notStartsWith \"" + literal() + "\"";
       case ST_INTERSECTS:
+        return "st_intersects(" + term() + ", " + literal() + ")";
       case ST_DISJOINT:
-        {
-          String functionName = op().name().toLowerCase(Locale.ROOT);
-          return functionName + "(" + term() + ", " + literal() + ")";
-        }
+        return "st_disjoint(" + term() + ", " + literal() + ")";
       case IN:
         return term() + " in (" + COMMA.join(literals()) + ")";
       case NOT_IN:

api/src/test/java/org/apache/iceberg/expressions/TestExpressionUtil.java

@@ -1328,25 +1328,59 @@ private static Stream<Arguments> geospatialPredicateParameters() {
     GeospatialBound max = GeospatialBound.createXY(3.0, 4.0);
     BoundingBox bbox = new BoundingBox(min, max);
 
+    GeospatialBound minXYZ = GeospatialBound.createXYZ(1.0, 2.0, 3.0);
+    GeospatialBound maxXYZ = GeospatialBound.createXYZ(3.0, 4.0, 5.0);
+    BoundingBox bboxXYZ = new BoundingBox(minXYZ, maxXYZ);
+
+    GeospatialBound minXYM = GeospatialBound.createXYM(1.0, 2.0, 3.0);
+    GeospatialBound maxXYM = GeospatialBound.createXYM(3.0, 4.0, 5.0);
+    BoundingBox bboxXYM = new BoundingBox(minXYM, maxXYM);
+
+    GeospatialBound minXYZM = GeospatialBound.createXYZM(1.0, 2.0, 3.0, 4.0);
+    GeospatialBound maxXYZM = GeospatialBound.createXYZM(3.0, 4.0, 5.0, 6.0);
+    BoundingBox bboxXYZM = new BoundingBox(minXYZM, maxXYZM);
+
     return Stream.of(
         Arguments.of(
-            Expressions.stIntersects("geometry", bbox), "st_intersects(geometry, (boundingbox))"),
+            Expressions.stIntersects("geometry", bbox),
+            "st_intersects(geometry, (boundingbox-xy))",
+            "(boundingbox-xy)"),
+        Arguments.of(
+            Expressions.stIntersects("geography", bbox),
+            "st_intersects(geography, (boundingbox-xy))",
+            "(boundingbox-xy)"),
+        Arguments.of(
+            Expressions.stDisjoint("geometry", bbox),
+            "st_disjoint(geometry, (boundingbox-xy))",
+            "(boundingbox-xy)"),
+        Arguments.of(
+            Expressions.stDisjoint("geography", bbox),
+            "st_disjoint(geography, (boundingbox-xy))",
+            "(boundingbox-xy)"),
         Arguments.of(
-            Expressions.stIntersects("geography", bbox), "st_intersects(geography, (boundingbox))"),
+            Expressions.stIntersects("geometry", bboxXYZ),
+            "st_intersects(geometry, (boundingbox-xyz))",
+            "(boundingbox-xyz)"),
         Arguments.of(
-            Expressions.stDisjoint("geometry", bbox), "st_disjoint(geometry, (boundingbox))"),
+            Expressions.stIntersects("geometry", bboxXYM),
+            "st_intersects(geometry, (boundingbox-xym))",
+            "(boundingbox-xym)"),
         Arguments.of(
-            Expressions.stDisjoint("geography", bbox), "st_disjoint(geography, (boundingbox))"));
+            Expressions.stIntersects("geometry", bboxXYZM),
+            "st_intersects(geometry, (boundingbox-xyzm))",
+            "(boundingbox-xyzm)"));
   }
 
   @ParameterizedTest
   @MethodSource("geospatialPredicateParameters")
   public void testSanitizeGeospatialPredicates(
-      UnboundPredicate<ByteBuffer> geoPredicate, String expectedSanitizedString) {
+      UnboundPredicate<ByteBuffer> geoPredicate,
+      String expectedSanitizedString,
+      String expectedLiteral) {
     Expression.Operation operation = geoPredicate.op();
     String columnName = geoPredicate.term().ref().name();
 
-    Expression predicateSanitized = Expressions.predicate(operation, columnName, "(boundingbox)");
+    Expression predicateSanitized = Expressions.predicate(operation, columnName, expectedLiteral);
     assertEquals(predicateSanitized, ExpressionUtil.sanitize(geoPredicate));
     assertEquals(predicateSanitized, ExpressionUtil.sanitize(STRUCT, geoPredicate, true));