asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ian Maxon (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Full-text implementation step 3
Date Mon, 19 Dec 2016 23:02:13 GMT
Ian Maxon has posted comments on this change.

Change subject: Full-text implementation step 3
......................................................................


Patch Set 8:

(7 comments)

The only other thing I'd add is to be sure everyone's onboard with the modification to AQL.jj

https://asterix-gerrit.ics.uci.edu/#/c/1388/8/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java:

Line 308:                     }
This whole thing is a bit longer than it needs to be, the variable is only used once and you
could simply push the condition on the if down into that one use instead.


https://asterix-gerrit.ics.uci.edu/#/c/1388/8/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java:

Line 286:                     break;
Why's this moved down here?


https://asterix-gerrit.ics.uci.edu/#/c/1388/8/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/InvertedIndexAccessMethod.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/InvertedIndexAccessMethod.java:

Line 1062:         //We can only optimize contains with full-text indexes.
No need for if/else here.


Line 1086: 
Also no need for if/else here.


https://asterix-gerrit.ics.uci.edu/#/c/1388/8/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/InvertedIndexJobGenParams.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/InvertedIndexJobGenParams.java:

Line 113:         // Read full-text search information.
Would be nice to replace all the magic #'s here with static final variables.


https://asterix-gerrit.ics.uci.edu/#/c/1388/8/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/BinaryHashSet.java
File hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/BinaryHashSet.java:

Line 282:                 // Set the count as zero
This should be final if it just never gets set.


https://asterix-gerrit.ics.uci.edu/#/c/1388/8/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java:

Line 113: 
You could make this a for, couldn't you?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1087854ac7cf5b6ef5094e27a1646f12f6a8653f
Gerrit-PatchSet: 8
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <wangsaeu@yahoo.com>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-HasComments: Yes

Mime
View raw message