Taewoo Kim has posted comments on this change. Change subject: [ASTERIXDB-1972][COMP][RT][TX] index-only plan ......................................................................
Patch Set 13: (104 comments) https://asterix-gerrit.ics.uci.edu/#/c/1866/1/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 883: return getFieldNameFromSubTree(optFuncExpr, subTree, assignOrUnnestIndex, varIndex, recordType, > MAJOR SonarQube violation: Done Line 1047: } > BLOCKER SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/1/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 425: return Collections.emptyList(); > MAJOR SonarQube violation: Done Line 483: IOptimizableFuncExpr optFuncExpr, IAType indexedFieldType, OptimizableOperatorSubTree probeSubTree) > MAJOR SonarQube violation: Done Line 578: break; > MAJOR SonarQube violation: Done Line 984: varAlreadyAdded = findVarInTripleVarList(unionVarMap, tVar, true); > MAJOR SonarQube violation: Done Line 1859: > MAJOR SonarQube violation: Done Line 2313: // First, check whether the given old variable can be deleted. If it is used somewhere else > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/4/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 1606: > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/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 582: default: > MAJOR SonarQube violation: Done Line 582: default: > MAJOR SonarQube violation: Done Line 595: } catch (HyracksDataException e) { > CRITICAL SonarQube violation: Done Line 595: } catch (HyracksDataException e) { > CRITICAL SonarQube violation: Done Line 595: } catch (HyracksDataException e) { > fix this? Done Line 608: } catch (HyracksDataException e) { > CRITICAL SonarQube violation: Done Line 608: } catch (HyracksDataException e) { > Fix this? Done Line 608: } catch (HyracksDataException e) { > CRITICAL SonarQube violation: Done PS13, Line 750: // Are we transforming a join plan? > considering keeping the old method and create a new one for index only? Done PS13, Line 752: topOpRef.getValue().getOperatorTag() != LogicalOperatorTag.SELECT > is there any chance to make the code agnostic to SELECT? Done. Line 857: case RECTANGLE: > MAJOR SonarQube violation: Done Line 857: case RECTANGLE: > MAJOR SonarQube violation: Done PS13, Line 939: // If there are ASSIGN operators before the given SELECT or JOIN operator, we need to propagate : // these variables to the UNIONALL operator, too. > update the comment to be based on variable usage/liveness. Done Line 947: for (int j = 0; j < usedVarsInAssignUnnestBeforeTopOpTmp.size(); j++) { > MAJOR SonarQube violation: Done Line 947: for (int j = 0; j < usedVarsInAssignUnnestBeforeTopOpTmp.size(); j++) { > MAJOR SonarQube violation: Done Line 967: if (!usedVarsAfterTopOp.contains(tmpVar)) { > MAJOR SonarQube violation: Done Line 967: if (!usedVarsAfterTopOp.contains(tmpVar)) { > MAJOR SonarQube violation: Done PS13, Line 962: if (afterTopOpRefs != null) { : for (Mutable<ILogicalOperator> afterTopOpRef : afterTopOpRefs) { : tmpVars.clear(); : VariableUtilities.getUsedVariables((ILogicalOperator) afterTopOpRef.getValue(), tmpVars); : for (LogicalVariable tmpVar : tmpVars) { : if (!usedVarsAfterTopOp.contains(tmpVar)) { : usedVarsAfterTopOp.add(tmpVar); : } : } : } : } > OperatorPropertiesUtils.getFreeVaraibles(....) Done PS13, Line 977: : // Is the used variables after SELECT operator from the primary index? : > UnionAll need to generate new variable, so that the query plan can stay as Done Line 1020: if (sIndexIdx > -1) { > MAJOR SonarQube violation: Done Line 1020: if (sIndexIdx > -1) { > MAJOR SonarQube violation: Done Line 1041: if (sIndexIdx > -1) { > MAJOR SonarQube violation: Done Line 1041: if (sIndexIdx > -1) { > MAJOR SonarQube violation: Done Line 1043: if (isIndexOnlyPlan && (secondaryKeyFieldUsedAfterSelectOrJoinOp > MAJOR SonarQube violation: Done Line 1043: if (isIndexOnlyPlan && (secondaryKeyFieldUsedAfterSelectOrJoinOp > MAJOR SonarQube violation: Done PS13, Line 1077: unionVarMap.add(new Triple<>(v, v, v)); > make v/v/v as SSA? Done Line 1112: if (expr.getValue().getExpressionTag() == LogicalExpressionTag.CONSTANT) { > MAJOR SonarQube violation: Done Line 1112: if (expr.getValue().getExpressionTag() == LogicalExpressionTag.CONSTANT) { > MAJOR SonarQube violation: Done Line 1120: if (constAssignVars.size() > 0) { > MAJOR SonarQube violation: Done Line 1120: if (constAssignVars.size() > 0) { > MAJOR SonarQube violation: Done Line 1278: if ((idxType == IndexType.RTREE && requireVerificationAfterSIdxSearch) > MAJOR SonarQube violation: Done Line 1278: if ((idxType == IndexType.RTREE && requireVerificationAfterSIdxSearch) > MAJOR SonarQube violation: Done Line 1302: for (int i = 0; i < uniqueUsedVarsInTopOp.size(); i++) { > MAJOR SonarQube violation: Done Line 1302: for (int i = 0; i < uniqueUsedVarsInTopOp.size(); i++) { > MAJOR SonarQube violation: Done Line 1312: if (fetchedSecondaryKeyFieldVarsFromPIdxLookUp != null) { > MAJOR SonarQube violation: Done Line 1312: if (fetchedSecondaryKeyFieldVarsFromPIdxLookUp != null) { > MAJOR SonarQube violation: Done PS13, Line 1376: else { : // Non-index-only plan optimization - still index-utilization plan. : return primaryIndexUnnestOp; : } > Pull that into a separate if block and then the current if branch doesn't n Done https://asterix-gerrit.ics.uci.edu/#/c/1866/4/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java: Line 184: } else { > MAJOR SonarQube violation: Done Line 185: // We don't consider an index on an external dataset or a primary index to be an index-only plan. > MAJOR SonarQube violation: Done Line 339: > MAJOR SonarQube violation: Done Line 340: ILogicalOperator primaryIndexUnnestOp = createSecondaryToPrimaryPlan(afterJoinRefs, joinRef, conditionRef, > MAJOR SonarQube violation: Done Line 341: indexSubTree.getAssignsAndUnnestsRefs(), indexSubTree, probeSubTree, chosenIndex, analysisCtx, true, > MAJOR SonarQube violation: Done Line 342: isLeftOuterJoin, true, context, newNullPlaceHolderVar); > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java: Line 95: private static List<Pair<FunctionIdentifier, Boolean>> FUNC_IDENTIFIERS = Collections.unmodifiableList( > final? Done PS13, Line 155: secondaryKeyFieldUsedAfterSelectOp > this variable is not needed for B-tree? Done PS13, Line 218: || dataSourceRefOp.getOperatorTag() == LogicalOperatorTag.LEFT_OUTER_UNNEST_MAP > Is the or branch covered by any tests? Removed. PS13, Line 232: // The whole plan is now changed into an index-only plan > you only use primary index for this branch, why it is related to index only Removed. PS13, Line 240: dataSourceRefOp.getOperatorTag() == LogicalOperatorTag.LEFT_OUTER_UNNEST_MAP > same as above Removed. PS13, Line 305: boolean secondaryKeyFieldUsedAfterJoinOp = false; > Is this variable necessary? Removed. PS13, Line 349: IsNull > IsNull -> isMissing? Done PS13, Line 391: primaryIndexUnnestOp > rename the variable? I changed the name. I just found that this is not an assignment so that I can't make this into one line. https://asterix-gerrit.ics.uci.edu/#/c/1866/13/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: PS13, Line 384: intersectAllSecondaryIndexes > factor out single index handling as another method and call the new method Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/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 117: private static List<Pair<FunctionIdentifier, Boolean>> FUNC_IDENTIFIERS = Collections.unmodifiableList( > MAJOR SonarQube violation: Done Line 117: private static List<Pair<FunctionIdentifier, Boolean>> FUNC_IDENTIFIERS = Collections.unmodifiableList( > MAJOR SonarQube violation: Done Line 129: private static List<Pair<FunctionIdentifier, Boolean>> SECOND_LEVEL_FUNC_IDENTIFIERS = Collections.unmodifiableList( > MAJOR SonarQube violation: Done Line 129: private static List<Pair<FunctionIdentifier, Boolean>> SECOND_LEVEL_FUNC_IDENTIFIERS = Collections.unmodifiableList( > MAJOR SonarQube violation: Done Line 235: if (fID != null && fID.equals(matchedFuncExpr.getFunctionIdentifier())) { > BLOCKER SonarQube violation: Done Line 235: if (fID != null && fID.equals(matchedFuncExpr.getFunctionIdentifier())) { > BLOCKER SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/1/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 173: Mutable<ILogicalOperator> additionalSubTreeOpRef; > MAJOR SonarQube violation: Done Line 354: op = (AbstractLogicalOperator) opRef.getValue(); > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/4/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/RTreeAccessMethod.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/RTreeAccessMethod.java: Line 143: > MAJOR SonarQube violation: Done Line 176: isIndexOnlyPlan = false; > MAJOR SonarQube violation: Done Line 178: } > MAJOR SonarQube violation: Done Line 179: } > MAJOR SonarQube violation: Done Line 183: ILogicalOperator primaryIndexUnnestOp = createSecondaryToPrimaryPlan(afterSelectRefs, selectRef, > MAJOR SonarQube violation: Done Line 192: > MAJOR SonarQube violation: Done Line 247: dataset.getDataverseName(), dataset.getDatasetName(), retainInput, requiresBroadcast); > MAJOR SonarQube violation: Done Line 248: // A spatial object is serialized in the constant of the func expr we are optimizing. > MAJOR SonarQube violation: Done Line 248: // A spatial object is serialized in the constant of the func expr we are optimizing. > MAJOR SonarQube violation: Done Line 403: if (isLeftOuterJoin && hasGroupBy) { > MAJOR SonarQube violation: Done Line 404: ScalarFunctionCallExpression LOJFuncExprs = analysisCtx.getLOJIsNullFuncInGroupBy(); > MAJOR SonarQube violation: Done Line 405: List<LogicalVariable> LOJNullVariables = new ArrayList<LogicalVariable>(); > MAJOR SonarQube violation: Done Line 406: LOJFuncExprs.getUsedVariables(LOJNullVariables); > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/RTreeAccessMethod.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/RTreeAccessMethod.java: Line 76: private static List<Pair<FunctionIdentifier, Boolean>> FUNC_IDENTIFIERS = Collections.unmodifiableList( > MAJOR SonarQube violation: Done Line 76: private static List<Pair<FunctionIdentifier, Boolean>> FUNC_IDENTIFIERS = Collections.unmodifiableList( > MAJOR SonarQube violation: Done Line 173: requireVerificationAfterSIdxSearch = indexOnlyPlanInfo.third; > MAJOR SonarQube violation: Done Line 173: requireVerificationAfterSIdxSearch = indexOnlyPlanInfo.third; > MAJOR SonarQube violation: Done PS13, Line 240: generateInstantTrylockResultFromIndexSearch = true; > change that to an one line assignment. Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/AbstractOperationCallbackFactory.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/AbstractOperationCallbackFactory.java: PS13, Line 34: protected boolean isIndexOnlyPlanEnabled; > Move to this flag to the specific call back need that? Done https://asterix-gerrit.ics.uci.edu/#/c/1866/1/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java: Line 470: dataset.getSearchCallbackFactory(storaegComponentProvider, theIndex, jobId, IndexOperation.SEARCH, > MAJOR SonarQube violation: Done Line 496: JobGenContext context, boolean retainInput, boolean retainMissing, Dataset dataset, String indexName, > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java: PS13, Line 558: public > copy the documentation here if it is public. Done https://asterix-gerrit.ics.uci.edu/#/c/1866/1/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/ATypeHierarchy.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/ATypeHierarchy.java: Line 401: public enum TypeCastingMathFunctionType { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/DoubleToInt16TypeConvertComputer.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/DoubleToInt16TypeConvertComputer.java: PS13, Line 62: switch (mathFunction) { : case CEIL: : sourceValue = Math.ceil(((ADouble) sourceObject).getDoubleValue()); : break; : case FLOOR: : sourceValue = Math.floor(((ADouble) sourceObject).getDoubleValue()); : break; : default: : sourceValue = ((ADouble) sourceObject).getDoubleValue(); : break; : } > mathFunction.covert(sourceObject, ...); Done https://asterix-gerrit.ics.uci.edu/#/c/1866/1/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Quadruple.java File hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Quadruple.java: Line 42: if (!(o instanceof Quadruple<?, ?, ?, ?>)) { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Quadruple.java File hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Quadruple.java: Line 18: public T1 first; > final? Changed to provide the type of the variables to private and made getter() and setter(). https://asterix-gerrit.ics.uci.edu/#/c/1866/13/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java: PS13, Line 252: AbstractLogicalOperator op = (AbstractLogicalOperator) r.getValue(); : for (Mutable<ILogicalOperator> i : op.getInputs()) { : typeOpRec(i, context); : } : if (op.hasNestedPlans()) { : for (ILogicalPlan p : ((AbstractOperatorWithNestedPlans) op).getNestedPlans()) { : typePlan(p, context); : } : } : context.computeAndSetTypeEnvironmentForOperator(op); > return typeOpRec(r.getValue(), context); Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/IntroduceProjectsRule.java File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/IntroduceProjectsRule.java: Line 82: return introduceProjects(parentOps, null, -1, opRef, Collections.<LogicalVariable> emptySet(), context); > MAJOR SonarQube violation: Done Line 82: return introduceProjects(parentOps, null, -1, opRef, Collections.<LogicalVariable> emptySet(), context); > MAJOR SonarQube violation: Done PS13, Line 186: boolean projectBeforeUnionFound = false; > Make the rule general and agnostic to specific operators. Done https://asterix-gerrit.ics.uci.edu/#/c/1866/5/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveUnusedAssignAndAggregateRule.java File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveUnusedAssignAndAggregateRule.java: Line 430: */ > MAJOR SonarQube violation: Done Line 436: > MAJOR SonarQube violation: Done Line 436: > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveUnusedAssignAndAggregateRule.java File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveUnusedAssignAndAggregateRule.java: PS13, Line 156: .getOperatorTag() == LogicalOperatorTag.AGGREGATE : || (op.getOperatorTag() == LogicalOperatorTag.UNIONALL && keepUnionOp > Fix it here instead of adding a special case handling method. Done https://asterix-gerrit.ics.uci.edu/#/c/1866/13/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java: PS13, Line 54: private byte[] returnValuesArrayForProccedResult = new byte[10]; > split into two byte arrays. split into two byte arrays: done. Passing those values through CursorFactory instead of opContext: it was hard for me to come up with a viable solution for this. Sorry. -- To view, visit https://asterix-gerrit.ics.uci.edu/1866 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifd5c9ab1cf2e4bedb7d8db582441919875e74d51 Gerrit-PatchSet: 13 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <wangs...@gmail.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
