[incubator-pinot] branch master updated: some geo function improvements (#6306)

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 4be939a  some geo function improvements (#6306)
4be939a is described below

commit 4be939ae2a1a87c8e914208b1928d3f943698f4c
Author: Yupeng Fu <yu...@users.noreply.github.com>
AuthorDate: Wed Dec 2 14:46:30 2020 -0800

    some geo function improvements (#6306)
    
    ## Description
     - Add a third parameter to `ST_Point` to create the point in geography, this simplifies the geographic point creation. Instead of `ST_GeogFromText(ST_AsText(ST_Point(group_lat, group_lon))`, we can now use `ST_Point(group_lat, group_lon, 1)`.
     - Support `ST_Point` creation from literal such as `ST_Point(44.183189,-115.761905)`
     - Swapped latitude, longitude for `ST_Distance` calculation for geography.
    
    ## Upgrade Notes
    - The signature of `ST_Point` is changed to take an optional third parameter. 0 (default) means geometry, 1 means geography
    - In `ST_Distance(ST_Point( y1,x1), ST_Point(y2, x2))` is changed to `ST_Distance(ST_Point(x1, y1), ST_Point(x1, y2))` for geography
    
    ## Release Notes
    The sequence of latitude, longitude is changed for the points in ST_Distance
---
 .../pinot/common/utils/request/RequestUtils.java   | 10 ++++++++
 .../pinot/core/geospatial/GeometryUtils.java       | 16 +++++++++++++
 .../transform/function/ScalarFunctions.java        | 18 +++++++++++++++
 .../transform/function/StContainsFunction.java     | 12 +++++-----
 .../transform/function/StDistanceFunction.java     | 11 +++++----
 .../transform/function/StPointFunction.java        | 18 +++++++++------
 .../function/LiteralTransformFunction.java         |  8 ++++++-
 .../transform/StDistanceFunctionTest.java          |  8 +++----
 .../geospatial/transform/StPointFunctionTest.java  | 27 +++++++++++++++++++---
 .../meetupRsvp_realtime_table_config.json          |  4 +++-
 10 files changed, 105 insertions(+), 27 deletions(-)

diff --git a/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
index f7863cb..7e330b9 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
@@ -41,6 +41,7 @@ import org.apache.pinot.pql.parsers.pql2.ast.IntegerLiteralAstNode;
 import org.apache.pinot.pql.parsers.pql2.ast.LiteralAstNode;
 import org.apache.pinot.pql.parsers.pql2.ast.PredicateAstNode;
 import org.apache.pinot.pql.parsers.pql2.ast.StringLiteralAstNode;
+import org.apache.pinot.spi.utils.BytesUtils;
 import org.apache.pinot.sql.parsers.SqlCompilationException;
 
 
@@ -131,6 +132,12 @@ public class RequestUtils {
     return expression;
   }
 
+  public static Expression getLiteralExpression(byte[] value) {
+    Expression expression = createNewLiteralExpression();
+    expression.getLiteral().setStringValue(BytesUtils.toHexString(value));
+    return expression;
+  }
+
   public static Expression getLiteralExpression(Integer value) {
     return getLiteralExpression(value.longValue());
   }
@@ -170,6 +177,9 @@ public class RequestUtils {
     if (object instanceof SqlLiteral) {
       return RequestUtils.getLiteralExpression((SqlLiteral) object);
     }
+    if (object instanceof byte[]) {
+      return RequestUtils.getLiteralExpression((byte[]) object);
+    }
     throw new SqlCompilationException(
         new IllegalArgumentException("Unsupported Literal value type - " + object.getClass()));
   }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/GeometryUtils.java b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/GeometryUtils.java
index 309d4dd..b382dcd 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/GeometryUtils.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/GeometryUtils.java
@@ -53,4 +53,20 @@ public class GeometryUtils {
   public static boolean isGeography(Geometry geometry) {
     return geometry.getSRID() == GEOGRAPHY_SRID;
   }
+
+  /**
+   * Sets the geometry to geography.
+   * @param geometry the geometry to set
+   */
+  public static void setGeography(Geometry geometry) {
+    geometry.setSRID(GEOGRAPHY_SRID);
+  }
+
+  /**
+   * Sets to geometry.
+   * @param geometry the geometry to set
+   */
+  public static void setGeometry(Geometry geometry) {
+    geometry.setSRID(0);
+  }
 }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/ScalarFunctions.java b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/ScalarFunctions.java
index 59e4e40..523842b 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/ScalarFunctions.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/ScalarFunctions.java
@@ -22,6 +22,7 @@ import org.apache.pinot.core.geospatial.GeometryUtils;
 import org.apache.pinot.core.geospatial.serde.GeometrySerializer;
 import org.apache.pinot.spi.annotations.ScalarFunction;
 import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.jts.geom.Point;
 import org.locationtech.jts.io.WKTWriter;
 
 
@@ -44,6 +45,23 @@ public class ScalarFunctions {
   }
 
   /**
+   * Creates a point.
+   *
+   * @param x x
+   * @param y y
+   * @param isGeography if it's geography
+   * @return the created point
+   */
+  @ScalarFunction
+  public static byte[] stPoint(double x, double y, int isGeography) {
+    Point point = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(x, y));
+    if (isGeography > 0) {
+      GeometryUtils.setGeography(point);
+    }
+    return GeometrySerializer.serialize(point);
+  }
+
+  /**
    * Saves the geometry object as WKT format.
    *
    * @param bytes the serialized geometry object
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StContainsFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StContainsFunction.java
index ef4f3d5..c693e65 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StContainsFunction.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StContainsFunction.java
@@ -25,6 +25,7 @@ import org.apache.pinot.core.geospatial.serde.GeometrySerializer;
 import org.apache.pinot.core.operator.blocks.ProjectionBlock;
 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.LiteralTransformFunction;
 import org.apache.pinot.core.operator.transform.function.TransformFunction;
 import org.apache.pinot.core.plan.DocIdSetPlanNode;
 import org.apache.pinot.spi.data.FieldSpec;
@@ -57,14 +58,14 @@ public class StContainsFunction extends BaseTransformFunction {
     TransformFunction transformFunction = arguments.get(0);
     Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
         "First argument must be single-valued for transform function: %s", getName());
-    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES,
-        "The first argument must be of bytes type");
+    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES
+        || transformFunction instanceof LiteralTransformFunction, "The first argument must be of bytes type");
     _firstArgument = transformFunction;
     transformFunction = arguments.get(1);
     Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
         "Second argument must be single-valued for transform function: %s", getName());
-    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES,
-        "The second argument must be of bytes type");
+    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES
+        || transformFunction instanceof LiteralTransformFunction, "The second argument must be of bytes type");
     _secondArgument = transformFunction;
   }
 
@@ -84,8 +85,7 @@ public class StContainsFunction extends BaseTransformFunction {
       Geometry firstGeometry = GeometrySerializer.deserialize(firstValues[i]);
       Geometry secondGeometry = GeometrySerializer.deserialize(secondValues[i]);
       if (GeometryUtils.isGeography(firstGeometry) || GeometryUtils.isGeography(secondGeometry)) {
-        throw new RuntimeException(
-            String.format("%s is available for Geometry objects only", FUNCTION_NAME));
+        throw new RuntimeException(String.format("%s is available for Geometry objects only", FUNCTION_NAME));
       }
       _results[i] = firstGeometry.contains(secondGeometry) ? 1 : 0;
     }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StDistanceFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StDistanceFunction.java
index 0b9066f..d79fa6c 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StDistanceFunction.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StDistanceFunction.java
@@ -26,6 +26,7 @@ import org.apache.pinot.core.geospatial.serde.GeometrySerializer;
 import org.apache.pinot.core.operator.blocks.ProjectionBlock;
 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.LiteralTransformFunction;
 import org.apache.pinot.core.operator.transform.function.TransformFunction;
 import org.apache.pinot.core.plan.DocIdSetPlanNode;
 import org.apache.pinot.spi.data.FieldSpec;
@@ -71,14 +72,14 @@ public class StDistanceFunction extends BaseTransformFunction {
     TransformFunction transformFunction = arguments.get(0);
     Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
         "First argument must be single-valued for transform function: %s", getName());
-    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES,
-        "The first argument must be of bytes type");
+    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES
+        || transformFunction instanceof LiteralTransformFunction, "The first argument must be of bytes type");
     _firstArgument = transformFunction;
     transformFunction = arguments.get(1);
     Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
         "Second argument must be single-valued for transform function: %s", getName());
-    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES,
-        "The second argument must be of bytes type");
+    Preconditions.checkArgument(transformFunction.getResultMetadata().getDataType() == FieldSpec.DataType.BYTES
+        || transformFunction instanceof LiteralTransformFunction, "The second argument must be of bytes type");
     _secondArgument = transformFunction;
   }
 
@@ -126,7 +127,7 @@ public class StDistanceFunction extends BaseTransformFunction {
     Point leftPoint = (Point) leftGeometry;
     Point rightPoint = (Point) rightGeometry;
 
-    return greatCircleDistance(leftPoint.getY(), leftPoint.getX(), rightPoint.getY(), rightPoint.getX());
+    return greatCircleDistance(leftPoint.getX(), leftPoint.getY(), rightPoint.getX(), rightPoint.getY());
   }
 
   /**
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StPointFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StPointFunction.java
index 9fe93dc..2c09102 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StPointFunction.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/geospatial/transform/function/StPointFunction.java
@@ -20,15 +20,12 @@ package org.apache.pinot.core.geospatial.transform.function;
 
 import com.google.common.base.Preconditions;
 import org.apache.pinot.core.common.DataSource;
-import org.apache.pinot.core.geospatial.GeometryUtils;
-import org.apache.pinot.core.geospatial.serde.GeometrySerializer;
 import org.apache.pinot.core.operator.blocks.ProjectionBlock;
 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.LiteralTransformFunction;
 import org.apache.pinot.core.operator.transform.function.TransformFunction;
 import org.apache.pinot.core.plan.DocIdSetPlanNode;
-import org.locationtech.jts.geom.Coordinate;
-import org.locationtech.jts.geom.Point;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -46,6 +43,7 @@ public class StPointFunction extends BaseTransformFunction {
   private TransformFunction _firstArgument;
   private TransformFunction _secondArgument;
   private byte[][] _results;
+  private int _isGeography = 0;
 
   @Override
   public String getName() {
@@ -54,8 +52,8 @@ public class StPointFunction extends BaseTransformFunction {
 
   @Override
   public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
-    Preconditions
-        .checkArgument(arguments.size() == 2, "2 arguments are required for transform function: %s", getName());
+    Preconditions.checkArgument(arguments.size() == 2 || arguments.size() == 3,
+        "2 or 3 arguments are required for transform function: %s", getName());
     TransformFunction transformFunction = arguments.get(0);
     Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
         "First argument must be single-valued for transform function: %s", getName());
@@ -64,6 +62,12 @@ public class StPointFunction extends BaseTransformFunction {
     Preconditions.checkArgument(transformFunction.getResultMetadata().isSingleValue(),
         "Second argument must be single-valued for transform function: %s", getName());
     _secondArgument = transformFunction;
+    if (arguments.size() == 3) {
+      transformFunction = arguments.get(2);
+      Preconditions.checkArgument(transformFunction instanceof LiteralTransformFunction,
+          "Third argument must be a literal of integer: %s", getName());
+      _isGeography = Integer.parseInt(((LiteralTransformFunction) transformFunction).getLiteral());
+    }
   }
 
   @Override
@@ -79,7 +83,7 @@ public class StPointFunction extends BaseTransformFunction {
     double[] firstValues = _firstArgument.transformToDoubleValuesSV(projectionBlock);
     double[] secondValues = _secondArgument.transformToDoubleValuesSV(projectionBlock);
     for (int i = 0; i < projectionBlock.getNumDocs(); i++) {
-      _results[i] = ScalarFunctions.stPoint(firstValues[i], secondValues[i]);
+      _results[i] = ScalarFunctions.stPoint(firstValues[i], secondValues[i], _isGeography);
     }
     return _results;
   }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java
index e8f93a7..1b02887 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java
@@ -29,6 +29,7 @@ import org.apache.pinot.core.operator.transform.TransformResultMetadata;
 import org.apache.pinot.core.plan.DocIdSetPlanNode;
 import org.apache.pinot.core.segment.index.readers.Dictionary;
 import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.utils.BytesUtils;
 
 
 /**
@@ -42,6 +43,7 @@ public class LiteralTransformFunction implements TransformFunction {
   private float[] _floatResult;
   private double[] _doubleResult;
   private String[] _stringResult;
+  private byte[][] _bytesResult;
 
   public LiteralTransformFunction(String literal) {
     _literal = literal;
@@ -145,7 +147,11 @@ public class LiteralTransformFunction implements TransformFunction {
 
   @Override
   public byte[][] transformToBytesValuesSV(ProjectionBlock projectionBlock) {
-    throw new UnsupportedOperationException();
+    if (_bytesResult == null) {
+      _bytesResult = new byte[DocIdSetPlanNode.MAX_DOC_PER_CALL][];
+      Arrays.fill(_bytesResult, BytesUtils.toBytes(_literal));
+    }
+    return _bytesResult;
   }
 
   @Override
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StDistanceFunctionTest.java b/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StDistanceFunctionTest.java
index f21205d..bdaeaaf 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StDistanceFunctionTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StDistanceFunctionTest.java
@@ -48,10 +48,10 @@ public class StDistanceFunctionTest extends GeoFunctionTest {
   @Test
   public void testGeogDistance()
       throws Exception {
-    assertDistance("POINT(-86.67 36.12)", "POINT(-118.40 33.94)", 2886448.9734367016, false);
-    assertDistance("POINT(-118.40 33.94)", "POINT(-86.67 36.12)", 2886448.9734367016, false);
-    assertDistance("POINT(-71.0589 42.3601)", "POINT(-71.2290 42.4430)", 16734.69743457383, false);
-    assertDistance("POINT(-86.67 36.12)", "POINT(-86.67 36.12)", 0.0, false);
+    assertDistance("POINT(36.12 -86.67)", "POINT(33.94 -118.40)", 2886448.9734367016, false);
+    assertDistance("POINT(33.94 -118.40)", "POINT(36.12 -86.67)", 2886448.9734367016, false);
+    assertDistance("POINT(42.3601 -71.0589)", "POINT(42.4430 -71.2290)", 16734.69743457383, false);
+    assertDistance("POINT(36.12 -86.67)", "POINT(36.12 -86.67)", 0.0, false);
 
     //  (FIXME): the follow testings require null handling
 //    assertDistance("POINT EMPTY", "POINT (40 30)", null);
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StPointFunctionTest.java b/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StPointFunctionTest.java
index 08c5225..e0fee8e 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StPointFunctionTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/geospatial/transform/StPointFunctionTest.java
@@ -36,15 +36,36 @@ import org.testng.annotations.Test;
 
 public class StPointFunctionTest extends BaseTransformFunctionTest {
   @Test
-  public void testStPointFunction() {
-    ExpressionContext expression =
-        QueryContextConverterUtils.getExpression(String.format("ST_Point(%s,%s)", DOUBLE_SV_COLUMN, DOUBLE_SV_COLUMN));
+  public void testStPointGeogFunction() {
+    testStPointFunction(0);
+    testStPointFunction(1);
+  }
+
+  @Test
+  public void testStPointLiteralFunction() {
+    ExpressionContext expression = QueryContextConverterUtils.getExpression(String.format("ST_Point(20,10, 1)"));
+    TransformFunction transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
+    byte[][] expectedValues = new byte[NUM_ROWS][];
+    for (int i = 0; i < NUM_ROWS; i++) {
+      Point point = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(20, 10));
+      GeometryUtils.setGeography(point);
+      expectedValues[i] = GeometrySerializer.serialize(point);
+    }
+    testTransformFunction(transformFunction, expectedValues);
+  }
+
+  private void testStPointFunction(int isGeography) {
+    ExpressionContext expression = QueryContextConverterUtils
+        .getExpression(String.format("ST_Point(%s,%s, %d)", DOUBLE_SV_COLUMN, DOUBLE_SV_COLUMN, isGeography));
     TransformFunction transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
     Assert.assertTrue(transformFunction instanceof StPointFunction);
     Assert.assertEquals(transformFunction.getName(), StPointFunction.FUNCTION_NAME);
     byte[][] expectedValues = new byte[NUM_ROWS][];
     for (int i = 0; i < NUM_ROWS; i++) {
       Point point = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(_doubleSVValues[i], _doubleSVValues[i]));
+      if (isGeography > 0) {
+        GeometryUtils.setGeography(point);
+      }
       expectedValues[i] = GeometrySerializer.serialize(point);
     }
     testTransformFunction(transformFunction, expectedValues);
diff --git a/pinot-tools/src/main/resources/examples/stream/meetupRsvp/meetupRsvp_realtime_table_config.json b/pinot-tools/src/main/resources/examples/stream/meetupRsvp/meetupRsvp_realtime_table_config.json
index bc9e972..0014b3a 100644
--- a/pinot-tools/src/main/resources/examples/stream/meetupRsvp/meetupRsvp_realtime_table_config.json
+++ b/pinot-tools/src/main/resources/examples/stream/meetupRsvp/meetupRsvp_realtime_table_config.json
@@ -7,7 +7,9 @@
     "segmentPushType": "APPEND",
     "segmentAssignmentStrategy": "BalanceNumSegmentAssignmentStrategy",
     "schemaName": "meetupRsvp",
-    "replication": "1"
+    "replication": "1",
+    "retentionTimeUnit": "DAYS",
+    "retentionTimeValue": "1"
   },
   "tenants": {},
   "tableIndexConfig": {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org