>From Ali Alsuliman <[email protected]>:

Attention is currently required from: Ali Alsuliman, Preetham Poluparthi, 
Rithwik Koul.

Ali Alsuliman has posted comments on this change by Rithwik Koul. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290?usp=email )

Change subject: [ASTERIXDB-3778][COMP] Drop constant ORDER BY sort keys
......................................................................


Patch Set 14:

(6 comments)

File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290/comment/88913fab_fbd0b244?usp=email
 :
PS14, Line 229:         normalization.add(new 
RemoveConstantOrderByExpressionsRule());
Let's run this rule only once after `EnforceStructuralPropertiesRule`


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290/comment/ed931d20_0a763fd3?usp=email
 :
PS14, Line 419:         physicalRewritesAllLevels.add(new 
RemoveConstantOrderByExpressionsRule());
Let's take this comment `// Strip sorts whose....` and put it inside 
`RemoveConstantOrderByExpressionsRule`.


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

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290/comment/1aadb484_1dd17583?usp=email
 :
PS14, Line 59:         AbstractLogicalOperator op = (AbstractLogicalOperator) 
opRef.getValue();
In order to determine if the order by operator is sitting between two 
exchanges, then you would need to check if `opRef` has inputs and its input at 
0 is `ORDER`. Then, the rest of the logic should be as is.

For now we should bail out if the `ORDER` operator is sitting between two 
exchanges.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290/comment/1367c366_08bd98c4?usp=email
 :
PS14, Line 86:             
opRef.setValue(orderOp.getInputs().get(0).getValue());
We probably need to call 
`context.computeAndSetTypeEnvironmentForOperator(parentOfOrderOperator)`


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290/comment/d90ff528_f5e361ac?usp=email
 :
PS14, Line 120:             cur = cur.getInputs().get(0).getValue();
See if you can adapt this to work on operators with multiple inputs (like a 
JOIN operator)


File 
asterixdb/asterix-app/src/test/resources/optimizerts/results/group-by/grouping-sets-empty-branch.plan:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290/comment/14fd3603_9bf5195b?usp=email
 :
PS14, Line 14:                       aggregate [] <- [] [cardinality: 0.0, 
doc-size: 0.0, op-cost: 0.0, total-cost: 0.0]
`[cardinality: 0.0, doc-size: 0.0, op-cost: 0.0, total-cost: 0.0]` for 
optimizer test, you can remove these.



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21290?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: lumina
Gerrit-Change-Id: I1156355d55b0f4cdde0044fb94e9c1b3ed15a3f2
Gerrit-Change-Number: 21290
Gerrit-PatchSet: 14
Gerrit-Owner: Rithwik Koul <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Preetham Poluparthi <[email protected]>
Gerrit-Reviewer: Rithwik Koul <[email protected]>
Gerrit-CC: Murtadha Hubail <[email protected]>
Gerrit-Attention: Rithwik Koul <[email protected]>
Gerrit-Attention: Ali Alsuliman <[email protected]>
Gerrit-Attention: Preetham Poluparthi <[email protected]>
Gerrit-Comment-Date: Mon, 01 Jun 2026 19:17:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Reply via email to