Ali Alsuliman has posted comments on this change. Change subject: [ASTERIXDB-2286][COMP][FUN][HYR] Parallel Sort Optimization ......................................................................
Patch Set 32: (5 comments) https://asterix-gerrit.ics.uci.edu/#/c/2393/31/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java: Line 342: final PhysicalOptimizationConfig physOptConf = new PhysicalOptimizationConfig(); > OptimizationConfUtil class is no longer used after this change, right? Let' Sure we can remove it later. I know 2 other places are using it. InvertedIndex search and during index creation https://asterix-gerrit.ics.uci.edu/#/c/2393/31/asterixdb/asterix-app/src/test/resources/optimizerts/results/btree-index-join/secondary-self-equi-join-index-only.plan File asterixdb/asterix-app/src/test/resources/optimizerts/results/btree-index-join/secondary-self-equi-join-index-only.plan: Line 26: -- ONE_TO_ONE_EXCHANGE |PARTITIONED| > why ASSIGN and STREAM_PROJECT were eliminated? Good question. This is because inlining of variables was not happening in index only plans. In the old plan: data-scan $1, $rec proj($rec) assing $2 = $rec.getField() proj($2) assign $3 = add($2, 20) proj($3) broadcast ..... The new plan is: data-scan $1, $rec proj($rec) assign $3 = add($rec.getField(), 20) proj($3) broadcast ..... This is fixed by InlineVariablesRule. I had to fix it since this was happening for plans with replicate or split operators, and parallel sort uses replicate. https://asterix-gerrit.ics.uci.edu/#/c/2393/31/asterixdb/asterix-app/src/test/resources/optimizerts/results/btree-index-join/sidx-non-idxonly-to-sidx-idxonly-equi-join_01.plan File asterixdb/asterix-app/src/test/resources/optimizerts/results/btree-index-join/sidx-non-idxonly-to-sidx-idxonly-equi-join_01.plan: Line 27: -- ASSIGN |PARTITIONED| > why stream_project operators were eliminated? Same thing as the other plan. The variable (field from a record) was inlined in the SELECT (it's $23) Old plan is: index $20, $rec assign $18=$rec.getField(1), $23 = $rec.getField(5) project([$$20, $$18, $$23]) select $23, $18 project ([$$20, $$18]) New plan is: index $20, $rec assign $18=$rec.getField(1), select $rec.getField(5), $18 project ([$$20, $$18]) https://asterix-gerrit.ics.uci.edu/#/c/2393/31/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/SequentialMergeExchangePOperator.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/SequentialMergeExchangePOperator.java: Line 51: return emptyUnaryRequirements(); > I think this needs more thought (in a follow up change). getRequiredPropert I agree. There were some assumptions at the time we came up with for some corner cases. Yes, we should look at it again. Line 57: AbstractLogicalOperator childOp = (AbstractLogicalOperator) op.getInputs().get(0).getValue(); > This method also needs more thought (in a follow up change). 1) Need to an Same reply. -- To view, visit https://asterix-gerrit.ics.uci.edu/2393 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I73e128029a46f45e6b68c23dfb9310d5de10582f Gerrit-PatchSet: 32 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
