You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by "Taewoo Kim (Code Review)" <do...@asterixdb.incubator.apache.org> on 2017/08/05 18:07:20 UTC
Change in asterixdb[master]: [ASTERIXDB-1972][COMP][RT][TX] index-only plan
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 <wa...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <wa...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-HasComments: Yes