Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
import org.apache.hadoop.hive.ql.exec.vector.VectorizationContext.InConstantType;
import org.apache.hadoop.hive.ql.exec.vector.VectorizationContextRegion;
import org.apache.hadoop.hive.ql.exec.vector.VectorizedSupport.Support;
import org.apache.hadoop.hive.ql.exec.vector.expressions.ConvertDecimal64ToDecimal;
import org.apache.hadoop.hive.ql.exec.vector.expressions.IdentityExpression;
import org.apache.hadoop.hive.ql.exec.vector.expressions.VectorExpression;
import org.apache.hadoop.hive.ql.exec.vector.expressions.aggregates.VectorAggregateExpression;
Expand Down Expand Up @@ -4797,6 +4798,8 @@ public static Operator<? extends OperatorDesc> vectorizeSelectOperator(

VectorExpression[] vectorSelectExprs = new VectorExpression[size];
int[] projectedOutputColumns = new int[size];
int[] vectorSelectExprIndexForCol = new int[size];
Arrays.fill(vectorSelectExprIndexForCol, -1);
for (int i = 0; i < size; i++) {
ExprNodeDesc expr = colList.get(i);
VectorExpression ve = vContext.getVectorExpression(expr);
Expand All @@ -4805,6 +4808,7 @@ public static Operator<? extends OperatorDesc> vectorizeSelectOperator(
// Suppress useless evaluation.
continue;
}
vectorSelectExprIndexForCol[i] = index;
vectorSelectExprs[index++] = ve;
}
if (index < size) {
Expand All @@ -4817,6 +4821,12 @@ public static Operator<? extends OperatorDesc> vectorizeSelectOperator(
// The following method introduces a cast if x or y is DECIMAL_64 and parent expression (x % y) is DECIMAL.
try {
fixDecimalDataTypePhysicalVariations(vContext, vectorSelectExprs);
for (int i = 0; i < size; i++) {
int exprIndex = vectorSelectExprIndexForCol[i];
if (exprIndex >= 0) {
projectedOutputColumns[i] = vectorSelectExprs[exprIndex].getOutputColumnNum();
}
Comment on lines +4824 to +4828

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change necessary? I found that projectedOutputColumns is set a few lines earlier here

projectedOutputColumns[i] = ve.getOutputColumnNum();

}
} finally {
vContext.freeMarkedScratchColumns();
}
Expand Down Expand Up @@ -4879,7 +4889,6 @@ private static VectorExpression fixDecimalDataTypePhysicalVariations(final Vecto
}
} else {
Object[] arguments;
int argumentCount = children.length + (parent.getOutputColumnNum() == -1 ? 0 : 1);
// VectorCoalesce receives arguments as an array.
// Need to handle it as a special case to avoid instantiation failure.
if (parent instanceof VectorCoalesce) {
Expand All @@ -4891,20 +4900,7 @@ private static VectorExpression fixDecimalDataTypePhysicalVariations(final Vecto
}
arguments[1] = parent.getOutputColumnNum();
} else {
if (parent instanceof DecimalColDivideDecimalScalar) {
arguments = new Object[argumentCount + 1];
arguments[children.length] = ((DecimalColDivideDecimalScalar) parent).getValue();
} else {
arguments = new Object[argumentCount];
}
for (int i = 0; i < children.length; i++) {
VectorExpression vce = children[i];
arguments[i] = vce.getOutputColumnNum();
}
}
// retain output column number from parent
if (parent.getOutputColumnNum() != -1) {
arguments[arguments.length - 1] = parent.getOutputColumnNum();
arguments = buildReinstantiationArgsForDecimal64(parent, children);
}
// re-instantiate the parent expression with new arguments
VectorExpression newParent = vContext.instantiateExpression(parent.getClass(), parent.getOutputTypeInfo(),
Expand All @@ -4913,14 +4909,86 @@ private static VectorExpression fixDecimalDataTypePhysicalVariations(final Vecto
newParent.setOutputDataTypePhysicalVariation(parent.getOutputDataTypePhysicalVariation());
newParent.setInputTypeInfos(parent.getInputTypeInfos());
newParent.setInputDataTypePhysicalVariations(dataTypePhysicalVariations);
newParent.setChildExpressions(parent.getChildExpressions());
newParent.setChildExpressions(children);
return newParent;
}
}
}
return parent;
}

/**
* Rebuild constructor arguments for a vector expression after wrapping DECIMAL_64 child
* expressions with {@link ConvertDecimal64ToDecimal}. Column reference inputs live in
* {@link VectorExpression#inputColumnNum} and are not included in childExpressions, so they
* must be preserved when re-instantiating the parent.
*/
static Object[] buildReinstantiationArgsForDecimal64(VectorExpression parent,
VectorExpression[] children) {
int[] inputColNums = extractParentInputColumnNums(parent);
replaceInputColsWithConvertedChildOutputs(inputColNums, children);
return buildParentConstructorArguments(inputColNums, parent);
}

private static int[] extractParentInputColumnNums(VectorExpression parent) {
int inputCount = 0;
for (int col : parent.inputColumnNum) {
if (col != -1) {
inputCount++;
}
}

int[] inputColNums = new int[inputCount];
int idx = 0;
for (int col : parent.inputColumnNum) {
if (col != -1) {
inputColNums[idx++] = col;
}
}
return inputColNums;
}

/**
* For each wrapped DECIMAL_64 child, replace its pre-conversion input column slot in
* {@code inputColNums} with the {@link ConvertDecimal64ToDecimal} output column.
*/
private static void replaceInputColsWithConvertedChildOutputs(int[] inputColNums,
VectorExpression[] children) {
for (VectorExpression child : children) {
int preConversionCol = getPreConversionColumnNum(child);
int convertedCol = child.getOutputColumnNum();
for (int i = 0; i < inputColNums.length; i++) {
if (inputColNums[i] == preConversionCol) {
inputColNums[i] = convertedCol;
break;
}
}
}
}

private static Object[] buildParentConstructorArguments(int[] inputColNums,
VectorExpression parent) {
int extraArgs = parent instanceof DecimalColDivideDecimalScalar ? 2 : 1;
Object[] arguments = new Object[inputColNums.length + extraArgs];
for (int i = 0; i < inputColNums.length; i++) {
arguments[i] = inputColNums[i];
}
int outputIndex = inputColNums.length;
if (parent instanceof DecimalColDivideDecimalScalar) {
arguments[outputIndex++] = ((DecimalColDivideDecimalScalar) parent).getValue();
}
arguments[outputIndex] = parent.getOutputColumnNum();
return arguments;
}

/** Column to match in {@code inputColNums} before replacing with a converted child output. */
private static int getPreConversionColumnNum(VectorExpression child) {
if (child instanceof ConvertDecimal64ToDecimal) {
return child.inputColumnNum[0];
}
return child.getOutputColumnNum();
}

private static void fillInPTFEvaluators(
List<WindowFunctionDef> windowsFunctions,
String[] evaluatorFunctionNames,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
CREATE TABLE test1(col2 varchar(28), col3 decimal(26,6), col4 varchar(28));
CREATE TABLE test2(col4 varchar(28));

INSERT INTO test1 VALUES ('abc', 1.00, 'DEF');
INSERT INTO test2 VALUES ('DEF');

EXPLAIN VECTORIZATION DETAIL
SELECT
aa.int_cost
FROM
(
SELECT
a.col4,
a.col3 * CASE WHEN a.col2 = 'bc' THEN 1.77 ELSE 0.72 END AS int_cost
FROM test1 a
) aa
INNER JOIN (
SELECT col4 FROM test2
) bb ON trim(aa.col4) = trim(bb.col4);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this query be simplified for reproducing the issue?
Does the subqueries and left join contributes to the incorrect result?
Can we use a simpler expression?


SELECT
aa.int_cost
FROM
(
SELECT
a.col4,
a.col3 * CASE WHEN a.col2 = 'bc' THEN 1.77 ELSE 0.72 END AS int_cost
FROM test1 a
) aa
INNER JOIN (
SELECT col4 FROM test2
) bb ON trim(aa.col4) = trim(bb.col4);

DROP TABLE test1;
DROP TABLE test2;
Loading
Loading