>From <[email protected]>: Attention is currently required from: Wail Alkowaileet. [email protected] has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364 )
Change subject: [ASTERIX-3109][COMP] Use of multiple array and column indexes.2 ...................................................................... Patch Set 11: Code-Review+1 (22 comments) Patchset: PS11: made some of the changes; please take a look. File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/array/AbstractOperatorFromSubplanRewrite.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/584ff3ab_0d67798f PS9, Line 159: //normalizedSelectCondition = keepOptimizableFunctions(normalizedSelectCondition).cloneExpression(); //ORIG > remove Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/0ffe900d_4278cfd8 PS9, Line 160: // MMK > remove Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/d017b1af_412eb00c PS9, Line 515: !expr.getExpressionTag().equals(LogicalExpressionTag.FUNCTION_CALL > LogicalExpressionTag.FUNCTION_CALL != expr. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/40b82cfe_c91697e8 PS9, Line 528: //return ifMissingOrNullFunction.getArguments().get(0).getValue().cloneExpression(); // ORIG > remove Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/8c8fde2d_cecb4f64 PS9, Line 529: // MMK > remove Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/4572d37a_dc6a6299 PS9, Line 542: /conjuncts.add(new MutableObject<>(conjunct.getValue().cloneExpression())); // ORIG > remove Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/91aa5fc3_1f552320 PS9, Line 543: // MMK > remove Done File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/array/JoinFromSubplanRewrite.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/2721e9c0_2254c441 PS9, Line 272: new HashSet<>(varsFromLeftBranch) > It would be better if we can declare a hash-set as a member and then reuse > it. […] This is not my code. When Glenn is back, we can ask him to take a look. I want to be very conservative at this point. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/da46e8f4_f28f6c92 PS9, Line 279: new HashSet<>(varsFromRightBranch).containsAll(usedVarsFromFunc) > Same as above This is not my code. When Glenn is back, we can ask him to take a look. I want to be very conservative at this point. File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/EnumerateJoinsRule.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/46e4659e_d3701aaf PS9, Line 152: printPlan(pp, (AbstractLogicalOperator) op, "Original Whole plan3"); > remove all printing Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/aaa3c38a_d6b942c5 PS9, Line 238: ILogicalOperator > make private Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/e6fb961f_5833b1cb PS9, Line 238: findSelectOrDataScan > Nothing changed here. […] I am looking for either operator. So the new is better. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/643a0b79_e6cbe64c PS9, Line 414: List<Mutable<ILogicalExpression>> conjs = new ArrayList<>(); > Make this as a member to the class (i.e., private final List<>... […] I like keeping things local wherever possible. If this is a problem, we can change it. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/13ca66cd_c0a26f50 PS9, Line 422: afce.removeAnnotation(SkipSecondaryIndexSearchExpressionAnnotation.class); // there may be some indexes here. : afce.putAnnotation(SkipSecondaryIndexSearchExpressionAnnotation.INSTANCE_ANY_INDEX); > I didn't get this. I changed the comment; see if that makes more sense. File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/JoinEnum.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/a1db026f_3aa48acc PS9, Line 747: //System.out.println(dumpJoinNodes(lastJnNum)); // helps with debugging locally; keep this line > remove Done File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/JoinNode.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/efe27580_da301bca PS9, Line 376: .getOperatorTag().equals(LogicalOperatorTag.SELECT > == Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/fc34573d_ac23b242 PS9, Line 382: : //selExprs.add(selOp.getCondition().getValue().cloneExpression()); > remove Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/b389f44f_3d8c627e PS9, Line 393: for (int i = 0; i < selExprs.size(); i++) > add braces […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/e678511f_d36d80f8 PS9, Line 398: ScalarFunctionCallExpression andExpr = new ScalarFunctionCallExpression( : BuiltinFunctions.getBuiltinFunctionInfo(AlgebricksBuiltinFunctions.AND)) > Move this below line 404 and remove the commented code there Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/6eaa7e63_cfb4854b PS9, Line 436: op = op.getInputs().get(0).getValue(); > In the second iteration op will be the same as selOp2 in the first iteration. > […] Not sure I follow. But the code is easier to follow this way for me. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364/comment/296df38d_db783dbc PS9, Line 446: while (combineDoubleSelectsBeforeSubPlans(leafInput)); > That is not needed if the method above is fixed I am handling the case there can be multiple selects (> 2) above the subplan. Then I collapse them incrementally. Easy to follow the changes for me. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17364 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: neo Gerrit-Change-Id: I2ad9a97a4de82e30aea89ac6a371d6a1b5c0ca87 Gerrit-Change-Number: 17364 Gerrit-PatchSet: 11 Gerrit-Owner: [email protected] Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-CC: Ali Alsuliman <[email protected]> Gerrit-Attention: Wail Alkowaileet <[email protected]> Gerrit-Comment-Date: Mon, 13 Feb 2023 20:01:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Wail Alkowaileet <[email protected]> Gerrit-MessageType: comment
