Revert changes in AbstractPointGeometryFieldMapper

The change made in AbstractPointGeometryFieldMapper class with commit 0503897 introduced a performace degradation during point data indexing. Reverting it therefore. Signed-off-by: Heemin Kim <heemin@amazon.com>
opensearch-project · Nov 15, 2022 · 18f8b16 · 18f8b16
1 parent b43e985
commit 18f8b16
Showing 1 changed file with 22 additions and 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
@@ -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<P extends ParsedPoint> extends Parser<List<P>> {
-        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<P> 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<P> 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<P> 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<P> 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<P> points, String format) {
             List<Object> result = new ArrayList<>();