Taewoo Kim has posted comments on this change. Change subject: Full-text implementation step 3 ......................................................................
Patch Set 8: (8 comments) @Ian: Thanks for the comments. 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 Done 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 196: private static void checkEachElementInFTSearchListPredicate(IACursor oListCursor, IAObject objectFromExpr) > MAJOR SonarQube violation: Done Line 286: break; > Why's this moved down here? I think the previous code has a bug. There is no difference between LENGTH vs SINGLE. They don't (can't generate any original field from the index. So, I think this is the right place. 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. Done Line 1086: > Also no need for if/else here. Done 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 Done 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. Done 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? Done -- 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-Reviewer: Taewoo Kim <[email protected]> Gerrit-HasComments: Yes
