stevenzwu commented on code in PR #14101:
URL: https://github.com/apache/iceberg/pull/14101#discussion_r2757377406
##########
api/src/test/java/org/apache/iceberg/expressions/TestExpressionBinding.java:
##########
@@ -509,4 +513,38 @@ public void testIsNullWithNestedStructs(List<Boolean>
requiredFields, Expression
TestHelpers.assertAllReferencesBound("NotNull", bound);
assertThat(bound.op()).isEqualTo(expression.negate().op());
}
+
+ @Test
+ public void testStIntersects() {
+ // Create a bounding box for testing
+ GeospatialBound min = GeospatialBound.createXY(1.0, 2.0);
+ GeospatialBound max = GeospatialBound.createXY(3.0, 4.0);
+ BoundingBox bbox = new BoundingBox(min, max);
+
+ Expression expr = Expressions.stIntersects("point", bbox);
Review Comment:
should we also test geography type for intersects?
##########
api/src/test/java/org/apache/iceberg/expressions/TestExpressionBinding.java:
##########
@@ -509,4 +513,38 @@ public void testIsNullWithNestedStructs(List<Boolean>
requiredFields, Expression
TestHelpers.assertAllReferencesBound("NotNull", bound);
assertThat(bound.op()).isEqualTo(expression.negate().op());
}
+
+ @Test
+ public void testStIntersects() {
+ // Create a bounding box for testing
+ GeospatialBound min = GeospatialBound.createXY(1.0, 2.0);
+ GeospatialBound max = GeospatialBound.createXY(3.0, 4.0);
+ BoundingBox bbox = new BoundingBox(min, max);
+
+ Expression expr = Expressions.stIntersects("point", bbox);
+ Expression bound = Binder.bind(STRUCT, expr);
+
+ TestHelpers.assertAllReferencesBound("ST_Intersects", bound);
+ BoundPredicate<?> pred = TestHelpers.assertAndUnwrap(bound);
+ assertThat(pred.op()).isEqualTo(Expression.Operation.ST_INTERSECTS);
+ assertThat(pred.term().ref().fieldId()).as("Should bind point
correctly").isEqualTo(7);
+
assertThat(pred.asLiteralPredicate().literal().value()).isEqualTo(bbox.toByteBuffer());
+ }
+
+ @Test
+ public void testStDisjoint() {
+ // Create a bounding box for testing
+ GeospatialBound min = GeospatialBound.createXY(1.0, 2.0);
+ GeospatialBound max = GeospatialBound.createXY(3.0, 4.0);
+ BoundingBox bbox = new BoundingBox(min, max);
+
+ Expression expr = Expressions.stDisjoint("geography", bbox);
Review Comment:
should we test geometry type for disjoint?
##########
api/src/test/java/org/apache/iceberg/expressions/TestExpressionBinding.java:
##########
@@ -62,7 +64,9 @@ public class TestExpressionBinding {
required(3, "data", Types.StringType.get()),
required(4, "var", Types.VariantType.get()),
optional(5, "nullable", Types.IntegerType.get()),
- optional(6, "always_null", Types.UnknownType.get()));
+ optional(6, "always_null", Types.UnknownType.get()),
+ required(7, "point", Types.GeometryType.crs84()),
Review Comment:
nit: name the field `geometry` as did for other test classes?
##########
api/src/test/java/org/apache/iceberg/expressions/TestExpressionUtil.java:
##########
@@ -1315,6 +1324,92 @@ private static VariantArray createArrayWithNestedTypes()
{
return (VariantArray) VariantValue.from(metadata, variantBB);
}
+ private static Stream<Arguments> geospatialPredicateParameters() {
+ GeospatialBound min = GeospatialBound.createXY(1.0, 2.0);
+ 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-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("geometry", bboxXYZ),
+ "st_intersects(geometry, (boundingbox-xyz))",
+ "(boundingbox-xyz)"),
+ Arguments.of(
+ Expressions.stIntersects("geometry", bboxXYM),
+ "st_intersects(geometry, (boundingbox-xym))",
+ "(boundingbox-xym)"),
+ Arguments.of(
+ Expressions.stIntersects("geometry", bboxXYZM),
+ "st_intersects(geometry, (boundingbox-xyzm))",
+ "(boundingbox-xyzm)"));
+ }
+
+ @ParameterizedTest
+ @MethodSource("geospatialPredicateParameters")
+ public void testSanitizeGeospatialPredicates(
+ UnboundPredicate<ByteBuffer> geoPredicate,
+ String expectedSanitizedString,
+ String expectedLiteral) {
+ Expression.Operation operation = geoPredicate.op();
+ String columnName = geoPredicate.term().ref().name();
+
+ Expression predicateSanitized = Expressions.predicate(operation,
columnName, expectedLiteral);
+ assertEquals(predicateSanitized, ExpressionUtil.sanitize(geoPredicate));
+ assertEquals(predicateSanitized, ExpressionUtil.sanitize(STRUCT,
geoPredicate, true));
+
+ assertThat(ExpressionUtil.toSanitizedString(geoPredicate))
+ .as("Sanitized string should be identical for geospatial predicates")
+ .isEqualTo(expectedSanitizedString);
+
+ assertThat(ExpressionUtil.toSanitizedString(STRUCT, geoPredicate, true))
+ .as("Sanitized string should be identical for geospatial predicates")
+ .isEqualTo(expectedSanitizedString);
+ }
+
+ @Test
+ public void testBoundingBoxLiteralNormalizesBuffer() {
+ GeospatialBound min = GeospatialBound.createXY(1.0, 2.0);
+ GeospatialBound max = GeospatialBound.createXY(3.0, 4.0);
+ BoundingBox box = new BoundingBox(min, max);
+ ByteBuffer serialized = box.toByteBuffer();
+
+ ByteBuffer padded = ByteBuffer.allocate(serialized.remaining() +
1).order(ByteOrder.BIG_ENDIAN);
+ padded.put((byte) 0x0);
Review Comment:
curious about why testing the padded byte buffer?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
