[CARBONDATA-4166] Geo spatial Query Enhancements #4127
Conversation
|
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3542/ |
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5289/ |
|
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3545/ |
|
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5290/ |
|
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3549/ |
|
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5294/ |
Indhumathi27
changed the title
geo/src/main/java/org/apache/carbondata/geo/scan/expression/PolygonListExpression.java
Outdated
Show resolved
Hide resolved
geo/src/main/java/org/apache/carbondata/geo/scan/expression/PolygonRangeListExpression.java
Outdated
Show resolved
Hide resolved
| @@ -30,6 +32,7 @@ object GeoUtilUDFs { | |||
| sparkSession.udf.register("LatLngToGeoId", new LatLngToGeoIdUDF) | |||
| sparkSession.udf.register("ToUpperLayerGeoId", new ToUpperLayerGeoIdUDF) | |||
| sparkSession.udf.register("ToRangeList", new ToRangeListUDF) | |||
| sparkSession.udf.register("ToRangeListAsString", new ToRangeListAsStringUDF) | |||
There was a problem hiding this comment.
these UDF are exposed to user right ? can we update in the document ?
There was a problem hiding this comment.
This udf is not exposed to user. This is for internal purpose only.
| val matchedStr = matcher.group | ||
| range = matchedStr | ||
| } | ||
| val ranges = PolygonRangeListExpression.getRangeListFromString(range) |
There was a problem hiding this comment.
here we need to check against null for ranges? some places we check and some places we don't. (example ToRangeListAsStringUDF) can we make it uniform? If not required, remove from other places also. also maybe extract a common method for matcher and find if possible.
There was a problem hiding this comment.
NULL check is already handled here in line:49. Handled some refactoring to extract common code to new method.
| import org.apache.carbondata.geo.scan.expression.PolygonRangeListExpression | ||
| import org.apache.carbondata.spark.rdd.CarbonScanRDD | ||
|
|
||
| case class BroadCastPolygonFilterPushJoin( |
There was a problem hiding this comment.
many things are common with BroadCastSIFilterPushJoin, we cannot extend the same class ?
There was a problem hiding this comment.
BroadCastPolygonFilterPushJoin implementation is not same as BroadCastSIFilterPushJoin in terms of filter and method definition. so, better to keep it seperate.
...ark/src/main/scala/org/apache/spark/sql/execution/joins/BroadCastPolygonFilterPushJoin.scala
Show resolved
Hide resolved
integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DMLStrategy.scala
Outdated
Show resolved
Hide resolved
|
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5308/ |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3563/ |
|
LGTM |
Why is this PR needed?
specified in SQL. If the polygon list grows in size, then the SQL will also be too long,
which may affect query performance, as SQL analysing cost will be more.
table join can be supported in order to support aggregation on spatial table columns
based on polygons.
What changes were proposed in this PR?
polygon table.
Does this PR introduce any user interface change?
Is any new testcase added?