>From Wail Alkowaileet <[email protected]>: Attention is currently required from: Ali Alsuliman, Vijay Sarathy, Wail Alkowaileet, Till Westmann. Wail Alkowaileet has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313 )
Change subject: [ASTERIXDB-3098][COMP] Add cardinality and cost to the EXPLAIN STRING plan. ...................................................................... Patch Set 4: (7 comments) File asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access_closed/explain_field_access_closed.1.plan: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313/comment/d40187e0_9644c340 PS4, Line 39: 51 Hmm.. the change should not have any effect on the plans? What's the reason for the variable changes? File asterixdb/asterix-app/src/test/resources/runtimets/results/external-dataset/common/parquet/array-access-pushdown/array-access-pushdown.15.plan: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313/comment/256fc771_0f16320a PS4, Line 18: aggregate [$$42] <- [empty-stream()] We should also include the costs in a subplan. File asterixdb/asterix-app/src/test/resources/runtimets/results/external-dataset/common/parquet/field-access-pushdown/field-access-pushdown.13.plan: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313/comment/b90ce743_db17ef7b PS4, Line 1: cardinality: 1000000.0 Maybe not related to this change. But how some queries have costs/cardinalities as 0 but not the others? File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/AbstractLogicalOperatorPrettyPrintVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313/comment/49e9c97f_be7a8d6d PS4, Line 61: for (Map.Entry<String, Object> anno : op.getAnnotations().entrySet()) { : if (anno.getValue() != null && anno.getKey().equals(OperatorAnnotations.OP_OUTPUT_CARDINALITY)) { : cardinality = (double) anno.getValue(); : } else if (anno.getValue() != null && anno.getKey().equals(OperatorAnnotations.OP_COST_TOTAL)) { : cost = (double) anno.getValue(); : } : } We do not need to loop through the annotations. We can get the values for both OperatorAnnotations.OP_OUTPUT_CARDINALITY and OperatorAnnotations.OP_COST_TOTAL by doing something like: op.getAnnotations().get(OperatorAnnotations.OP_OUTPUT_CARDINALITY) See the comment below https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313/comment/ae276435_aac8c137 PS4, Line 77: HashMap<String, Double> We probably can break down this method and the one above. So that we can get card values and avoid creating a map for each operator. For example: protected Double getCardValue(ILogicalOperator op, String key) { Map<String, Object> annotations = op.getAnnotations(); if(annotations != null && annotations.containsKey(key)) { return annotations.get(key); } return null; } Then, to get a value (e.g., OperatorAnnotations.OP_OUTPUT_CARDINALITY): .... Double cardValue = getCardValue(op, OperatorAnnotations.OP_OUTPUT_CARDINALITY); .... File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313/comment/52a188ed_73c26841 PS4, Line 129: + Let's avoid string concatenation. For example: buffer.append("cardinality: "); buffer.append(cardCost.get("cardinality")); https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313/comment/464950fd_d71308b3 PS4, Line 137: cardinality Let's declare those constants as static final members in AbstractLogicalOperatorPrettyPrintVisitor so we don't have to repeat them. Also to unify the terminologies in both the JSON and STRING plans protected static final String CARDINALITY_KEY = "cardinality"; protected static final String OP_COST_KEY = "op-cost"; .... etc And call them in LogicalOperatorPrettyPrintVisitor and LogicalOperatorPrettyPrintVisitorJson -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17313 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: Ie697b90cd4396208ab8f59c68e20ced236903fb8 Gerrit-Change-Number: 17313 Gerrit-PatchSet: 4 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: Till Westmann <[email protected]> Gerrit-Reviewer: Vijay Sarathy <[email protected]> Gerrit-Reviewer: Wail Alkowaileet Gerrit-CC: Wail Alkowaileet <[email protected]> Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Attention: Vijay Sarathy <[email protected]> Gerrit-Attention: Wail Alkowaileet Gerrit-Attention: Till Westmann <[email protected]> Gerrit-Comment-Date: Thu, 19 Jan 2023 22:36:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
