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 <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-HasComments: Yes
