>From Ali Alsuliman <[email protected]>: Attention is currently required from: Janhavi Tripurwar, Murtadha Hubail, Peeyush Gupta.
Ali Alsuliman has posted comments on this change by Janhavi Tripurwar. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441?usp=email ) Change subject: [ASTERIXDB-3665]: Compute RangeMap at Compile time ...................................................................... Patch Set 7: (9 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/JoinEnum.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/9096b725_d4d98827?usp=email : PS7, Line 1051: attachStaticRangeMapIfNeeded(); What if CBO is disabled at the query level but we do have samples? (i.e. this is the case where samples have been collected, but the user would like not to use CBO). https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/81f21076_5cae0119?usp=email : PS7, Line 1072: for (Pair<OrderOperator.IOrder, Mutable<ILogicalExpression>> p : rootOrd.getOrderExpressions()) { This is the same loop as the above. Can we consolidate them into one? File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/Stats.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/7e9669a0_965c352f?usp=email : PS7, Line 932: RangeMapUtil rangeMapUtil = new RangeMapUtil(BuiltinFunctions.RANGE_MAP, BuiltinFunctions.LOCAL_SAMPLING, Those could be used directly from inside the RangeMapUtil class. Then, we wouldn't need to instantiate an object (typically util classes provide static methods, i.e. they are stateless), and we would use all static methods. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/892dc0db_4b2b9fb0?usp=email : PS7, Line 938: ILogicalOperator parent = joinEnum.findDataSourceScanOperatorParent(newRoot); What if we have a join? don't we need to replace both of the data-scans? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/9728ea0a_baa85dcb?usp=email : PS7, Line 948: List<LogicalVariable> outVars = new ArrayList<>(sortExprs.size()); Why not only use partitionFields? partitionFields should be enough, right? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/792fb0a0_2c06f48d?usp=email : PS7, Line 994: private RangeMap deserializeRangeMap(List<List<IAObject>> val) throws AlgebricksException { We should probably have this logic (both the serialization and deserialization) as part of the RangeMapUtil. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/1d1dd069_e3ce8f05?usp=email : PS7, Line 996: ABinary ab = val.get(0).get(0) instanceof ANull ? null : ((ABinary) val.get(0).get(0)); Is this ever going to be null? if yes, then it's going to cause NPE. You might want to do it this way: if (val.get(0).get(0).getType().getTypeTag() != ATypeTag.BINARY) { return null; } ABinary ab = ((ABinary) val.get(0).get(0)); File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractStableSortPOperator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/a7de73c0_4e660075?usp=email : PS7, Line 162: private String getFullParallelAnnotation(AbstractLogicalOperator sortOp, INodeDomain clusterDomain, We could simplify this now, right? we could just remove this method and instead use getRangeMap() directly, correct? File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/RangeMapUtil.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/e3f63959_ce1ad83e?usp=email : PS7, Line 122: boolean isTwoStep = buildRangeMapAtCompileTime ? false : true; Remind me to ask you about this flag. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: phoenix Gerrit-Change-Id: I51f99463d24596e74b5d435f718236de9016f2e8 Gerrit-Change-Number: 20441 Gerrit-PatchSet: 7 Gerrit-Owner: Janhavi Tripurwar <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-CC: Murtadha Hubail <[email protected]> Gerrit-CC: Peeyush Gupta <[email protected]> Gerrit-Attention: Murtadha Hubail <[email protected]> Gerrit-Attention: Peeyush Gupta <[email protected]> Gerrit-Attention: Janhavi Tripurwar <[email protected]> Gerrit-Comment-Date: Wed, 05 Nov 2025 01:38:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
