>From Ali Alsuliman <[email protected]>: Attention is currently required from: [email protected]. Ali Alsuliman has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383 )
Change subject: [ASTERIXDB-3555][COMP] Use Join Samples to get Join Selectivity ...................................................................... Patch Set 5: (9 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/Stats.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/a72d2974_79eadc14 PS5, Line 192: (idxDetails1.getSourceCardinality() < idxDetails1.getSampleCardinalityTarget() Given the numbers we saw that show the new way is mostly better, it feels like we should relax this comparison a bit. It will be unfortunate to use the naive way if the source cardinality is larger than the sample by just a couple of records. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/8b14f1c4_4ab772f3 PS5, Line 201: if (!(joinExpr.getFunctionIdentifier().equals(AlgebricksBuiltinFunctions.EQ))) This is not needed since it's already checked above. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/e1f049dc_f64ba038 PS5, Line 204: double sel = naiveJoinSelectivity(joinExpr, card1, card2, idx1, idx2); This can be just return naiveJoinSelectivity(joinExpr, card1, card2, idx1, idx2); https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/02b086aa_d4b9e462 PS5, Line 213: if (!(joinExpr.getFunctionIdentifier().equals(AlgebricksBuiltinFunctions.EQ))) { This is not needed since it's already checked above. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/8c688d78_e2826c1b PS5, Line 216: List<LogicalVariable> exprUsedVars = new ArrayList<>(); : joinExpr.getUsedVariables(exprUsedVars); This is already computed at the beginning of 'findJoinSelectivity()'. You can pass it from there to here and avoid re-computing it. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/717f342d_5c448649 PS5, Line 262: joinEnum.allJoinOps.get(0); I didn't follow why we always pick the first Join operator. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/e64b3796_2352b8d4 PS5, Line 265: abjoin.getInputs().get(0) Where do we switch back the input to the original? or am I missing something? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/14108274_3cd01407 PS5, Line 279: List<ILogicalExpression> ignore = This part can be removed: List<ILogicalExpression> ignore https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383/comment/4fbc1432_254559d9 PS5, Line 289: OperatorManipulationUtil.bottomUpCopyOperators I don't think we need re-copy the scanOp. Since you are copying the whole op at the beginning, the scanOp itself should be also a copy. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19383 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: I863dc378d5ead9f8b48f09f7bc400f5bdaba4839 Gerrit-Change-Number: 19383 Gerrit-PatchSet: 5 Gerrit-Owner: [email protected] Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-CC: Ali Alsuliman <[email protected]> Gerrit-Attention: [email protected] Gerrit-Comment-Date: Wed, 05 Feb 2025 17:37:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
