>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 31:

(10 comments)

File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/AttachCompileTimeRangeMapRule.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/8666c25a_9240dbdc?usp=email
 :
PS31, Line 77:         if (orderOp.getOperatorTag() == LogicalOperatorTag.ORDER
It is cleaner if you do all of this like:
if (parallelSortNotApplicable()) {
  return false;
}
RangeMapUtil.attachRangeMapIfNeeded(orderOp, context);
return true;

parallelSortNotApplicable(orderOp) {
if (orderOp.getOperatorTag() != LogicalOperatorTag.ORDER
|| orderOp.getAnnotations().containsKey(OperatorAnnotations.USE_STATIC_RANGE)
|| hasIntervalMergeJoin(orderOp)
|| !shouldComputeRangeMap(orderOp, context)
|| hasLimitRunningAggregate()) {
  return true;
}
return false;
}


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/5f241db3_facb5ce3?usp=email
 :
PS31, Line 120:         if 
(orderOp.getAnnotations().containsKey(OperatorAnnotations.USE_STATIC_RANGE)
This must be always false because we only get here when there is no 
USE_STATIC_RANGE. So, this condition can be removed.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/85d66278_56f42039?usp=email
 :
PS31, Line 139:         if (scanOps.isEmpty()) {
Move this check up right after calling collectDataSourceScanOperators()


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/52499a30_a3c312fe?usp=email
 :
PS31, Line 146:         while (op != null && (op.getOperatorTag() == 
LogicalOperatorTag.EXCHANGE
Not sure how this while-loop can determine if it is "single tuple" or not.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/e5b12ce9_b0d92b52?usp=email
 :
PS31, Line 156:             return ((AggregateOperator) op).isGlobal();
Are you sure this is correct meaning isGlobal() indicates a single tuple output?


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/784ff1d2_5170e9bf?usp=email
 :
PS31, Line 178:     private static boolean 
hasIntervalMergeJoin(ILogicalOperator op) {
Add {} in the if-blocks.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/f8a4c90a_d3fd748b?usp=email
 :
PS31, Line 183:             if (pop != null && 
pop.getOperatorTag().toString().contains("INTERVAL_MERGE_JOIN")) {
Why toString()? Why not compare the enum directly?


File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/OperatorUtils.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/ab1aad67_008f371f?usp=email
 :
PS31, Line 410:             return null;
Add {}


File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/RangeMapUtil.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/4396e91e_6aba03fc?usp=email
 :
PS31, Line 270:         String plan = new ALogicalPlanImpl(topRef).toString();
Why is this here? This toString() is expensive.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20441/comment/fef1fd0a_f4ce65ac?usp=email
 :
PS31, Line 280:             DataSourceScanOperator scan;
DataSourceScanOperator scan = (DataSourceScanOperator) op;



--
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: master
Gerrit-Change-Id: I51f99463d24596e74b5d435f718236de9016f2e8
Gerrit-Change-Number: 20441
Gerrit-PatchSet: 31
Gerrit-Owner: Janhavi Tripurwar <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Janhavi Tripurwar <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Peeyush Gupta <[email protected]>
Gerrit-CC: Murtadha Hubail <[email protected]>
Gerrit-Attention: Murtadha Hubail <[email protected]>
Gerrit-Attention: Peeyush Gupta <[email protected]>
Gerrit-Attention: Janhavi Tripurwar <[email protected]>
Gerrit-Comment-Date: Thu, 11 Dec 2025 04:31:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Reply via email to