Change in asterixdb[master]: [ASTERIXDB-3157][COMP] Spurious warnings about inapplicable hints.

[AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>]

From Ali Alsuliman <al...@gmail.com>:

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 <vi...@couchbase.com>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Vijay Sarathy <vi...@couchbase.com>
Gerrit-Attention: Vijay Sarathy <vi...@couchbase.com>
Gerrit-Comment-Date: Fri, 14 Apr 2023 09:57:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment