Taewoo Kim has posted comments on this change.

Change subject: [ASTERIXDB-1984][COMP] probe-subtree init not required
......................................................................


Patch Set 17:

(6 comments)

Thanks Dmitry for the initial comments.

https://asterix-gerrit.ics.uci.edu/#/c/1880/17/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceJoinAccessMethodRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceJoinAccessMethodRule.java:

Line 328:                         e.printStackTrace();
> Same. Why exception is caught and swallowed here?
Yingyi suggested that a lambda expression is more understandable for this 
place. I modified the function. However, for the exception, other than choosing 
this, "throw an exception" didn't help. Could you give me some hint to modify 
this?


Line 335:                         e.printStackTrace();
> Same. Why exception is caught and swallowed here?
Yingyi suggested that a lambda expression is more understandable for this 
place. I modified the function. However, for the exception, other than choosing 
this, "throw an exception" didn't help. Could you give me some hint to modify 
this?


Line 413:         boolean leftSubTreeInitialized = 
leftSubTree.initFromSubTree(joinOp.getInputs().get(0), false);
> leftSubTreeInitialized is not needed and should probably be removed because
At least we need to set the root operator of the subtree. So, that 
initialization is required.


Line 421:         if (leftSubTree.hasDataSourceScan() || 
rightSubTree.hasDataSourceScan()) {
> Can we remove leftSubTree.hasDataSourceScan() , because The left subtree wa
That's correct. Modified.


https://asterix-gerrit.ics.uci.edu/#/c/1880/17/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceSelectAccessMethodRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceSelectAccessMethodRule.java:

Line 343:                     } catch (Exception e) {
> What kind of exception is caught here and why is it swallowed? It didn't do
Same for the join case. I tried to change it to a lambda expression and "throw 
an exception" didn't work here. Could you give me some hint?


https://asterix-gerrit.ics.uci.edu/#/c/1880/17/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java:

Line 103:             return true;
> Should this return 'false' because the subtree was not fully initialized? R
Makes sense. Modified.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1880
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib353c85bf627d8dd65dba0ea307dee428edb4a25
Gerrit-PatchSet: 17
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <wangs...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <wangs...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to