>From Vijay Sarathy <[email protected]>: Attention is currently required from: Ali Alsuliman, Till Westmann. Vijay Sarathy has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299 )
Change subject: [ASTERIXDB-3093][COMP] CBO enhancements and fixes after calibration tests. ...................................................................... Patch Set 6: (10 comments) Commit Message: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/ed9cb7b0_b9465ae9 PS4, Line 7: ASTERIXDB-XXXX > Either use [NO ISSUE] (instead of [ASTERIXDB-XXXX]) or file the issue with a > list of enhancements an […] Yes, I used XXXX only as a place holder. I will file an issue for this and update the commit message. I have created https://issues.apache.org/jira/browse/ASTERIXDB-3093 for this File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/cost/CostMethods.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/0e3cdebc_98f06c65 PS4, Line 40: /* 32 MB */ * 1024 / 8; // 4 GB; > I'm not sure I followed this part. Reverted to the default getMaxMemorySize() File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/EnumerateJoinsRule.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/00cfb7a7_2318e54a PS4, Line 232: AssignOperator aOp = (AssignOperator) nextOp; : List<LogicalVariable> vars = new ArrayList<>(); : aOp.getExpressions().get(0).getValue().getUsedVariables(vars); : //if (vars.size() > 1) // This is to avoid the case where the vars do not belong to the same leaf Input. This is too restrictive but safe : //return false; > I don't see this is being used. Removed unused code. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/b30bc59e_8b99f439 PS4, Line 242: LogicalOperatorTag tag; : while (true) { : if (op.getInputs().size() > 1) : return null; // Assuming only a linear plan for single table queries (as leafInputs are linear). : tag = op.getOperatorTag(); : if (tag == LogicalOperatorTag.EMPTYTUPLESOURCE) : return null; // if this happens, there is nothing we can do in CBO code since there is no datasourcescan : //if ((tag == LogicalOperatorTag.SELECT) || (tag == LogicalOperatorTag.DATASOURCESCAN)) { : if (tag == LogicalOperatorTag.SELECT) { // there must be a select operator for CBO to do any optimization. : return op; : } : : op = op.getInputs().get(0).getValue(); : } > can we clean this up? remove commented out code (and probably rename the > method since it's only look […] Changed method name to findSelect(), removed commented out code, and enclosed if blocks in {} https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/72cffa62_c44c7f37 PS4, Line 294: findSelectOrDataScan > we just called findSelectOrDataScan() up above. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/c1e362e1_9b13144b PS4, Line 388: i++; > why do we have another "i++" at the bottom of the loop as well? Removed the bottom i++ https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/e3921e1e_fa25f349 PS4, Line 409: root = aOp; > not sure why we need this assignment (and the root variable), but we could > just return aOp Removed root and simply returning aOp https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/e795525f_d303fc76 PS4, Line 507: if (internalEdges.size() == 0) : return op; > enclose in {} Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/e9aa5e92_4db31cd0 PS4, Line 634: HashSet<LogicalVariable> vars = new HashSet<>(); > take this line out before the for-loop and clear the set here for re-use. […] Removed unused code involving vars File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299/comment/1632543e_e0e053ed PS4, Line 3674: if (hintToken != null) { : SourceLocation sourceLoc = getSourceLocation(hintToken); : hintCollector.remove(sourceLoc); : } > I think what should have been done is to have another fetchHints() that > returns a list of hintToken […] Talked with Ali, will do it in a follow-up change. Will file an issue titled “improve logic for collecting multiple hints” -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17299 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: Ifa7da177ab94ba39ee2d3e24813eced48e3b8304 Gerrit-Change-Number: 17299 Gerrit-PatchSet: 6 Gerrit-Owner: Vijay Sarathy <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Vijay Sarathy <[email protected]> Gerrit-CC: Till Westmann <[email protected]> Gerrit-CC: Till Westmann <[email protected]> Gerrit-CC: [email protected] Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Attention: Till Westmann <[email protected]> Gerrit-Comment-Date: Mon, 05 Dec 2022 22:31:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Till Westmann <[email protected]> Comment-In-Reply-To: Ali Alsuliman <[email protected]> Gerrit-MessageType: comment
