From 18f8b1617388e940d2f84003ae3165a543bef1b8 Mon Sep 17 00:00:00 2001 From: Heemin Kim Date: Mon, 14 Nov 2022 18:19:39 -0800 Subject: [PATCH] Revert changes in AbstractPointGeometryFieldMapper The change made in AbstractPointGeometryFieldMapper class with commit 050389736599e40c49278e900d5060f75b959282 introduced a performace degradation during point data indexing. Reverting it therefore. Signed-off-by: Heemin Kim --- .../AbstractPointGeometryFieldMapper.java | 74 ++++++------------- 1 file changed, 22 insertions(+), 52 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java index b5db3546185ba..658bcf295262d 100644 --- a/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -36,14 +36,11 @@ import org.opensearch.common.CheckedBiFunction; import org.opensearch.common.Explicit; import org.opensearch.common.ParseField; -import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.geo.GeoPoint; import org.opensearch.common.geo.GeometryFormat; import org.opensearch.common.geo.GeometryParser; -import org.opensearch.common.xcontent.DeprecationHandler; import org.opensearch.common.xcontent.LoggingDeprecationHandler; -import org.opensearch.common.xcontent.NamedXContentRegistry; import org.opensearch.common.xcontent.XContentBuilder; -import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentParser; import org.opensearch.geometry.Geometry; import org.opensearch.geometry.Point; @@ -245,7 +242,6 @@ default boolean isNormalizable(double coord) { * @opensearch.internal */ public static class PointParser

extends Parser> { - private static final int MAX_NUMBER_OF_VALUES_IN_ARRAY_FORMAT = 3; /** * Note that this parser is only used for formatting values. */ @@ -285,27 +281,32 @@ private P process(P in) { @Override public List

parse(XContentParser parser) throws IOException, ParseException { + if (parser.currentToken() == XContentParser.Token.START_ARRAY) { - parser.nextToken(); - if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { - XContentBuilder xContentBuilder = reconstructArrayXContent(parser); - try ( - XContentParser subParser = createParser( - parser.getXContentRegistry(), - parser.getDeprecationHandler(), - xContentBuilder - ); - ) { - return Collections.singletonList(process(objectParser.apply(subParser, pointSupplier.get()))); + XContentParser.Token token = parser.nextToken(); + P point = pointSupplier.get(); + ArrayList

points = new ArrayList<>(); + if (token == XContentParser.Token.VALUE_NUMBER) { + double x = parser.doubleValue(); + parser.nextToken(); + double y = parser.doubleValue(); + token = parser.nextToken(); + if (token == XContentParser.Token.VALUE_NUMBER) { + GeoPoint.assertZValue(ignoreZValue, parser.doubleValue()); + } else if (token != XContentParser.Token.END_ARRAY) { + throw new OpenSearchParseException("field type does not accept > 3 dimensions"); } + + point.resetCoords(x, y); + points.add(process(point)); } else { - ArrayList

points = new ArrayList<>(); - while (parser.currentToken() != XContentParser.Token.END_ARRAY) { - points.add(process(objectParser.apply(parser, pointSupplier.get()))); - parser.nextToken(); + while (token != XContentParser.Token.END_ARRAY) { + points.add(process(objectParser.apply(parser, point))); + point = pointSupplier.get(); + token = parser.nextToken(); } - return points; } + return points; } else if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { if (nullValue == null) { return null; @@ -317,37 +318,6 @@ public List

parse(XContentParser parser) throws IOException, ParseException { } } - private XContentParser createParser( - NamedXContentRegistry namedXContentRegistry, - DeprecationHandler deprecationHandler, - XContentBuilder xContentBuilder - ) throws IOException { - XContentParser subParser = xContentBuilder.contentType() - .xContent() - .createParser(namedXContentRegistry, deprecationHandler, BytesReference.bytes(xContentBuilder).streamInput()); - subParser.nextToken(); - return subParser; - } - - private XContentBuilder reconstructArrayXContent(XContentParser parser) throws IOException { - XContentBuilder builder = XContentFactory.jsonBuilder().startArray(); - int numberOfValuesAdded = 0; - while (parser.currentToken() != XContentParser.Token.END_ARRAY) { - if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) { - throw new OpenSearchParseException("numeric value expected"); - } - builder.value(parser.doubleValue()); - parser.nextToken(); - - // Allows one more value to be added so that the error case can be handled by a parser - if (++numberOfValuesAdded > MAX_NUMBER_OF_VALUES_IN_ARRAY_FORMAT) { - break; - } - } - builder.endArray(); - return builder; - } - @Override public Object format(List

points, String format) { List result = new ArrayList<>();