>From Tin Vu <[email protected]>: Tin Vu has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224 )
Change subject: [ASTERIXDB-2839] Added Optimized Spatial Joins ...................................................................... Patch Set 7: (4 comments) https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FilterRefineSpatialDistanceJoin.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FilterRefineSpatialDistanceJoin.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FilterRefineSpatialDistanceJoin.java@60 PS6, Line 60: FilterRefineSpatialDistanceJoin Same comments as with FilterRefineSpatialJoin: refactor class name and add logical plan changes in Javadoc. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FilterRefineSpatialJoin.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FilterRefineSpatialJoin.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FilterRefineSpatialJoin.java@53 PS6, Line 53: : * SELECT COUNT(*) FROM ParkSet AS ps, LakeSet AS ls : * WHERE /*+ spatial-partitioning -180.0 -83.0 180.0 90.0 10 10 */ st_intersects(ps.geom,ls.geom); : * : * Becomes, : * : * SELECT COUNT(*) FROM ParkSet AS ps, LakeSet AS ls : * WHERE /*+ spatial-partitioning -180.0 -83.0 180.0 90.0 10 10 */ : * spatial_intersect(st_mbr(ps.geom),st_mbr(ls.geom)) and st_intersects(ps.geom,ls.geom); Comment from Preston: it is good to make an example of SQL++ query to show how this rule change a query. However, this is not enough to reflect what the rule does. This rule is applied in the logical level so that it would be better if we can provide another example that show how the 'logical plan' looks like before and after this rule. In particular, it should shows that this rule 'find the join operator with spatial-related condition then update the join condition'. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FilterRefineSpatialJoin.java@66 PS6, Line 66: FilterRefineSpatialJoin Refactor this class/file name to ***Rule https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224/6/asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/AbstractSTGeometryDoubleNDescriptor.java File asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/AbstractSTGeometryDoubleNDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224/6/asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/AbstractSTGeometryDoubleNDescriptor.java@50 PS6, Line 50: n Can you change this variable name to d or e, which imply that this is a double number? n looks like an integer number. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/10224 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: Ibea81d8dda6084c42e04dc528e8c245ed2177226 Gerrit-Change-Number: 10224 Gerrit-PatchSet: 7 Gerrit-Owner: Tin Vu <[email protected]> Gerrit-Reviewer: Akil Sevim <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Tin Vu <[email protected]> Gerrit-Comment-Date: Sat, 27 Feb 2021 20:39:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
