Taewoo Kim has posted comments on this change. Change subject: Fulltext search initial implementation ......................................................................
Patch Set 14: (84 comments) https://asterix-gerrit.ics.uci.edu/#/c/989/14/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 359: private boolean isMatched(IAType type1, IAType type2, boolean useListDomain, boolean isFullTextSearchCase) > MAJOR SonarQube violation: Done Line 705: return null; > MAJOR SonarQube violation: Done Line 751: return null; > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/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 145: && arg2 instanceof ConstantExpression) { > Since "instanceof" could cause a performance issues, can this be replaced w Done Line 159: OptimizableFuncExpr newOptFuncExpr = null; > MAJOR SonarQube violation: Done Line 210: "Currently, a phrase search in Full-text is not supported. Each expression should include only one keyword." > MAJOR SonarQube violation: Done Line 220: String arg2Value = null; > MAJOR SonarQube violation: Done Line 221: AOrderedList aOList = null; > MAJOR SonarQube violation: Done Line 222: AUnorderedList aUnOList = null; > MAJOR SonarQube violation: Done Line 224: if (objectFromExpr instanceof AString) { > See my comment above... Done Line 227: } else if (objectFromExpr instanceof AOrderedList) { > See my comment above... Done Line 232: if (objectFromExpr instanceof AString) { > See my comment above... Done Line 239: } else if (objectFromExpr instanceof AUnorderedList) { > See my comment above... We don't have AUnorderedList type. So, I think this may be the only option. Line 244: if (objectFromExpr instanceof AString) { > See my comment above... Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/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 1242: case DISJUNCTIVE: { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/AqlExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/AqlExpressionToPlanTranslator.java: Line 219: Mutable<ILogicalOperator> topOp = tupSource; > MAJOR SonarQube violation: Done Line 228: Pair<ILogicalExpression, Mutable<ILogicalOperator>> p2 = null; > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/AqlPlusExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/AqlPlusExpressionToPlanTranslator.java: Line 712: return new Pair<ILogicalOperator, LogicalVariable>(a, assignedVar); > MAJOR SonarQube violation: Done Line 790: return new Pair<ILogicalOperator, LogicalVariable>(a, assignedVar); > MAJOR SonarQube violation: Done Line 1066: case CONTAINS: { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java: Line 955: Mutable<ILogicalOperator> topOp = tupSource; > MAJOR SonarQube violation: Done Line 1032: return new Pair<Mutable<ILogicalOperator>, ILogicalExpression>(topOp, currExpr); > MAJOR SonarQube violation: Done Line 1262: case CONTAINS: { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/expression/FTContainsExpr.java File asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/expression/FTContainsExpr.java: Line 37: public class FTContainsExpr extends OperatorExpr { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/visitor/AQLCloneAndSubstituteVariablesVisitor.java File asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/visitor/AQLCloneAndSubstituteVariablesVisitor.java: Line 107: List<Expression> exprs = new ArrayList<Expression>(oldExprList.size()); > MAJOR SonarQube violation: Done Line 114: return new Pair<ILangExpression, VariableSubstitutionEnvironment>(oe, env); > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj File asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj: Line 1741: op.addOperand(operand); > indent Done Line 1742: op.setCurrentop(true); > indent Done Line 1744: try{ > missing space Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java: Line 249: public final static FunctionIdentifier STRING_CONTAINS = new FunctionIdentifier(FunctionConstants.ASTERIX_NS, > revert static final -> final static Done Line 251: public final static FunctionIdentifier STRING_STARTS_WITH = new FunctionIdentifier(FunctionConstants.ASTERIX_NS, > revert static final -> final static Done Line 429: public final static FunctionIdentifier EDIT_DISTANCE_STRING_CONTAINS = new FunctionIdentifier(FunctionConstants.ASTERIX_NS, > MAJOR SonarQube violation: Done Line 429: public final static FunctionIdentifier EDIT_DISTANCE_STRING_CONTAINS = new FunctionIdentifier(FunctionConstants.ASTERIX_NS, > revert static final -> final static Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/util/AsterixConstantExpressionUtil.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/util/AsterixConstantExpressionUtil.java: Line 37: public class AsterixConstantExpressionUtil { > MAJOR SonarQube violation: Done Line 51: if (b) { > inline? Done Line 69: if (obj instanceof AString) { > Suggest using obj.getType() and typeTag instead Done Line 77: if (obj instanceof AOrderedList) { > Suggest using obj.getType() and typeTag instead Done Line 85: if (obj instanceof AUnorderedList) { > Suggest using obj.getType() and typeTag instead Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/ComparisonHelper.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/ComparisonHelper.java: Line 91: private final IBinaryComparator strLowerCaseTokenCmp = AqlBinaryComparatorFactoryProvider.UTF8STRING_LOWERCASE_TOKEN_POINTABLE_INSTANCE > CRITICAL SonarQube violation: Done Line 93: private IBinaryTokenizer tokenizerForLeftArray = null; > CRITICAL SonarQube violation: Done Line 94: private IBinaryTokenizer tokenizerForRightArray = null; > CRITICAL SonarQube violation: Done Line 97: private IBinaryHashFunction hashFunc = null; > CRITICAL SonarQube violation: Done Line 100: private BinaryEntry keyEntry = null; > CRITICAL SonarQube violation: Done Line 104: private BinaryHashSet rightHashSet = null; > CRITICAL SonarQube violation: Done Line 550: int numBytesToStoreLength = 0; > MAJOR SonarQube violation: Done Line 663: default: > MAJOR SonarQube violation: Done Line 700: "Currently, a phrase search in Full-text is not supported. Each expression should include only one keyword."); > MAJOR SonarQube violation: Done Line 704: case UNORDEREDLIST: > Considering splitting this into different cases for future work? We could ( Sure. Those two cases should differ. But, here, we just check whether t1, t2, or t3 consists of a phrase or not. And this method will be removed when we will support a phrase search. Line 708: "Currently, a phrase search in Full-text is not supported. Each expression should include only one keyword." > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/FullTextContainsDescriptor.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/FullTextContainsDescriptor.java: Line 35: public static final IFunctionDescriptorFactory FACTORY = new IFunctionDescriptorFactory() { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/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 60: private final List<ByteBuffer> frames = new ArrayList<ByteBuffer>(); > MAJOR SonarQube violation: Done Line 70: public int off; > MAJOR SonarQube violation: Done Line 71: public int len; > MAJOR SonarQube violation: Done Line 135: int bucket = 0; > MAJOR SonarQube violation: Done Line 157: int frameNum = 0; > MAJOR SonarQube violation: Done Line 158: int frameOff = 0; > MAJOR SonarQube violation: Done Line 159: int entryKeyOff = 0; > MAJOR SonarQube violation: Done Line 160: int entryKeyLen = 0; > MAJOR SonarQube violation: Done Line 161: int entryCount = 0; > MAJOR SonarQube violation: Done Line 237: // [2 byte for key offset] [2 byte for key length] [1 byte for key count] [2 byte for the frame num] [2 byte for the frame offset] > MAJOR SonarQube violation: Done Line 298: return hasNextElement(false); > hasNext() should be a no-op if next() has not been called since the last ha Done Line 362: return val; > This seems scary- if we call next() without calling hasNext(), we'll get th Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java: Line 459: return hasNextEntry(false); > hasNext() should be a no-op if next() has not been called since the last ha Done Line 582: private void clearCount() { > MAJOR SonarQube violation: Done Line 587: public TuplePointer next() { > MAJOR SonarQube violation: Done Line 588: return currentTuplePointer; > same comment as in other iterator- seems scary to not guard against forgett Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/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 106: // if (isFullTextSearchQuery && queryFieldIndexesLength > FTContainsParams.getNumParams()) { > MAJOR SonarQube violation: Done Line 133: "Currently, a phrase search in Full-text is not supported. Each expression should include only one keyword."); > MAJOR SonarQube violation: Done Line 139: "Currently, a phrase search in Full-text is not supported. Each expression should include only one keyword."); > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/FTContainsParams.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/FTContainsParams.java: Line 35: public static String disjunctiveFTSearchOption = "any"; > MAJOR SonarQube violation: Done Line 35: public static String disjunctiveFTSearchOption = "any"; > CRITICAL SonarQube violation: Done Line 36: public static String conjunctiveFTSearchOption = "all"; > MAJOR SonarQube violation: Done Line 36: public static String conjunctiveFTSearchOption = "all"; > CRITICAL SonarQube violation: Done Line 39: public static byte[] disjunctiveFTSearchOptionArray = UTF8StringUtil.writeStringToBytes(disjunctiveFTSearchOption); > CRITICAL SonarQube violation: Done Line 39: public static byte[] disjunctiveFTSearchOptionArray = UTF8StringUtil.writeStringToBytes(disjunctiveFTSearchOption); > CRITICAL SonarQube violation: Done Line 39: public static byte[] disjunctiveFTSearchOptionArray = UTF8StringUtil.writeStringToBytes(disjunctiveFTSearchOption); > MAJOR SonarQube violation: Done Line 40: public static byte[] conjunctiveFTSearchOptionArray = UTF8StringUtil.writeStringToBytes(conjunctiveFTSearchOption); > CRITICAL SonarQube violation: Done Line 40: public static byte[] conjunctiveFTSearchOptionArray = UTF8StringUtil.writeStringToBytes(conjunctiveFTSearchOption); > MAJOR SonarQube violation: Done Line 40: public static byte[] conjunctiveFTSearchOptionArray = UTF8StringUtil.writeStringToBytes(conjunctiveFTSearchOption); > CRITICAL SonarQube violation: Done Line 45: public FTContainsParams() { > MAJOR SonarQube violation: Done Line 45: public FTContainsParams() { > MAJOR SonarQube violation: Done Line 54: return 1; > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/989/14/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/tokenizers/TokenizerInfo.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/tokenizers/TokenizerInfo.java: Line 27: public static enum TokenizerType { > MAJOR SonarQube violation: Done Line 32: public TokenizerInfo() { > MAJOR SonarQube violation: Done -- To view, visit https://asterix-gerrit.ics.uci.edu/989 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I71887c2ea847e4488f4c98a11f8a5bcad02cac5a Gerrit-PatchSet: 14 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <[email protected]> Gerrit-Reviewer: Heri Ramampiaro <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
