Skip to content

Commit

Permalink
Rename some functions in SpatialCoordinateTypes (#104201)
Browse files Browse the repository at this point in the history
  • Loading branch information
iverase authored Jan 10, 2024
1 parent 709c0f5 commit 4b21499
Show file tree
Hide file tree
Showing 20 changed files with 60 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -447,11 +447,11 @@ public void testBytesRefBlock() {
}

public void testBytesRefBlockOnGeoPoints() {
testBytesRefBlock(() -> GEO.pointAsWKB(GeometryTestUtils.randomPoint()), false, GEO::wkbAsString);
testBytesRefBlock(() -> GEO.asWkb(GeometryTestUtils.randomPoint()), false, GEO::wkbToWkt);
}

public void testBytesRefBlockOnCartesianPoints() {
testBytesRefBlock(() -> CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint()), false, CARTESIAN::wkbAsString);
testBytesRefBlock(() -> CARTESIAN.asWkb(ShapeTestUtils.randomPoint()), false, CARTESIAN::wkbToWkt);
}

public void testBytesRefBlockBuilderWithNulls() {
Expand Down Expand Up @@ -930,7 +930,7 @@ public static RandomBlock randomBlock(
}
case BYTES_REF -> {
BytesRef b = bytesRefFromPoints
? GEO.pointAsWKB(pointSupplier.get())
? GEO.asWkb(pointSupplier.get())
: new BytesRef(randomRealisticUnicodeOfLength(4));
valuesAtPosition.add(b);
((BytesRefBlock.Builder) builder).appendBytesRef(b);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ public static void assertData(
if (expectedType == Type.DATETIME) {
expectedValue = rebuildExpected(expectedValue, Long.class, x -> UTC_DATE_TIME_FORMATTER.formatMillis((long) x));
} else if (expectedType == Type.GEO_POINT) {
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbAsString((BytesRef) x));
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbToWkt((BytesRef) x));
} else if (expectedType == Type.CARTESIAN_POINT) {
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> CARTESIAN.wkbAsString((BytesRef) x));
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> CARTESIAN.wkbToWkt((BytesRef) x));
} else if (expectedType == Type.IP) {
// convert BytesRef-packed IP to String, allowing subsequent comparison with what's expected
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> DocValueFormat.IP.format((BytesRef) x));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,8 @@ public enum Type {
Long.class
),
BOOLEAN(Booleans::parseBoolean, Boolean.class),
GEO_POINT(x -> x == null ? null : GEO.stringAsWKB(x), BytesRef.class),
CARTESIAN_POINT(x -> x == null ? null : CARTESIAN.stringAsWKB(x), BytesRef.class);
GEO_POINT(x -> x == null ? null : GEO.wktToWkb(x), BytesRef.class),
CARTESIAN_POINT(x -> x == null ? null : CARTESIAN.wktToWkb(x), BytesRef.class);

private static final Map<String, Type> LOOKUP = new HashMap<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,14 @@ protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Pa
@Override
protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Params params, int valueIndex)
throws IOException {
return builder.value(GEO.wkbAsString(((BytesRefBlock) block).getBytesRef(valueIndex, scratch)));
return builder.value(GEO.wkbToWkt(((BytesRefBlock) block).getBytesRef(valueIndex, scratch)));
}
};
case "cartesian_point" -> new PositionToXContent(block) {
@Override
protected XContentBuilder valueToXContent(XContentBuilder builder, ToXContent.Params params, int valueIndex)
throws IOException {
return builder.value(CARTESIAN.wkbAsString(((BytesRefBlock) block).getBytesRef(valueIndex, scratch)));
return builder.value(CARTESIAN.wkbToWkt(((BytesRefBlock) block).getBytesRef(valueIndex, scratch)));
}
};
case "boolean" -> new PositionToXContent(block) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ private static Object valueAt(String dataType, Block block, int offset, BytesRef
}
case "boolean" -> ((BooleanBlock) block).getBoolean(offset);
case "version" -> new Version(((BytesRefBlock) block).getBytesRef(offset, scratch)).toString();
case "geo_point" -> GEO.wkbAsString(((BytesRefBlock) block).getBytesRef(offset, scratch));
case "cartesian_point" -> CARTESIAN.wkbAsString(((BytesRefBlock) block).getBytesRef(offset, scratch));
case "geo_point" -> GEO.wkbToWkt(((BytesRefBlock) block).getBytesRef(offset, scratch));
case "cartesian_point" -> CARTESIAN.wkbToWkt(((BytesRefBlock) block).getBytesRef(offset, scratch));
case "unsupported" -> UnsupportedValueSource.UNSUPPORTED_OUTPUT;
case "_source" -> {
BytesRef val = ((BytesRefBlock) block).getBytesRef(offset, scratch);
Expand Down Expand Up @@ -163,12 +163,12 @@ static Page valuesToPage(BlockFactory blockFactory, List<ColumnInfo> columns, Li
}
case "geo_point" -> {
// This just converts WKT to WKB, so does not need CRS knowledge, we could merge GEO and CARTESIAN here
BytesRef wkb = GEO.stringAsWKB(value.toString());
BytesRef wkb = GEO.wktToWkb(value.toString());
((BytesRefBlock.Builder) builder).appendBytesRef(wkb);
}
case "cartesian_point" -> {
// This just converts WKT to WKB, so does not need CRS knowledge, we could merge GEO and CARTESIAN here
BytesRef wkb = CARTESIAN.stringAsWKB(value.toString());
BytesRef wkb = CARTESIAN.wktToWkb(value.toString());
((BytesRefBlock.Builder) builder).appendBytesRef(wkb);
}
default -> throw EsqlIllegalArgumentException.illegalDataType(dataTypes.get(c));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ protected NodeInfo<? extends Expression> info() {

@ConvertEvaluator(extraName = "FromString", warnExceptions = { IllegalArgumentException.class })
static BytesRef fromKeyword(BytesRef in) {
return CARTESIAN.stringAsWKB(in.utf8ToString());
return CARTESIAN.wktToWkb(in.utf8ToString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ protected NodeInfo<? extends Expression> info() {

@ConvertEvaluator(extraName = "FromString", warnExceptions = { IllegalArgumentException.class })
static BytesRef fromKeyword(BytesRef in) {
return GEO.stringAsWKB(in.utf8ToString());
return GEO.wktToWkb(in.utf8ToString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ static BytesRef fromUnsignedLong(long lng) {

@ConvertEvaluator(extraName = "FromGeoPoint")
static BytesRef fromGeoPoint(BytesRef wkb) {
return new BytesRef(GEO.wkbAsString(wkb));
return new BytesRef(GEO.wkbToWkt(wkb));
}

@ConvertEvaluator(extraName = "FromCartesianPoint")
static BytesRef fromCartesianPoint(BytesRef wkb) {
return new BytesRef(CARTESIAN.wkbAsString(wkb));
return new BytesRef(CARTESIAN.wkbToWkt(wkb));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1642,7 +1642,7 @@ private static Object mapToLiteralValue(PlanStreamInput in, DataType dataType, O
}

private static BytesRef longAsWKB(DataType dataType, long encoded) {
return dataType == GEO_POINT ? GEO.longAsWKB(encoded) : CARTESIAN.longAsWKB(encoded);
return dataType == GEO_POINT ? GEO.longAsWkb(encoded) : CARTESIAN.longAsWkb(encoded);
}

private static long wkbAsLong(DataType dataType, BytesRef wkb) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,8 @@ private Page randomPage(List<ColumnInfo> columns) {
new BytesRef(UnsupportedValueSource.UNSUPPORTED_OUTPUT)
);
case "version" -> ((BytesRefBlock.Builder) builder).appendBytesRef(new Version(randomIdentifier()).toBytesRef());
case "geo_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(GEO.pointAsWKB(GeometryTestUtils.randomPoint()));
case "cartesian_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(
CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint())
);
case "geo_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(GEO.asWkb(GeometryTestUtils.randomPoint()));
case "cartesian_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(CARTESIAN.asWkb(ShapeTestUtils.randomPoint()));
case "null" -> builder.appendNull();
case "_source" -> {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ public static Literal randomLiteral(DataType type) {
case "time_duration" -> Duration.ofMillis(randomLongBetween(-604800000L, 604800000L)); // plus/minus 7 days
case "text" -> new BytesRef(randomAlphaOfLength(50));
case "version" -> randomVersion().toBytesRef();
case "geo_point" -> GEO.pointAsWKB(GeometryTestUtils.randomPoint());
case "cartesian_point" -> CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint());
case "geo_point" -> GEO.asWkb(GeometryTestUtils.randomPoint());
case "cartesian_point" -> CARTESIAN.asWkb(ShapeTestUtils.randomPoint());
case "null" -> null;
case "_source" -> {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -913,18 +913,12 @@ public static List<TypedDataSupplier> timeDurationCases() {
}

private static List<TypedDataSupplier> geoPointCases() {
return List.of(
new TypedDataSupplier("<geo_point>", () -> GEO.pointAsWKB(GeometryTestUtils.randomPoint()), EsqlDataTypes.GEO_POINT)
);
return List.of(new TypedDataSupplier("<geo_point>", () -> GEO.asWkb(GeometryTestUtils.randomPoint()), EsqlDataTypes.GEO_POINT));
}

private static List<TypedDataSupplier> cartesianPointCases() {
return List.of(
new TypedDataSupplier(
"<cartesian_point>",
() -> CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint()),
EsqlDataTypes.CARTESIAN_POINT
)
new TypedDataSupplier("<cartesian_point>", () -> CARTESIAN.asWkb(ShapeTestUtils.randomPoint()), EsqlDataTypes.CARTESIAN_POINT)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static Iterable<Object[]> parameters() {
EsqlDataTypes.CARTESIAN_POINT,
bytesRef -> null,
bytesRef -> {
var exception = expectThrows(Exception.class, () -> CARTESIAN.stringAsWKB(bytesRef.utf8ToString()));
var exception = expectThrows(Exception.class, () -> CARTESIAN.wktToWkb(bytesRef.utf8ToString()));
return List.of(
"Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.",
"Line -1:-1: " + exception
Expand All @@ -60,12 +60,12 @@ public static Iterable<Object[]> parameters() {
List.of(
new TestCaseSupplier.TypedDataSupplier(
"<cartesian point as string>",
() -> new BytesRef(CARTESIAN.pointAsString(ShapeTestUtils.randomPoint())),
() -> new BytesRef(CARTESIAN.asWkt(ShapeTestUtils.randomPoint())),
DataTypes.KEYWORD
)
),
EsqlDataTypes.CARTESIAN_POINT,
bytesRef -> CARTESIAN.stringAsWKB(((BytesRef) bytesRef).utf8ToString()),
bytesRef -> CARTESIAN.wktToWkb(((BytesRef) bytesRef).utf8ToString()),
List.of()
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static Iterable<Object[]> parameters() {
EsqlDataTypes.GEO_POINT,
bytesRef -> null,
bytesRef -> {
var exception = expectThrows(Exception.class, () -> GEO.stringAsWKB(bytesRef.utf8ToString()));
var exception = expectThrows(Exception.class, () -> GEO.wktToWkb(bytesRef.utf8ToString()));
return List.of(
"Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.",
"Line -1:-1: " + exception
Expand All @@ -60,12 +60,12 @@ public static Iterable<Object[]> parameters() {
List.of(
new TestCaseSupplier.TypedDataSupplier(
"<geo point as string>",
() -> new BytesRef(GEO.pointAsString(GeometryTestUtils.randomPoint())),
() -> new BytesRef(GEO.asWkt(GeometryTestUtils.randomPoint())),
DataTypes.KEYWORD
)
),
EsqlDataTypes.GEO_POINT,
bytesRef -> GEO.stringAsWKB(((BytesRef) bytesRef).utf8ToString()),
bytesRef -> GEO.wktToWkb(((BytesRef) bytesRef).utf8ToString()),
List.of()
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ public static Iterable<Object[]> parameters() {
suppliers,
"ToStringFromGeoPointEvaluator[field=" + read + "]",
DataTypes.KEYWORD,
wkb -> new BytesRef(GEO.wkbAsString(wkb)),
wkb -> new BytesRef(GEO.wkbToWkt(wkb)),
List.of()
);
TestCaseSupplier.forUnaryCartesianPoint(
suppliers,
"ToStringFromCartesianPointEvaluator[field=" + read + "]",
DataTypes.KEYWORD,
wkb -> new BytesRef(CARTESIAN.wkbAsString(wkb)),
wkb -> new BytesRef(CARTESIAN.wkbToWkt(wkb)),
List.of()
);
TestCaseSupplier.forUnaryIp(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ protected static void points(
BiFunction<Integer, Stream<BytesRef>, Matcher<Object>> matcher
) {
cases.add(new TestCaseSupplier(name + "(" + dataType.typeName() + ")", List.of(dataType), () -> {
BytesRef wkb = spatial.pointAsWKB(randomPoint.get());
BytesRef wkb = spatial.asWkb(randomPoint.get());
return new TestCaseSupplier.TestCase(
List.of(new TestCaseSupplier.TypedData(List.of(wkb), dataType, "field")),
evaluatorName + "[field=Attribute[channel=0]]",
Expand All @@ -479,7 +479,7 @@ protected static void points(
}));
for (Block.MvOrdering ordering : Block.MvOrdering.values()) {
cases.add(new TestCaseSupplier(name + "(<" + dataType.typeName() + "s>) " + ordering, List.of(dataType), () -> {
List<BytesRef> mvData = randomList(1, 100, () -> spatial.pointAsWKB(randomPoint.get()));
List<BytesRef> mvData = randomList(1, 100, () -> spatial.asWkb(randomPoint.get()));
putInOrder(mvData, ordering);
return new TestCaseSupplier.TestCase(
List.of(new TestCaseSupplier.TypedData(mvData, dataType, "field")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ private static EsqlQueryResponse regularData() {
);

BytesRefArray geoPoints = new BytesRefArray(2, BigArrays.NON_RECYCLING_INSTANCE);
geoPoints.append(GEO.pointAsWKB(new Point(12, 56)));
geoPoints.append(GEO.pointAsWKB(new Point(-97, 26)));
geoPoints.append(GEO.asWkb(new Point(12, 56)));
geoPoints.append(GEO.asWkb(new Point(-97, 26)));
// values
List<Page> values = List.of(
new Page(
Expand All @@ -272,8 +272,8 @@ private static EsqlQueryResponse regularData() {
blockFactory.newIntArrayVector(new int[] { 11 * 60 + 48, 4 * 60 + 40 }, 2).asBlock(),
blockFactory.newBytesRefArrayVector(geoPoints, 2).asBlock(),
blockFactory.newBytesRefBlockBuilder(2)
.appendBytesRef(CARTESIAN.pointAsWKB(new Point(1234, 5678)))
.appendBytesRef(CARTESIAN.pointAsWKB(new Point(-9753, 2611)))
.appendBytesRef(CARTESIAN.asWkb(new Point(1234, 5678)))
.appendBytesRef(CARTESIAN.asWkb(new Point(-9753, 2611)))
.build()
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ public class TextFormatterTests extends ESTestCase {

private static final BytesRefArray geoPoints = new BytesRefArray(2, BigArrays.NON_RECYCLING_INSTANCE);
static {
geoPoints.append(GEO.pointAsWKB(new Point(12, 56)));
geoPoints.append(GEO.pointAsWKB(new Point(-97, 26)));
geoPoints.append(GEO.asWkb(new Point(12, 56)));
geoPoints.append(GEO.asWkb(new Point(-97, 26)));
}

EsqlQueryResponse esqlResponse = new EsqlQueryResponse(
Expand All @@ -72,8 +72,8 @@ public class TextFormatterTests extends ESTestCase {
).asBlock(),
blockFactory.newBytesRefArrayVector(geoPoints, 2).asBlock(),
blockFactory.newBytesRefBlockBuilder(2)
.appendBytesRef(CARTESIAN.pointAsWKB(new Point(1234, 5678)))
.appendBytesRef(CARTESIAN.pointAsWKB(new Point(-9753, 2611)))
.appendBytesRef(CARTESIAN.asWkb(new Point(1234, 5678)))
.appendBytesRef(CARTESIAN.asWkb(new Point(-9753, 2611)))
.build(),
blockFactory.newConstantNullBlock(2)
)
Expand Down Expand Up @@ -146,8 +146,8 @@ public void testFormatWithoutHeader() {
).asBlock(),
blockFactory.newBytesRefArrayVector(geoPoints, 2).asBlock(),
blockFactory.newBytesRefBlockBuilder(2)
.appendBytesRef(CARTESIAN.pointAsWKB(new Point(1234, 5678)))
.appendBytesRef(CARTESIAN.pointAsWKB(new Point(-9753, 2611)))
.appendBytesRef(CARTESIAN.asWkb(new Point(1234, 5678)))
.appendBytesRef(CARTESIAN.asWkb(new Point(-9753, 2611)))
.build(),
blockFactory.newConstantNullBlock(2)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,6 @@ public long pointAsLong(double x, double y) {

public abstract long pointAsLong(double x, double y);

public String pointAsString(Point point) {
return WellKnownText.toWKT(point);
}

public BytesRef pointAsWKB(Point point) {
return new BytesRef(WellKnownBinary.toWKB(point, ByteOrder.LITTLE_ENDIAN));
}

public BytesRef longAsWKB(long encoded) {
return pointAsWKB(longAsPoint(encoded));
}

public long wkbAsLong(BytesRef wkb) {
Geometry geometry = WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
if (geometry instanceof Point point) {
Expand All @@ -83,18 +71,30 @@ public long wkbAsLong(BytesRef wkb) {
}
}

public BytesRef stringAsWKB(String string) {
public BytesRef longAsWkb(long encoded) {
return asWkb(longAsPoint(encoded));
}

public String asWkt(Geometry geometry) {
return WellKnownText.toWKT(geometry);
}

public BytesRef asWkb(Geometry geometry) {
return new BytesRef(WellKnownBinary.toWKB(geometry, ByteOrder.LITTLE_ENDIAN));
}

public BytesRef wktToWkb(String wkt) {
// TODO: we should be able to transform WKT to WKB without building the geometry
// we should as well use different validator for cartesian and geo?
try {
Geometry geometry = WellKnownText.fromWKT(GeometryValidator.NOOP, false, string);
Geometry geometry = WellKnownText.fromWKT(GeometryValidator.NOOP, false, wkt);
return new BytesRef(WellKnownBinary.toWKB(geometry, ByteOrder.LITTLE_ENDIAN));
} catch (Exception e) {
throw new IllegalArgumentException("Failed to parse WKT: " + e.getMessage(), e);
}
}

public String wkbAsString(BytesRef wkb) {
public String wkbToWkt(BytesRef wkb) {
return WellKnownText.fromWKB(wkb.bytes, wkb.offset, wkb.length);
}
}
Loading

0 comments on commit 4b21499

Please sign in to comment.