Revert expression parser changes and defer them to apache#14856

Author: Kontinuation

core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java

@@ -31,9 +31,6 @@
 import java.util.function.Supplier;
 import org.apache.iceberg.Schema;
 import org.apache.iceberg.SingleValueParser;
-import org.apache.iceberg.expressions.Expression.Operation;
-import org.apache.iceberg.geospatial.BoundingBox;
-import org.apache.iceberg.geospatial.GeospatialBound;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
 import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
@@ -162,13 +159,8 @@ public <T> Void predicate(BoundPredicate<T> pred) {
 
             if (pred.isLiteralPredicate()) {
               gen.writeFieldName(VALUE);
-              if (pred.op() == Operation.ST_INTERSECTS || pred.op() == Operation.ST_DISJOINT) {
-                ByteBuffer value = (ByteBuffer) pred.asLiteralPredicate().literal().value();
-                geospatialBoundingBox(BoundingBox.fromByteBuffer(value));
-              } else {
-                SingleValueParser.toJson(
-                    pred.term().type(), pred.asLiteralPredicate().literal().value(), gen);
-              }
+              SingleValueParser.toJson(
+                  pred.term().type(), pred.asLiteralPredicate().literal().value(), gen);
             } else if (pred.isSetPredicate()) {
               gen.writeArrayFieldStart(VALUES);
               for (T value : pred.asSetPredicate().literalSet()) {
@@ -200,11 +192,6 @@ public <T> Void predicate(UnboundPredicate<T> pred) {
                 }
                 gen.writeEndArray();
 
-              } else if (pred.op() == Expression.Operation.ST_INTERSECTS
-                  || pred.op() == Expression.Operation.ST_DISJOINT) {
-                gen.writeFieldName(VALUE);
-                ByteBuffer value = (ByteBuffer) pred.literal().value();
-                geospatialBoundingBox(BoundingBox.fromByteBuffer(value));
               } else {
                 gen.writeFieldName(VALUE);
                 unboundLiteral(pred.literal().value());
@@ -242,44 +229,6 @@ private void unboundLiteral(Object object) throws IOException {
       }
     }
 
-    private void geospatialBoundingBox(BoundingBox value) throws IOException {
-      gen.writeStartObject();
-
-      // Write x coordinate
-      gen.writeFieldName("x");
-      gen.writeStartObject();
-      gen.writeNumberField("min", value.min().x());
-      gen.writeNumberField("max", value.max().x());
-      gen.writeEndObject();
-
-      // Write y coordinate
-      gen.writeFieldName("y");
-      gen.writeStartObject();
-      gen.writeNumberField("min", value.min().y());
-      gen.writeNumberField("max", value.max().y());
-      gen.writeEndObject();
-
-      // Write z coordinate if present
-      if (value.min().hasZ() || value.max().hasZ()) {
-        gen.writeFieldName("z");
-        gen.writeStartObject();
-        gen.writeNumberField("min", value.min().z());
-        gen.writeNumberField("max", value.max().z());
-        gen.writeEndObject();
-      }
-
-      // Write m coordinate if present
-      if (value.min().hasM() || value.max().hasM()) {
-        gen.writeFieldName("m");
-        gen.writeStartObject();
-        gen.writeNumberField("min", value.min().m());
-        gen.writeNumberField("max", value.max().m());
-        gen.writeEndObject();
-      }
-
-      gen.writeEndObject();
-    }
-
     private String operationType(Expression.Operation op) {
       return op.toString().replaceAll("_", "-").toLowerCase(Locale.ENGLISH);
     }
@@ -357,9 +306,6 @@ static Expression fromJson(JsonNode json, Schema schema) {
         return Expressions.or(
             fromJson(JsonUtil.get(LEFT, json), schema),
             fromJson(JsonUtil.get(RIGHT, json), schema));
-      case ST_INTERSECTS:
-      case ST_DISJOINT:
-        return geospatialPredicateFromJson(op, json);
     }
 
     return predicateFromJson(op, json, schema);
@@ -428,22 +374,6 @@ private static <T> UnboundPredicate<T> predicateFromJson(
     }
   }
 
-  private static Expression geospatialPredicateFromJson(Expression.Operation op, JsonNode node) {
-    UnboundTerm<ByteBuffer> term = term(JsonUtil.get(TERM, node));
-    Preconditions.checkArgument(node.has(VALUE), "Cannot parse %s predicate: missing value", op);
-    Preconditions.checkArgument(
-        !node.has(VALUES), "Cannot parse %s predicate: has invalid values field", op);
-    BoundingBox boundingBox = geospatialBoundingBox(JsonUtil.get(VALUE, node));
-    switch (op) {
-      case ST_INTERSECTS:
-        return Expressions.stIntersects(term, boundingBox);
-      case ST_DISJOINT:
-        return Expressions.stDisjoint(term, boundingBox);
-      default:
-        throw new UnsupportedOperationException("Unsupported geospatial operation: " + op);
-    }
-  }
-
   private static <T> T literal(JsonNode valueNode, Function<JsonNode, T> toValue) {
     if (valueNode.isObject() && valueNode.has(TYPE)) {
       String type = JsonUtil.getString(TYPE, valueNode);
@@ -456,51 +386,6 @@ private static <T> T literal(JsonNode valueNode, Function<JsonNode, T> toValue)
     return toValue.apply(valueNode);
   }
 
-  private static BoundingBox geospatialBoundingBox(JsonNode valueNode) {
-    // X and Y coordinates are required
-    double xMin = valueNode.get("x").get("min").asDouble();
-    double xMax = valueNode.get("x").get("max").asDouble();
-    double yMin = valueNode.get("y").get("min").asDouble();
-    double yMax = valueNode.get("y").get("max").asDouble();
-
-    // Create GeospatialBound objects for min and max
-    GeospatialBound minBound;
-    GeospatialBound maxBound;
-
-    // Check if Z coordinate exists
-    boolean hasZ = valueNode.has("z");
-    // Check if M coordinate exists
-    boolean hasM = valueNode.has("m");
-
-    if (hasZ && hasM) {
-      // Both Z and M present
-      double zMin = valueNode.get("z").get("min").asDouble();
-      double zMax = valueNode.get("z").get("max").asDouble();
-      double mMin = valueNode.get("m").get("min").asDouble();
-      double mMax = valueNode.get("m").get("max").asDouble();
-      minBound = GeospatialBound.createXYZM(xMin, yMin, zMin, mMin);
-      maxBound = GeospatialBound.createXYZM(xMax, yMax, zMax, mMax);
-    } else if (hasZ) {
-      // Only Z present, no M
-      double zMin = valueNode.get("z").get("min").asDouble();
-      double zMax = valueNode.get("z").get("max").asDouble();
-      minBound = GeospatialBound.createXYZ(xMin, yMin, zMin);
-      maxBound = GeospatialBound.createXYZ(xMax, yMax, zMax);
-    } else if (hasM) {
-      // Only M present, no Z
-      double mMin = valueNode.get("m").get("min").asDouble();
-      double mMax = valueNode.get("m").get("max").asDouble();
-      minBound = GeospatialBound.createXYM(xMin, yMin, mMin);
-      maxBound = GeospatialBound.createXYM(xMax, yMax, mMax);
-    } else {
-      // Only X and Y present
-      minBound = GeospatialBound.createXY(xMin, yMin);
-      maxBound = GeospatialBound.createXY(xMax, yMax);
-    }
-
-    return new BoundingBox(minBound, maxBound);
-  }
-
   private static Object asObject(JsonNode node) {
     if (node.isIntegralNumber() && node.canConvertToLong()) {
       return node.asLong();

core/src/test/java/org/apache/iceberg/expressions/TestExpressionParser.java

@@ -28,8 +28,6 @@
 import java.nio.ByteBuffer;
 import java.util.UUID;
 import org.apache.iceberg.Schema;
-import org.apache.iceberg.geospatial.BoundingBox;
-import org.apache.iceberg.geospatial.GeospatialBound;
 import org.apache.iceberg.types.Types;
 import org.junit.jupiter.api.Test;
 
@@ -53,9 +51,7 @@ public class TestExpressionParser {
           required(114, "dec_9_0", Types.DecimalType.of(9, 0)),
           required(115, "dec_11_2", Types.DecimalType.of(11, 2)),
           required(116, "dec_38_10", Types.DecimalType.of(38, 10)), // maximum precision
-          required(117, "time", Types.TimeType.get()),
-          required(118, "geometry", Types.GeometryType.crs84()),
-          required(119, "geography", Types.GeographyType.crs84()));
+          required(117, "time", Types.TimeType.get()));
   private static final Schema SCHEMA = new Schema(SUPPORTED_PRIMITIVES.fields());
 
   @Test
@@ -98,22 +94,7 @@ public void testSimpleExpressions() {
           Expressions.or(
               Expressions.greaterThan(Expressions.day("ts"), "2022-08-14"),
               Expressions.equal("date", "2022-08-14")),
-          Expressions.not(Expressions.in("l", 1, 2, 3, 4)),
-          Expressions.stIntersects(
-              "geometry",
-              new BoundingBox(GeospatialBound.createXY(1, 2), GeospatialBound.createXY(3, 4))),
-          Expressions.stDisjoint(
-              "geometry",
-              new BoundingBox(
-                  GeospatialBound.createXYM(1, 2, 3), GeospatialBound.createXYM(3, 4, 5))),
-          Expressions.stIntersects(
-              "geography",
-              new BoundingBox(
-                  GeospatialBound.createXYZ(1, 2, 3), GeospatialBound.createXYZ(3, 4, 5))),
-          Expressions.stDisjoint(
-              "geography",
-              new BoundingBox(
-                  GeospatialBound.createXYZM(1, 2, 3, 4), GeospatialBound.createXYZM(3, 4, 5, 6)))
+          Expressions.not(Expressions.in("l", 1, 2, 3, 4))
         };
 
     for (Expression expr : expressions) {
@@ -563,122 +544,4 @@ public void testNegativeScaleDecimalLiteral() {
     assertThat(ExpressionParser.toJson(ExpressionParser.fromJson(expected), true))
         .isEqualTo(expected);
   }
-
-  @Test
-  public void testSpatialPredicate() {
-    String expected =
-        "{\n"
-            + "  \"type\" : \"st-intersects\",\n"
-            + "  \"term\" : \"column-name\",\n"
-            + "  \"value\" : {\n"
-            + "    \"x\" : {\n"
-            + "      \"min\" : 1.0,\n"
-            + "      \"max\" : 3.0\n"
-            + "    },\n"
-            + "    \"y\" : {\n"
-            + "      \"min\" : 2.0,\n"
-            + "      \"max\" : 4.0\n"
-            + "    }\n"
-            + "  }\n"
-            + "}";
-
-    Expression expression =
-        Expressions.stIntersects(
-            "column-name",
-            new BoundingBox(GeospatialBound.createXY(1, 2), GeospatialBound.createXY(3, 4)));
-    assertThat(ExpressionParser.toJson(expression, true)).isEqualTo(expected);
-    assertThat(ExpressionParser.toJson(ExpressionParser.fromJson(expected), true))
-        .isEqualTo(expected);
-
-    expected =
-        "{\n"
-            + "  \"type\" : \"st-intersects\",\n"
-            + "  \"term\" : \"column-name\",\n"
-            + "  \"value\" : {\n"
-            + "    \"x\" : {\n"
-            + "      \"min\" : 1.0,\n"
-            + "      \"max\" : 3.0\n"
-            + "    },\n"
-            + "    \"y\" : {\n"
-            + "      \"min\" : 2.0,\n"
-            + "      \"max\" : 4.0\n"
-            + "    },\n"
-            + "    \"m\" : {\n"
-            + "      \"min\" : 3.0,\n"
-            + "      \"max\" : 5.0\n"
-            + "    }\n"
-            + "  }\n"
-            + "}";
-
-    expression =
-        Expressions.stIntersects(
-            "column-name",
-            new BoundingBox(
-                GeospatialBound.createXYM(1, 2, 3), GeospatialBound.createXYM(3, 4, 5)));
-    assertThat(ExpressionParser.toJson(expression, true)).isEqualTo(expected);
-    assertThat(ExpressionParser.toJson(ExpressionParser.fromJson(expected), true))
-        .isEqualTo(expected);
-
-    expected =
-        "{\n"
-            + "  \"type\" : \"st-intersects\",\n"
-            + "  \"term\" : \"column-name\",\n"
-            + "  \"value\" : {\n"
-            + "    \"x\" : {\n"
-            + "      \"min\" : 1.0,\n"
-            + "      \"max\" : 3.0\n"
-            + "    },\n"
-            + "    \"y\" : {\n"
-            + "      \"min\" : 2.0,\n"
-            + "      \"max\" : 4.0\n"
-            + "    },\n"
-            + "    \"z\" : {\n"
-            + "      \"min\" : 3.0,\n"
-            + "      \"max\" : 5.0\n"
-            + "    }\n"
-            + "  }\n"
-            + "}";
-
-    expression =
-        Expressions.stIntersects(
-            "column-name",
-            new BoundingBox(
-                GeospatialBound.createXYZ(1, 2, 3), GeospatialBound.createXYZ(3, 4, 5)));
-    assertThat(ExpressionParser.toJson(expression, true)).isEqualTo(expected);
-    assertThat(ExpressionParser.toJson(ExpressionParser.fromJson(expected), true))
-        .isEqualTo(expected);
-
-    expected =
-        "{\n"
-            + "  \"type\" : \"st-intersects\",\n"
-            + "  \"term\" : \"column-name\",\n"
-            + "  \"value\" : {\n"
-            + "    \"x\" : {\n"
-            + "      \"min\" : 1.0,\n"
-            + "      \"max\" : 3.0\n"
-            + "    },\n"
-            + "    \"y\" : {\n"
-            + "      \"min\" : 2.0,\n"
-            + "      \"max\" : 4.0\n"
-            + "    },\n"
-            + "    \"z\" : {\n"
-            + "      \"min\" : 3.0,\n"
-            + "      \"max\" : 5.0\n"
-            + "    },\n"
-            + "    \"m\" : {\n"
-            + "      \"min\" : 4.0,\n"
-            + "      \"max\" : 6.0\n"
-            + "    }\n"
-            + "  }\n"
-            + "}";
-
-    expression =
-        Expressions.stIntersects(
-            "column-name",
-            new BoundingBox(
-                GeospatialBound.createXYZM(1, 2, 3, 4), GeospatialBound.createXYZM(3, 4, 5, 6)));
-    assertThat(ExpressionParser.toJson(expression, true)).isEqualTo(expected);
-    assertThat(ExpressionParser.toJson(ExpressionParser.fromJson(expected), true))
-        .isEqualTo(expected);
-  }
 }