Jackie-Jiang commented on code in PR #14405:
URL: https://github.com/apache/pinot/pull/14405#discussion_r1857452451
##########
pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java:
##########
@@ -176,8 +176,13 @@ public enum TransformFunctionType {
// Geo constructors
ST_GEOG_FROM_TEXT("ST_GeogFromText", ReturnTypes.VARBINARY,
OperandTypes.CHARACTER),
ST_GEOM_FROM_TEXT("ST_GeomFromText", ReturnTypes.VARBINARY,
OperandTypes.CHARACTER),
+
+ ST_GEOG_FROM_GEO_JSON("ST_GeogFromGeoJson", ReturnTypes.VARBINARY,
OperandTypes.CHARACTER),
+ ST_GEOM_FROM_GEO_JSON("ST_GeomFromGeoJson", ReturnTypes.VARBINARY,
OperandTypes.CHARACTER),
Review Comment:
Keep the name consistent as the standard one, same for other places
```suggestion
ST_GEOM_FROM_GEO_JSON("ST_GeomFromGeoJSON", ReturnTypes.VARBINARY,
OperandTypes.CHARACTER),
```
##########
pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/ConstructFromGeoJsonFunction.java:
##########
@@ -0,0 +1,94 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.geospatial.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.Utils;
+import org.apache.pinot.core.operator.ColumnContext;
+import org.apache.pinot.core.operator.blocks.ValueBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.operator.transform.function.BaseTransformFunction;
+import org.apache.pinot.core.operator.transform.function.TransformFunction;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+import org.apache.pinot.segment.local.utils.GeometrySerializer;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.io.ParseException;
+import org.locationtech.jts.io.geojson.GeoJsonReader;
+
+
+/**
+ * An abstract class for implementing the geo constructor functions from GEO
JSON.
+ */
+abstract class ConstructFromGeoJsonFunction extends BaseTransformFunction {
+
+ protected TransformFunction _transformFunction;
+ protected byte[][] _results;
Review Comment:
You can directly use `_bytesValuesSV`, and `initBytesValuesSV()` from the
base class, same for other functions
##########
pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StAsGeoJsonFunction.java:
##########
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.geospatial.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.ColumnContext;
+import org.apache.pinot.core.operator.blocks.ValueBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.operator.transform.function.BaseTransformFunction;
+import org.apache.pinot.core.operator.transform.function.TransformFunction;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+import org.apache.pinot.segment.local.utils.GeometrySerializer;
+import org.apache.pinot.segment.local.utils.GeometryUtils;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.locationtech.jts.geom.Geometry;
+
+
+/**
+ * Returns the GEOJson representation of the geometry object.
+ */
+public class StAsGeoJsonFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "ST_AsGeoJson";
+
+ private TransformFunction _transformFunction;
+ private String[] _results;
+
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String,
ColumnContext> columnContextMap) {
+ super.init(arguments, columnContextMap);
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exactly 1 argument is required for transform function: " +
FUNCTION_NAME);
+
+ TransformFunction transformFunction = arguments.get(0);
+
Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
+ "Argument must be single-valued for transform function: " +
FUNCTION_NAME);
+
Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType()
== FieldSpec.DataType.BYTES,
+ "The argument must be of bytes type");
+
+ _transformFunction = transformFunction;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return STRING_SV_NO_DICTIONARY_METADATA;
+ }
+
+ public String[] transformToStringValuesSV(ValueBlock valueBlock) {
+ if (_results == null) {
+ _results = new String[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ byte[][] values = _transformFunction.transformToBytesValuesSV(valueBlock);
+ // use single buffer instead of allocating separate StringBuffer per row
+ StringBuilderWriter buffer = new StringBuilderWriter();
+
+ try {
+ for (int i = 0; i < valueBlock.getNumDocs(); i++) {
+ Geometry geometry = GeometrySerializer.deserialize(values[i]);
+ GeometryUtils.GEO_JSON_WRITER.write(geometry, buffer);
+ _results[i] = buffer.getString();
+ buffer.clear();
+ }
+ } catch (IOException ioe) {
+ // should never happen
Review Comment:
Even though it should not happen, let's wrap it as a runtime exception and
throw it to guarantee it works as expected. Same for other places
##########
pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/ConstructFromGeoJsonFunction.java:
##########
@@ -0,0 +1,94 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.geospatial.transform.function;
+
+import com.google.common.base.Preconditions;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.Utils;
+import org.apache.pinot.core.operator.ColumnContext;
+import org.apache.pinot.core.operator.blocks.ValueBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.operator.transform.function.BaseTransformFunction;
+import org.apache.pinot.core.operator.transform.function.TransformFunction;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+import org.apache.pinot.segment.local.utils.GeometrySerializer;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.io.ParseException;
+import org.locationtech.jts.io.geojson.GeoJsonReader;
+
+
+/**
+ * An abstract class for implementing the geo constructor functions from GEO
JSON.
+ */
+abstract class ConstructFromGeoJsonFunction extends BaseTransformFunction {
+
+ protected TransformFunction _transformFunction;
+ protected byte[][] _results;
+ protected GeoJsonReader _reader;
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String,
ColumnContext> columnContextMap) {
+ super.init(arguments, columnContextMap);
+
+ Preconditions.checkArgument(arguments.size() == 1,
+ "Exactly 1 argument is required for transform function: " + getName());
+
+ TransformFunction transformFunction = arguments.get(0);
+
+
Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
+ "The argument must be single-valued for transform function: " +
getName());
+
Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType()
== FieldSpec.DataType.STRING,
+ "The argument must be of string type");
+
+ _transformFunction = transformFunction;
+ _reader = getGeoJsonReader();
+ }
+
+ abstract protected GeoJsonReader getGeoJsonReader();
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BYTES_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public byte[][] transformToBytesValuesSV(ValueBlock valueBlock) {
+ if (_results == null) {
+ _results = new byte[DocIdSetPlanNode.MAX_DOC_PER_CALL][];
+ }
+ String[] argumentValues =
_transformFunction.transformToStringValuesSV(valueBlock);
+ // use single reader instead of allocating separate instance per row
+ StringReader reader = new StringReader();
+
+ int length = valueBlock.getNumDocs();
+ for (int i = 0; i < length; i++) {
+ try {
+ reader.setString(argumentValues[i]);
+ Geometry geometry = _reader.read(reader);
+ _results[i] = GeometrySerializer.serialize(geometry);
+ } catch (ParseException e) {
+ Utils.rethrowException(
Review Comment:
No need to use re-throw for runtime exception
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
