>From Preetham Poluparthi <preetha...@apache.org>:
Preetham Poluparthi has uploaded this change for review. (
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20282 )
Change subject: [ASTERIXDB-3632] Fix NPE in Index Advisor when secondary
primary index is present
......................................................................
[ASTERIXDB-3632] Fix NPE in Index Advisor when secondary primary index is
present
- user model changes: no
- storage format changes: no
- interface changes: no
Ext-ref: MB-66492
Change-Id: I346728dd634934416d7115a06ab15572b5a98854
---
M
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdvisorConditionParser.java
M
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdviseIndexRule.java
2 files changed, 58 insertions(+), 31 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb
refs/changes/82/20282/1
diff --git
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdviseIndexRule.java
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdviseIndexRule.java
index 9a7fbdd..f5e7500 100644
---
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdviseIndexRule.java
+++
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdviseIndexRule.java
@@ -122,6 +122,11 @@
String datasetName = jobGenParams.getDatasetName();
Index fakeIndex = fakeIndexProvider.getIndex(databaseName,
dataverse, datasetName, indexName);
+ if (fakeIndex == null || !(fakeIndex.getIndexDetails() instanceof
Index.ValueIndexDetails)) {
+ // skips secondary primary index like
+ // create primary index sec_primary_idx on A;
+ return;
+ }
Index actualIndex = lookupIndex(databaseName, dataverse,
datasetName,
((Index.ValueIndexDetails)
fakeIndex.getIndexDetails()).getKeyFieldNames(), actualIndexProvider);
diff --git
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdvisorConditionParser.java
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdvisorConditionParser.java
index a86b1b5..19fcdee 100644
---
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdvisorConditionParser.java
+++
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdvisorConditionParser.java
@@ -47,11 +47,27 @@
public class AdvisorConditionParser {
+ private static class ExprRef {
+ private final ILogicalExpression expr;
+ private final ILogicalOperator op;
+
+ public ExprRef(Mutable<ILogicalExpression> expr, ILogicalOperator op) {
+ this.expr = expr.getValue().cloneExpression();
+ this.op = op;
+ }
+
+ public ILogicalExpression getExpr() {
+ return expr;
+ }
+
+ public ILogicalOperator getOp() {
+ return op;
+ }
+ }
+
public static ScanFilter parseScanNode(ILogicalOperator op,
IOptimizationContext context)
throws AlgebricksException {
-
- List<Mutable<ILogicalExpression>> filterExprs = new ArrayList<>();
- IVariableTypeEnvironment typeEnv = PushdownUtil.getTypeEnv(op,
context);
+ List<ExprRef> filterExprRefs = new ArrayList<>();
ILogicalOperator tempOp = op;
do {
if (tempOp.getOperatorTag() == LogicalOperatorTag.SELECT) {
@@ -59,35 +75,33 @@
ILogicalExpression condition =
selectOp.getCondition().getValue();
List<Mutable<ILogicalExpression>> conjs = new ArrayList<>();
if (condition.splitIntoConjuncts(conjs)) {
- filterExprs.addAll(conjs);
+ filterExprRefs.addAll(conjs.stream().map(expr -> new
ExprRef(expr, selectOp)).toList());
} else {
- filterExprs.add(selectOp.getCondition());
+ filterExprRefs.add(new ExprRef(selectOp.getCondition(),
selectOp));
}
}
tempOp = tempOp.getInputs().getFirst().getValue();
} while (tempOp.hasInputs());
- filterExprs = OperatorManipulationUtil.cloneExpressions(filterExprs);
-
- filterExprs.removeIf(expr -> !(expr.getValue() instanceof
AbstractFunctionCallExpression));
+ filterExprRefs.removeIf(exprRef -> !(exprRef.getExpr() instanceof
AbstractFunctionCallExpression));
tempOp = op;
do {
if (tempOp.getOperatorTag() == LogicalOperatorTag.ASSIGN) {
- replaceExprsWithAssign((AssignOperator) tempOp, filterExprs);
+ replaceExprsWithAssign((AssignOperator) tempOp,
filterExprRefs);
}
tempOp = tempOp.getInputs().getFirst().getValue();
} while (tempOp.hasInputs());
List<ScanFilterCondition> filterConditions = new ArrayList<>();
- for (Mutable<ILogicalExpression> filterExpr : filterExprs) {
- ScanFilterCondition filterCondition =
parseCondition(filterExpr.getValue(), typeEnv);
+ for (ExprRef exprRef : filterExprRefs) {
+ IVariableTypeEnvironment typeEnv =
PushdownUtil.getTypeEnv(exprRef.getOp(), context);
+ ScanFilterCondition filterCondition =
parseCondition(exprRef.getExpr(), typeEnv);
if (filterCondition != null) {
filterConditions.add(filterCondition);
}
}
-
return new ScanFilter(filterConditions);
}
@@ -96,9 +110,7 @@
if (!(logicalExpression instanceof AbstractFunctionCallExpression
expr)) {
return null;
}
-
FunctionIdentifier fi = expr.getFunctionIdentifier();
-
if (!BTreeAccessMethod.INSTANCE.getOptimizableFunctions().contains(new
Pair<>(fi, false))) {
return null;
}
@@ -145,23 +157,21 @@
public static JoinFilter parseJoinNode(AbstractBinaryJoinOperator joinOp,
IOptimizationContext context)
throws AlgebricksException {
- List<Mutable<ILogicalExpression>> joinExprs = new ArrayList<>();
- IVariableTypeEnvironment typeEnv = PushdownUtil.getTypeEnv(joinOp,
context);
-
+ List<ExprRef> joinExprs = new ArrayList<>();
ILogicalExpression joinExpression = joinOp.getCondition().getValue();
List<Mutable<ILogicalExpression>> conjs = new ArrayList<>();
if (joinExpression.splitIntoConjuncts(conjs)) {
- joinExprs.addAll(conjs);
+ joinExprs.addAll(conjs.stream().map(expr -> new ExprRef(expr,
joinOp)).toList());
} else {
- joinExprs.add(joinOp.getCondition());
+ joinExprs.add(new ExprRef(joinOp.getCondition(), joinOp));
}
- joinExprs = OperatorManipulationUtil.cloneExpressions(joinExprs);
- traverseAndReplace(joinOp, joinExprs);
+ traverseAndReplace(joinOp, joinExprs);
List<JoinFilterCondition> joinConditions = new ArrayList<>();
- for (Mutable<ILogicalExpression> joinExpr : joinExprs) {
- JoinFilterCondition joinCondition =
parseJoinCondition(joinExpr.getValue(), typeEnv);
+ for (ExprRef joinExprRef : joinExprs) {
+ IVariableTypeEnvironment typeEnv =
PushdownUtil.getTypeEnv(joinExprRef.getOp(), context);
+ JoinFilterCondition joinCondition =
parseJoinCondition(joinExprRef.getExpr(), typeEnv);
if (joinCondition != null) {
joinConditions.add(joinCondition);
}
@@ -170,25 +180,22 @@
return new JoinFilter(joinConditions);
}
- private static void traverseAndReplace(AbstractLogicalOperator op,
List<Mutable<ILogicalExpression>> exprs) {
-
+ private static void traverseAndReplace(AbstractLogicalOperator op,
List<ExprRef> exprs) {
if (op.getOperatorTag() == LogicalOperatorTag.ASSIGN) {
replaceExprsWithAssign((AssignOperator) op, exprs);
}
-
for (Mutable<ILogicalOperator> input : op.getInputs()) {
traverseAndReplace((AbstractLogicalOperator) input.getValue(),
exprs);
}
-
}
- public static void replaceExprsWithAssign(AssignOperator assignOp,
List<Mutable<ILogicalExpression>> exprs) {
- for (Mutable<ILogicalExpression> filterExpr : exprs) {
- if (filterExpr.getValue().getExpressionTag() ==
LogicalExpressionTag.CONSTANT) {
+ private static void replaceExprsWithAssign(AssignOperator assignOp,
List<ExprRef> exprRefs) {
+ for (ExprRef exprRef : exprRefs) {
+ if (exprRef.getExpr().getExpressionTag() ==
LogicalExpressionTag.CONSTANT) {
continue;
}
for (int i = 0; i < assignOp.getVariables().size(); i++) {
-
OperatorManipulationUtil.replaceVarWithExpr((AbstractFunctionCallExpression)
filterExpr.getValue(),
+
OperatorManipulationUtil.replaceVarWithExpr((AbstractFunctionCallExpression)
exprRef.getExpr(),
assignOp.getVariables().get(i),
assignOp.getExpressions().get(i).getValue());
}
}
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20282
To unsubscribe, or for help writing mail filters, visit
https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I346728dd634934416d7115a06ab15572b5a98854
Gerrit-Change-Number: 20282
Gerrit-PatchSet: 1
Gerrit-Owner: Preetham Poluparthi <preetha...@apache.org>
Gerrit-MessageType: newchange
