You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ar...@apache.org on 2017/12/22 12:45:26 UTC
[1/2] drill git commit: DRILL-6028: Allow splitting generated code in
ChainedHashTable into blocks to avoid "code too large" error
Repository: drill
Updated Branches:
refs/heads/master 40de8ca4f -> e25c58f7b
DRILL-6028: Allow splitting generated code in ChainedHashTable into blocks to avoid "code too large" error
1. Added new parameter seedValue to getHashBuild and getHashProbe methods in HashTableTemplate.
2. Generate logical expression for each key so its can be split into blocks if number of expressions in method exceeds upper limit.
3. ParameterExpression was added to generate reference to method parameter during code generation.
closes #1071
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/ef4c63d5
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/ef4c63d5
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/ef4c63d5
Branch: refs/heads/master
Commit: ef4c63d56e2339721867a964b724883a9b5d1fbe
Parents: 40de8ca
Author: Arina Ielchiieva <ar...@gmail.com>
Authored: Tue Dec 12 18:45:57 2017 +0200
Committer: Arina Ielchiieva <ar...@gmail.com>
Committed: Fri Dec 22 13:35:51 2017 +0200
----------------------------------------------------------------------
.../sig/ConstantExpressionIdentifier.java | 7 +++-
.../apache/drill/exec/expr/ClassGenerator.java | 18 +++++++--
.../drill/exec/expr/EvaluationVisitor.java | 23 ++++++++++++
.../physical/impl/common/ChainedHashTable.java | 29 ++++++++++-----
.../physical/impl/common/HashTableTemplate.java | 9 +++--
.../exec/planner/physical/HashPrelUtil.java | 20 +++++++---
.../org/apache/drill/TestUnionDistinct.java | 35 ++++++++++++++++++
.../common/expression/ValueExpressions.java | 39 ++++++++++++++++++++
.../visitors/AbstractExprVisitor.java | 7 +++-
.../expression/visitors/AggregateChecker.java | 8 +++-
.../expression/visitors/ConstantChecker.java | 8 +++-
.../common/expression/visitors/ExprVisitor.java | 4 +-
.../visitors/ExpressionValidator.java | 7 +++-
13 files changed, 187 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java
index d98c06b..1e71773 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -226,4 +226,9 @@ public class ConstantExpressionIdentifier implements ExprVisitor<Boolean, Identi
IdentityHashMap<LogicalExpression, Object> value) throws RuntimeException {
return e.getInput().accept(this, value);
}
+
+ @Override
+ public Boolean visitParameter(ValueExpressions.ParameterExpression e, IdentityHashMap<LogicalExpression, Object> value) throws RuntimeException {
+ return false;
+ }
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
index 8547ed4..d4ee60a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
@@ -225,7 +225,7 @@ public class ClassGenerator<T>{
}
public JLabel getEvalBlockLabel (String prefix) {
- return getEvalBlock().label(prefix + labelIndex ++);
+ return getEvalBlock().label(prefix + labelIndex++);
}
/**
@@ -543,12 +543,24 @@ public class ClassGenerator<T>{
}
public HoldingContainer declare(MajorType t, boolean includeNewInstance) {
+ return declare(t, "out", includeNewInstance);
+ }
+
+ /**
+ * Adds local variable declaration based on given name and type.
+ *
+ * @param t major type
+ * @param name variable name
+ * @param includeNewInstance whether to create new instance
+ * @return holder instance
+ */
+ public HoldingContainer declare(MajorType t, String name, boolean includeNewInstance) {
JType holderType = getHolderType(t);
JVar var;
if (includeNewInstance) {
- var = getEvalBlock().decl(holderType, "out" + index, JExpr._new(holderType));
+ var = getEvalBlock().decl(holderType, name + index, JExpr._new(holderType));
} else {
- var = getEvalBlock().decl(holderType, "out" + index);
+ var = getEvalBlock().decl(holderType, name + index);
}
JFieldRef outputSet = null;
if (t.getMode() == DataMode.OPTIONAL) {
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
index cb62b27..dcc2668 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
@@ -37,6 +37,7 @@ import org.apache.drill.common.expression.NullExpression;
import org.apache.drill.common.expression.PathSegment;
import org.apache.drill.common.expression.SchemaPath;
import org.apache.drill.common.expression.TypedNullConstant;
+import org.apache.drill.common.expression.ValueExpressions;
import org.apache.drill.common.expression.ValueExpressions.BooleanExpression;
import org.apache.drill.common.expression.ValueExpressions.DateExpression;
import org.apache.drill.common.expression.ValueExpressions.Decimal18Expression;
@@ -350,6 +351,28 @@ public class EvaluationVisitor {
}
+ /**
+ * <p>
+ * Creates local variable based on given parameter type and name and assigns parameter to this local instance.
+ * </p>
+ *
+ * <p>
+ * Example: <br/>
+ * IntHolder seedValue0 = new IntHolder();<br/>
+ * seedValue0 .value = seedValue;
+ * </p>
+ *
+ * @param e parameter expression
+ * @param generator class generator
+ * @return holder instance
+ */
+ @Override
+ public HoldingContainer visitParameter(ValueExpressions.ParameterExpression e, ClassGenerator<?> generator) {
+ HoldingContainer out = generator.declare(e.getMajorType(), e.getName(), true);
+ generator.getEvalBlock().assign(out.getValue(), JExpr.ref(e.getName()));
+ return out;
+ }
+
private HoldingContainer visitValueVectorWriteExpression(ValueVectorWriteExpression e, ClassGenerator<?> generator) {
final LogicalExpression child = e.getChild();
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
index dd4d76e..9bab67d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
@@ -18,13 +18,14 @@
package org.apache.drill.exec.physical.impl.common;
import java.io.IOException;
-import java.util.Arrays;
import java.util.List;
import org.apache.drill.common.expression.ErrorCollector;
import org.apache.drill.common.expression.ErrorCollectorImpl;
import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.expression.ValueExpressions;
import org.apache.drill.common.logical.data.NamedExpression;
+import org.apache.drill.common.types.TypeProtos;
import org.apache.drill.common.types.Types;
import org.apache.drill.exec.compile.sig.GeneratorMapping;
import org.apache.drill.exec.compile.sig.MappingSet;
@@ -154,7 +155,6 @@ public class ChainedHashTable {
ErrorCollector collector = new ErrorCollectorImpl();
VectorContainer htContainerOrig = new VectorContainer(); // original ht container from which others may be cloned
- LogicalExpression[] htKeyExprs = new LogicalExpression[htConfig.getKeyExprsBuild().size()];
TypedFieldId[] htKeyFieldIds = new TypedFieldId[htConfig.getKeyExprsBuild().size()];
int i = 0;
@@ -241,10 +241,10 @@ public class ChainedHashTable {
return;
}
- for (int i=0; i<keyExprs.length; i++) {
+ for (int i = 0; i < keyExprs.length; i++) {
final LogicalExpression expr = keyExprs[i];
cg.setMappingSet(incomingMapping);
- HoldingContainer left = cg.addExpr(expr, ClassGenerator.BlkCreateMode.FALSE);
+ HoldingContainer left = cg.addExpr(expr, ClassGenerator.BlkCreateMode.TRUE_IF_BOUND);
cg.setMappingSet(htableMapping);
ValueVectorReadExpression vvrExpr = new ValueVectorReadExpression(htKeyFieldIds[i]);
@@ -286,7 +286,7 @@ public class ChainedHashTable {
boolean useSetSafe = !Types.isFixedWidthType(expr.getMajorType()) || Types.isRepeated(expr.getMajorType());
ValueVectorWriteExpression vvwExpr = new ValueVectorWriteExpression(htKeyFieldIds[i++], expr, useSetSafe);
- cg.addExpr(vvwExpr, ClassGenerator.BlkCreateMode.FALSE); // this will write to the htContainer at htRowIdx
+ cg.addExpr(vvwExpr, ClassGenerator.BlkCreateMode.TRUE_IF_BOUND);
}
}
@@ -320,11 +320,22 @@ public class ChainedHashTable {
* aggregate. For join we need to hash everything as double (both for distribution and for comparison) but
* for aggregation we can avoid the penalty of casting to double
*/
- LogicalExpression hashExpression = HashPrelUtil.getHashExpression(Arrays.asList(keyExprs), incomingProbe != null);
- final LogicalExpression materializedExpr = ExpressionTreeMaterializer.materializeAndCheckErrors(hashExpression, batch, context.getFunctionRegistry());
- HoldingContainer hash = cg.addExpr(materializedExpr);
- cg.getEvalBlock()._return(hash.getValue());
+ /*
+ Generate logical expression for each key so expression can be split into blocks if number of expressions in method exceeds upper limit.
+ `seedValue` is used as holder to pass generated seed value for the new methods.
+ */
+ String seedValue = "seedValue";
+ LogicalExpression seed = ValueExpressions.getParameterExpression(seedValue, Types.required(TypeProtos.MinorType.INT));
+
+ for (LogicalExpression expr : keyExprs) {
+ LogicalExpression hashExpression = HashPrelUtil.getHashExpression(expr, seed, incomingProbe != null);
+ LogicalExpression materializedExpr = ExpressionTreeMaterializer.materializeAndCheckErrors(hashExpression, batch, context.getFunctionRegistry());
+ HoldingContainer hash = cg.addExpr(materializedExpr, ClassGenerator.BlkCreateMode.TRUE_IF_BOUND);
+ cg.getEvalBlock().assign(JExpr.ref(seedValue), hash.getValue());
+ }
+
+ cg.getEvalBlock()._return(JExpr.ref(seedValue));
}
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
index 7a086aa..6cbbdcb 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
@@ -578,7 +578,7 @@ public abstract class HashTableTemplate implements HashTable {
}
public int getHashCode(int incomingRowIdx) throws SchemaChangeException {
- return getHashBuild(incomingRowIdx);
+ return getHashBuild(incomingRowIdx, 0);
}
/** put() uses the hash code (from gethashCode() above) to insert the key(s) from the incoming
@@ -667,7 +667,8 @@ public abstract class HashTableTemplate implements HashTable {
// Return -1 if key is not found in the hash table. Otherwise, return the global index of the key
@Override
public int containsKey(int incomingRowIdx, boolean isProbe) throws SchemaChangeException {
- int hash = isProbe ? getHashProbe(incomingRowIdx) : getHashBuild(incomingRowIdx);
+ int seedValue = 0;
+ int hash = isProbe ? getHashProbe(incomingRowIdx, seedValue) : getHashBuild(incomingRowIdx, seedValue);
int bucketIndex = getBucketIndex(hash, numBuckets());
for ( currentIdxHolder.value = startIndices.getAccessor().get(bucketIndex);
@@ -814,8 +815,8 @@ public abstract class HashTableTemplate implements HashTable {
// These methods will be code-generated in the context of the outer class
protected abstract void doSetup(@Named("incomingBuild") RecordBatch incomingBuild, @Named("incomingProbe") RecordBatch incomingProbe) throws SchemaChangeException;
- protected abstract int getHashBuild(@Named("incomingRowIdx") int incomingRowIdx) throws SchemaChangeException;
+ protected abstract int getHashBuild(@Named("incomingRowIdx") int incomingRowIdx, @Named("seedValue") int seedValue) throws SchemaChangeException;
- protected abstract int getHashProbe(@Named("incomingRowIdx") int incomingRowIdx) throws SchemaChangeException;
+ protected abstract int getHashProbe(@Named("incomingRowIdx") int incomingRowIdx, @Named("seedValue") int seedValue) throws SchemaChangeException;
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashPrelUtil.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashPrelUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashPrelUtil.java
index 4300c82..25b7017 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashPrelUtil.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashPrelUtil.java
@@ -25,7 +25,13 @@ import org.apache.drill.common.expression.FieldReference;
import org.apache.drill.common.expression.FunctionCall;
import org.apache.drill.common.expression.LogicalExpression;
import org.apache.drill.common.expression.ValueExpressions;
+import org.apache.drill.common.logical.data.NamedExpression;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.common.types.Types;
+import org.apache.drill.exec.expr.DirectExpression;
+import org.apache.drill.exec.expr.ValueVectorReadExpression;
import org.apache.drill.exec.planner.physical.DrillDistributionTrait.DistributionField;
+import org.apache.drill.exec.record.TypedFieldId;
import java.util.ArrayList;
import java.util.List;
@@ -100,7 +106,7 @@ public class HashPrelUtil {
final String functionName = hashAsDouble ? HASH32_DOUBLE_FUNCTION_NAME : HASH32_FUNCTION_NAME;
- T func = helper.createCall(functionName, ImmutableList.of(inputExprs.get(0), seed ));
+ T func = helper.createCall(functionName, ImmutableList.of(inputExprs.get(0), seed));
for (int i = 1; i<inputExprs.size(); i++) {
func = helper.createCall(functionName, ImmutableList.of(inputExprs.get(i), func));
}
@@ -109,11 +115,15 @@ public class HashPrelUtil {
}
/**
- * Return a hash expression : hash32(field1, hash32(field2, hash32(field3, 0)));
+ * Creates hash expression for input field and seed.
+ *
+ * @param field field expression
+ * @param seed seed expression
+ * @param hashAsDouble whether to use the hash as double function or regular hash64 function
+ * @return hash expression
*/
- public static LogicalExpression getHashExpression(List<LogicalExpression> fields, boolean hashAsDouble){
- final LogicalExpression seed = ValueExpressions.getInt(0); // Hash Table seed
- return createHashExpression(fields, seed, HASH_HELPER_LOGICALEXPRESSION, hashAsDouble);
+ public static LogicalExpression getHashExpression(LogicalExpression field, LogicalExpression seed, boolean hashAsDouble) {
+ return createHashExpression(ImmutableList.of(field), seed, HASH_HELPER_LOGICALEXPRESSION, hashAsDouble);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/exec/java-exec/src/test/java/org/apache/drill/TestUnionDistinct.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestUnionDistinct.java b/exec/java-exec/src/test/java/org/apache/drill/TestUnionDistinct.java
index b76d308..4b8140e 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestUnionDistinct.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestUnionDistinct.java
@@ -19,6 +19,7 @@ package org.apache.drill;
import com.google.common.collect.Lists;
+import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.drill.categories.OperatorTest;
import org.apache.drill.categories.SqlTest;
@@ -32,6 +33,7 @@ import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.experimental.categories.Category;
+import java.io.File;
import java.nio.file.Paths;
import java.util.List;
@@ -754,4 +756,37 @@ public class TestUnionDistinct extends BaseTestQuery {
}
}
+ @Test
+ public void testUnionWithManyColumns() throws Exception {
+ int columnsCount = 1200;
+ File file = new File(dirTestWatcher.getRootDir(), "union_for_" + columnsCount + "_columns.csv");
+
+ StringBuilder line = new StringBuilder();
+ StringBuilder columns = new StringBuilder();
+
+ for (int i = 0; i <= columnsCount; i++) {
+ line.append(i).append(",");
+ columns.append("columns[").append(i).append("]").append(",");
+ }
+ line.deleteCharAt(line.length() - 1);
+ columns.deleteCharAt(columns.length() - 1);
+
+ try {
+ /*
+ 0,1,2...1200
+ 0,1,2...1200
+ */
+ FileUtils.writeStringToFile(file, String.format("%1$s\n%1$s", line.toString()));
+
+ /*
+ select columns[0], columns[1] ... columns[1200] from dfs.`union_for_1200_columns.csv`
+ union
+ select columns[0], columns[1] ... columns[1200] from dfs.`union_for_1200_columns.csv`
+ */
+ test("select %1$s from dfs.`%2$s` union select %1$s from dfs.`%2$s`", columns.toString(), file.getName());
+ } finally {
+ FileUtils.deleteQuietly(file);
+ }
+ }
+
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/logical/src/main/java/org/apache/drill/common/expression/ValueExpressions.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/expression/ValueExpressions.java b/logical/src/main/java/org/apache/drill/common/expression/ValueExpressions.java
index 556135f..527b6b7 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/ValueExpressions.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/ValueExpressions.java
@@ -21,6 +21,7 @@ import java.math.BigDecimal;
import java.util.GregorianCalendar;
import java.util.Iterator;
+import com.google.common.collect.ImmutableList;
import org.apache.drill.common.expression.visitors.ExprVisitor;
import org.apache.drill.common.types.TypeProtos.DataMode;
import org.apache.drill.common.types.TypeProtos.MajorType;
@@ -135,6 +136,10 @@ public class ValueExpressions {
}
+ public static LogicalExpression getParameterExpression(String name, MajorType type) {
+ return new ParameterExpression(name, type, ExpressionPosition.UNKNOWN);
+ }
+
protected static abstract class ValueExpression<V> extends LogicalExpressionBase {
public final V value;
@@ -679,4 +684,38 @@ public class ValueExpressions {
}
}
+ /**
+ * Is used to identify method parameter based on given name and type.
+ */
+ public static class ParameterExpression extends LogicalExpressionBase {
+
+ private final String name;
+ private final MajorType type;
+
+ protected ParameterExpression(String name, MajorType type, ExpressionPosition pos) {
+ super(pos);
+ this.name = name;
+ this.type = type;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ @Override
+ public MajorType getMajorType() {
+ return type;
+ }
+
+ @Override
+ public <T, V, E extends Exception> T accept(ExprVisitor<T, V, E> visitor, V value) throws E {
+ return visitor.visitParameter(this, value);
+ }
+
+ @Override
+ public Iterator<LogicalExpression> iterator() {
+ return ImmutableList.<LogicalExpression>of().iterator();
+ }
+ }
+
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java b/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java
index a31e6e8..189e33d 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -39,6 +39,7 @@ import org.apache.drill.common.expression.ValueExpressions.IntExpression;
import org.apache.drill.common.expression.ValueExpressions.IntervalDayExpression;
import org.apache.drill.common.expression.ValueExpressions.IntervalYearExpression;
import org.apache.drill.common.expression.ValueExpressions.LongExpression;
+import org.apache.drill.common.expression.ValueExpressions.ParameterExpression;
import org.apache.drill.common.expression.ValueExpressions.QuotedString;
import org.apache.drill.common.expression.ValueExpressions.TimeExpression;
import org.apache.drill.common.expression.ValueExpressions.TimeStampExpression;
@@ -172,5 +173,9 @@ public abstract class AbstractExprVisitor<T, VAL, EXCEP extends Exception> imple
throw new UnsupportedOperationException(String.format("Expression of type %s not handled by visitor type %s.", e.getClass().getCanonicalName(), this.getClass().getCanonicalName()));
}
+ @Override
+ public T visitParameter(ParameterExpression e, VAL value) throws EXCEP {
+ return visitUnknown(e, value);
+ }
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/logical/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java b/logical/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
index 46789e3..9a3cdcc 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -29,6 +29,7 @@ import org.apache.drill.common.expression.LogicalExpression;
import org.apache.drill.common.expression.NullExpression;
import org.apache.drill.common.expression.SchemaPath;
import org.apache.drill.common.expression.TypedNullConstant;
+import org.apache.drill.common.expression.ValueExpressions;
import org.apache.drill.common.expression.ValueExpressions.BooleanExpression;
import org.apache.drill.common.expression.ValueExpressions.DateExpression;
import org.apache.drill.common.expression.ValueExpressions.Decimal18Expression;
@@ -203,4 +204,9 @@ public final class AggregateChecker implements ExprVisitor<Boolean, ErrorCollect
return false;
}
+ @Override
+ public Boolean visitParameter(ValueExpressions.ParameterExpression e, ErrorCollector value) throws RuntimeException {
+ return false;
+ }
+
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/logical/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java b/logical/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
index ccb11a6..67fe12f 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -29,6 +29,7 @@ import org.apache.drill.common.expression.LogicalExpression;
import org.apache.drill.common.expression.NullExpression;
import org.apache.drill.common.expression.SchemaPath;
import org.apache.drill.common.expression.TypedNullConstant;
+import org.apache.drill.common.expression.ValueExpressions;
import org.apache.drill.common.expression.ValueExpressions.BooleanExpression;
import org.apache.drill.common.expression.ValueExpressions.DateExpression;
import org.apache.drill.common.expression.ValueExpressions.Decimal18Expression;
@@ -207,4 +208,9 @@ final class ConstantChecker implements ExprVisitor<Boolean, ErrorCollector, Runt
return true;
}
+ @Override
+ public Boolean visitParameter(ValueExpressions.ParameterExpression e, ErrorCollector value) throws RuntimeException {
+ return false;
+ }
+
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/logical/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java b/logical/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java
index 2d0cd8c..7c59f3c 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -39,6 +39,7 @@ import org.apache.drill.common.expression.ValueExpressions.IntExpression;
import org.apache.drill.common.expression.ValueExpressions.IntervalDayExpression;
import org.apache.drill.common.expression.ValueExpressions.IntervalYearExpression;
import org.apache.drill.common.expression.ValueExpressions.LongExpression;
+import org.apache.drill.common.expression.ValueExpressions.ParameterExpression;
import org.apache.drill.common.expression.ValueExpressions.QuotedString;
import org.apache.drill.common.expression.ValueExpressions.TimeExpression;
import org.apache.drill.common.expression.ValueExpressions.TimeStampExpression;
@@ -69,4 +70,5 @@ public interface ExprVisitor<T, VAL, EXCEP extends Exception> {
public T visitUnknown(LogicalExpression e, VAL value) throws EXCEP;
public T visitCastExpression(CastExpression e, VAL value) throws EXCEP;
public T visitConvertExpression(ConvertExpression e, VAL value) throws EXCEP;
+ public T visitParameter(ParameterExpression e, VAL value) throws EXCEP;
}
http://git-wip-us.apache.org/repos/asf/drill/blob/ef4c63d5/logical/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java b/logical/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java
index eb1394c..e8cbc2b 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/visitors/ExpressionValidator.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -231,4 +231,9 @@ public class ExpressionValidator implements ExprVisitor<Void, ErrorCollector, Ru
return e.getInput().accept(this, value);
}
+ @Override
+ public Void visitParameter(ValueExpressions.ParameterExpression e, ErrorCollector value) throws RuntimeException {
+ return null;
+ }
+
}
[2/2] drill git commit: DRILL-5996: Add ability to re-run queries
from Profiles tab with impersonation and without authentication
Posted by ar...@apache.org.
DRILL-5996: Add ability to re-run queries from Profiles tab with impersonation and without authentication
closes #1061
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/e25c58f7
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/e25c58f7
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/e25c58f7
Branch: refs/heads/master
Commit: e25c58f7bf0ad07d3611b85d6d82d05549a28791
Parents: ef4c63d
Author: Arina Ielchiieva <ar...@gmail.com>
Authored: Mon Dec 4 19:33:03 2017 +0200
Committer: Arina Ielchiieva <ar...@gmail.com>
Committed: Fri Dec 22 13:36:18 2017 +0200
----------------------------------------------------------------------
.../server/rest/profile/ProfileResources.java | 12 +++----
.../server/rest/profile/ProfileWrapper.java | 29 ++++++++++++----
.../src/main/resources/rest/profile/profile.ftl | 18 ++++++++--
.../src/main/resources/rest/query/query.ftl | 31 ++---------------
.../resources/rest/static/js/querySubmission.js | 36 ++++++++++++++++++++
5 files changed, 82 insertions(+), 44 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/e25c58f7/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
index fdd391e..8751ee6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
@@ -123,17 +123,17 @@ public class ProfileResources {
public String getLink() { return link; }
- @Override
- public int compareTo(ProfileInfo other) {
- return time.compareTo(other.time);
- }
-
public String getForeman() { return foreman; }
public double getTotalCost() { return totalCost; }
public String getQueueName() { return queueName; }
+ @Override
+ public int compareTo(ProfileInfo other) {
+ return time.compareTo(other.time);
+ }
+
/**
* Generates link which will return query profile in json representation.
*
@@ -370,7 +370,7 @@ public class ProfileResources {
@Path("/profiles/{queryid}")
@Produces(MediaType.TEXT_HTML)
public Viewable getProfile(@PathParam("queryid") String queryId){
- ProfileWrapper wrapper = new ProfileWrapper(getQueryProfile(queryId));
+ ProfileWrapper wrapper = new ProfileWrapper(getQueryProfile(queryId), work.getContext().getConfig());
return ViewableWithPermissions.create(authEnabled.get(), "/rest/profile/profile.ftl", sc, wrapper);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/e25c58f7/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java
index 3a7d432..9c2b438 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileWrapper.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -26,6 +26,7 @@ import java.util.Map;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.Maps;
import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
import org.apache.drill.exec.proto.UserBitShared.MajorFragmentProfile;
import org.apache.drill.exec.proto.UserBitShared.MinorFragmentProfile;
@@ -35,6 +36,7 @@ import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
import org.apache.drill.exec.proto.helper.QueryIdHelper;
import org.apache.drill.exec.server.options.OptionList;
import org.apache.drill.exec.server.options.OptionValue;
+import org.apache.drill.exec.server.rest.WebServer;
import static com.fasterxml.jackson.databind.SerializationFeature.INDENT_OUTPUT;
@@ -47,15 +49,16 @@ public class ProfileWrapper {
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ProfileWrapper.class);
private static final ObjectMapper mapper = new ObjectMapper().enable(INDENT_OUTPUT);
- private QueryProfile profile;
- private String id;
+ private final QueryProfile profile;
+ private final String id;
private final List<FragmentWrapper> fragmentProfiles;
private final List<OperatorWrapper> operatorProfiles;
- private OptionList options;
private final HashMap<String, Long> majorFragmentTallyMap;
- private long majorFragmentTallyTotal;
+ private final long majorFragmentTallyTotal;
+ private final OptionList options;
+ private final boolean onlyImpersonationEnabled;
- public ProfileWrapper(final QueryProfile profile) {
+ public ProfileWrapper(final QueryProfile profile, DrillConfig drillConfig) {
this.profile = profile;
this.id = QueryIdHelper.getQueryId(profile.getId());
@@ -68,7 +71,7 @@ public class ProfileWrapper {
fragmentProfiles.add(new FragmentWrapper(major, profile.getStart()));
}
this.fragmentProfiles = fragmentProfiles;
- majorFragmentTallyMap = new HashMap<String, Long>(majors.size());
+ this.majorFragmentTallyMap = new HashMap<>(majors.size());
this.majorFragmentTallyTotal = tallyMajorFragmentCost(majors);
final List<OperatorWrapper> ows = new ArrayList<>();
@@ -108,12 +111,16 @@ public class ProfileWrapper {
}
this.operatorProfiles = ows;
+ OptionList options;
try {
options = mapper.readValue(profile.getOptionsJson(), OptionList.class);
} catch (Exception e) {
logger.error("Unable to deserialize query options", e);
options = new OptionList();
}
+ this.options = options;
+
+ this.onlyImpersonationEnabled = WebServer.isImpersonationOnlyEnabled(drillConfig);
}
private long tallyMajorFragmentCost(List<MajorFragmentProfile> majorFragments) {
@@ -297,4 +304,12 @@ public class ProfileWrapper {
}
return map;
}
+
+ /**
+ * @return true if impersonation is enabled without authentication,
+ * is needed to indicated if user name should be included when re-running the query
+ */
+ public boolean isOnlyImpersonationEnabled() {
+ return onlyImpersonationEnabled;
+ }
}
http://git-wip-us.apache.org/repos/asf/drill/blob/e25c58f7/exec/java-exec/src/main/resources/rest/profile/profile.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/profile/profile.ftl b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
index ff78da3..1f38d2e 100644
--- a/exec/java-exec/src/main/resources/rest/profile/profile.ftl
+++ b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
@@ -15,6 +15,10 @@
<script src="/static/js/dagre-d3.min.js"></script>
<script src="/static/js/graph.js"></script>
<script src="/static/js/jquery.dataTables-1.10.16.min.js"></script>
+<#if model.isOnlyImpersonationEnabled()>
+ <script src="/static/js/jquery.form.js"></script>
+ <script src="/static/js/querySubmission.js"></script>
+</#if>
<script>
var globalconfig = {
@@ -71,7 +75,15 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
</div>
<div id="query-edit" class="tab-pane">
<p>
- <form role="form" action="/query" method="POST">
+
+ <#if model.isOnlyImpersonationEnabled()>
+ <div class="form-group">
+ <label for="userName">User Name</label>
+ <input type="text" size="30" name="userName" id="userName" placeholder="User Name" value="${model.getProfile().user}">
+ </div>
+ </#if>
+
+ <form role="form" id="queryForm" action="/query" method="POST">
<div class="form-group">
<textarea class="form-control" id="query" name="query" style="font-family: Courier;">${model.getProfile().query}</textarea>
</div>
@@ -95,7 +107,9 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
</label>
</div>
</div>
- <button type="submit" class="btn btn-default">Re-run query</button>
+ <button class="btn btn-default" type=<#if model.isOnlyImpersonationEnabled()>"button" onclick="doSubmitQueryWithUserName()"<#else>"submit"</#if>>
+ Re-run query
+ </button>
</form>
</p>
<p>
http://git-wip-us.apache.org/repos/asf/drill/blob/e25c58f7/exec/java-exec/src/main/resources/rest/query/query.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/query/query.ftl b/exec/java-exec/src/main/resources/rest/query/query.ftl
index f9765eb..93194d0 100644
--- a/exec/java-exec/src/main/resources/rest/query/query.ftl
+++ b/exec/java-exec/src/main/resources/rest/query/query.ftl
@@ -13,6 +13,7 @@
<#macro page_head>
<#if model?? && model>
<script src="/static/js/jquery.form.js"></script>
+ <script src="/static/js/querySubmission.js"></script>
</#if>
</#macro>
@@ -59,38 +60,10 @@
<textarea class="form-control" id="query" rows="5" name="query" style="font-family: Courier;"></textarea>
</div>
- <button class="btn btn-default" type=<#if model?? && model>"button" onclick="doSubmit()"<#else>"submit"</#if>>
+ <button class="btn btn-default" type=<#if model?? && model>"button" onclick="doSubmitQueryWithUserName()"<#else>"submit"</#if>>
Submit
</button>
</form>
-
- <#if model?? && model>
- <script>
- function doSubmit() {
- var userName = document.getElementById("userName").value;
- if (!userName.trim()) {
- alert("Please fill in User Name field");
- return;
- }
- $.ajax({
- type: "POST",
- beforeSend: function (request) {
- request.setRequestHeader("User-Name", userName);
- },
- url: "/query",
- data: $("#queryForm").serializeArray(),
- success: function (response) {
- var newDoc = document.open("text/html", "replace");
- newDoc.write(response);
- newDoc.close();
- },
- error: function (request, textStatus, errorThrown) {
- alert(errorThrown);
- }
- });
- }
- </script>
- </#if>
</#macro>
<@page_html/>
http://git-wip-us.apache.org/repos/asf/drill/blob/e25c58f7/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js b/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js
new file mode 100644
index 0000000..638ddbf
--- /dev/null
+++ b/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership. The ASF licenses this file to
+ * You under the Apache License, Version 2.0 (the "License"); you may not use
+ * this file except in compliance with the License. You may obtain a copy of
+ * the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ * by applicable law or agreed to in writing, software distributed under the
+ * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ * OF ANY KIND, either express or implied. See the License for the specific
+ * language governing permissions and limitations under the License.
+ */
+
+function doSubmitQueryWithUserName() {
+ var userName = document.getElementById("userName").value;
+ if (!userName.trim()) {
+ alert("Please fill in User Name field");
+ return;
+ }
+ $.ajax({
+ type: "POST",
+ beforeSend: function (request) {
+ request.setRequestHeader("User-Name", userName);
+ },
+ url: "/query",
+ data: $("#queryForm").serializeArray(),
+ success: function (response) {
+ var newDoc = document.open("text/html", "replace");
+ newDoc.write(response);
+ newDoc.close();
+ },
+ error: function (request, textStatus, errorThrown) {
+ alert(errorThrown);
+ }
+ });
+}
\ No newline at end of file