Yingyi Bu has posted comments on this change. Change subject: Change logical plan to apply filter from 2ndary index ......................................................................
Patch Set 11: (27 comments) https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/BTreeSearchPOperator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/BTreeSearchPOperator.java: PS11, Line 118: getPropagateIndexFilter getPropageIndexFilter -> propagateIndexFilter https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/IndexSearchPOperator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/IndexSearchPOperator.java: PS11, Line 66: getScanVariables Can it be just as.getVariables()? https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/InvertedIndexPOperator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/InvertedIndexPOperator.java: PS11, Line 125: propagateIndexFilter It looks that propagateIndexFilter can be retrieved from unnestMapOp so you do not need this parameter? https://asterix-gerrit.ics.uci.edu/#/c/1727/7/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java: PS7, Line 355: sta What does this TODO exactly mean? https://asterix-gerrit.ics.uci.edu/#/c/1727/7/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceLSMComponentFilterRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceLSMComponentFilterRule.java: PS7, Line 241: ArrayList ArrayList -> List https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceLSMComponentFilterRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceLSMComponentFilterRule.java: PS11, Line 244: Mutable Generalize the filter propagation using a loop? In other words, make the code agnostic to specific operators (e.g., intersect, order) ? For now, you can assume there is no variables that kills variables. Maybe I should add a general visitor to enforce variables later because it is needed in several places. PS11, Line 271: warning Throw an exception? PS11, Line 313: ArrayList ArrayList -> List PS11, Line 317: AbstractLogicalOperator cast is not needed: ILogicalOperator descendantOp = queue.poll().getValue(); PS11, Line 319: continue It should never be null? PS11, Line 328: IllegalStateException error code https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/btree-btree-search-wo-query-filter.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/btree-btree-search-wo-query-filter.aql: PS11, Line 36: output write statement is not needed. https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/btree-btree-search.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/btree-btree-search.aql: PS11, Line 36: rttest write statement is not needed. https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/inverted-btree-search-wo-query-filter.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/inverted-btree-search-wo-query-filter.aql: PS11, Line 36: filter write statement is not needed. https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/inverted-btree-search.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/inverted-btree-search.aql: PS11, Line 36: output write statement is not needed. https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/multi-index-btree-search-wo-query-filter.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/multi-index-btree-search-wo-query-filter.aql: PS11, Line 38: nc1 write statement is not needed https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/multi-index-btree-search.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/multi-index-btree-search.aql: PS11, Line 38: output write statement is not needed. https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/rtree-btree-search-wo-query-filter.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/rtree-btree-search-wo-query-filter.aql: PS11, Line 36: output write statement is not needed https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/rtree-btree-search.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/filter/rtree-btree-search.aql: PS11, Line 36: output write statement is not needed https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/optimizerts/results/filter/btree-btree-search-wo-query-filter.plan File asterixdb/asterix-app/src/test/resources/optimizerts/results/filter/btree-btree-search-wo-query-filter.plan: PS11, Line 9: ONE_TO_ONE_EXCHANGE Is the physical plan here different with vs. without filter? If not, should we add the filter into the physical plan, e.g., in the primary BTree Search, if there is a filter? https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-app/src/test/resources/runtimets/queries/filters/delete/delete.11.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/filters/delete/delete.11.ddl.aql: PS11, Line 20: drop The last drop dataverse is not needed because after each test, the test executor automatically clean up dataverses. https://asterix-gerrit.ics.uci.edu/#/c/1727/11/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java: PS11, Line 430: boolean propagateFilter Is the boolean info somehow can be obtained from Dataset dataset, since filter should be a property of the dataset? PS11, Line 476: propagateFilter Is the boolean info somehow can be obtained from Dataset dataset, since filter should be a property of the dataset? https://asterix-gerrit.ics.uci.edu/#/c/1727/11/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractScanOperator.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractScanOperator.java: PS11, Line 40: ; the method is the same as getVariables()? https://asterix-gerrit.ics.uci.edu/#/c/1727/11/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractUnnestMapOperator.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractUnnestMapOperator.java: PS11, Line 113: setPropagateIndexFilter(boolean propagateIndexFilter) setPropagateIndexFilter(boolean propagateIndexFilter) -> markPropagageIndexFilter(), since the argument is always true? PS11, Line 117: getPropagateIndexFilter getPropagateIndexFilter() -> propagateFilter() https://asterix-gerrit.ics.uci.edu/#/c/1727/11/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/rtree/AbstractRTreeOperatorTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/rtree/AbstractRTreeOperatorTest.java: PS11, Line 26: .*; expand imports -- To view, visit https://asterix-gerrit.ics.uci.edu/1727 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e2fe0208662e5dcd49d1a22bfb58f96533e9497 Gerrit-PatchSet: 11 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Luo Chen <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
