>From Ali Alsuliman <[email protected]>:

Attention is currently required from: Vijay Sarathy.
Ali Alsuliman has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457 )

Change subject: [ASTERIXDB-3157][COMP] Spurious warnings about inapplicable 
hints.
......................................................................


Patch Set 2:

(6 comments)

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

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457/comment/a78efb07_864a5248
PS2, Line 191: void
make it private if it's only used here


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

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457/comment/dda67a0f_09e80a2b
PS2, Line 82: HashMap
Typically we use the interface type for variables definition and the specific 
implementation when creating the object for the variable. Let's make this 
Map<IExpressionAnnotation, Warning>


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

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457/comment/ac5fdd6a_a9ffb14b
PS2, Line 499: || hintHashJoin != null
is the meaning here that you want to add a plan node when there is a hint even 
if the cost is higher?


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457/comment/b1d60d61_3adabb33
PS2, Line 757: warningCollector.shouldWarn()
This "shouldWarn()" has the side effect of increasing the warning count. I 
don't think you want to increase the warning count just yet since you might 
decide later that this warning should not be reported. If that's the case, then 
remove the call to "shouldWarn()" and only have it at the place where you 
actually report these warnings.


https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457/comment/8b167e46_a22aefb1
PS2, Line 906: PlanNode findCheapestPlan()
make it private if it's only used here (or protected if to be used by 
subclasses)


File 
asterixdb/asterix-app/src/test/resources/optimizerts/results_cbo/joins/nlj_partitioning_property_1.plan:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457/comment/4f763267_eef374d1
PS2, Line 1: -- DISTRIBUTE_RESULT  |UNPARTITIONED|
Did you check that these plan changes are OK?



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17457
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: Id44519cbba2fd5eed8b9f85944f7a850ea4da3d3
Gerrit-Change-Number: 17457
Gerrit-PatchSet: 2
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: Vijay Sarathy <[email protected]>
Gerrit-Attention: Vijay Sarathy <[email protected]>
Gerrit-Comment-Date: Fri, 14 Apr 2023 09:57:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to