Skip to content

HIVE-28329: Fix vectorized col * CASE decimal multiply returning zero #6575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kokila-19 wants to merge 1 commit into
base: master
Choose a base branch
from
+757 −16
Open

HIVE-28329: Fix vectorized col * CASE decimal multiply returning zero #6575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
95 changes: 79 additions & 16 deletions ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,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 @@ -4798,6 +4799,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 @@ -4806,6 +4809,7 @@ public static Operator<? extends OperatorDesc> vectorizeSelectOperator(
// Suppress useless evaluation.
continue;
}
vectorSelectExprIndexForCol[i] = index;
vectorSelectExprs[index++] = ve;
}
if (index < size) {
Expand All @@ -4818,6 +4822,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 +4825 to +4829

@kasakrisz kasakrisz Jul 10, 2026

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

hive/ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java

Line 4804 in 82edd2a

projectedOutputColumns[i] = ve.getOutputColumnNum();

}
} finally {
vContext.freeMarkedScratchColumns();
}
Expand Down Expand Up @@ -4880,7 +4890,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 @@ -4892,20 +4901,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 = rebuildArgumentsForDecimal64(parent, children);
}
// re-instantiate the parent expression with new arguments
VectorExpression newParent = vContext.instantiateExpression(parent.getClass(), parent.getOutputTypeInfo(),
Expand All @@ -4914,14 +4910,81 @@ 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[] rebuildArgumentsForDecimal64(VectorExpression parent,
VectorExpression[] children) {
int[] updatedInputCols = extractInputColumnNums(parent);
replaceConvertedChildColumns(updatedInputCols, children);
return createDecimal64Arguments(updatedInputCols, parent);
}

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

int[] updatedInputCols = new int[inputCount];

@kasakrisz kasakrisz Jul 10, 2026

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.

nit. Why is this called updatedInputCols?
IIUC these are the non -1 column numbers, the extracted ones, right?

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

private static void replaceConvertedChildColumns(int[] updatedInputCols,

@kasakrisz kasakrisz Jul 10, 2026

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.

nit. It is not clear what is replaced to what from the method signature.

VectorExpression[] children) {
for (VectorExpression child : children) {
int sourceCol = getSourceColumnForDecimal64Conversion(child);
int convertedCol = child.getOutputColumnNum();
for (int i = 0; i < updatedInputCols.length; i++) {
if (updatedInputCols[i] == sourceCol) {
updatedInputCols[i] = convertedCol;
break;
}
}
}
}

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

private static int getSourceColumnForDecimal64Conversion(VectorExpression child) {
if (child instanceof ConvertDecimal64ToDecimal) {
return child.inputColumnNum[0];
}
return child.getOutputColumnNum();
}

private static void fillInPTFEvaluators(
List<WindowFunctionDef> windowsFunctions,
String[] evaluatorFunctionNames,
Expand Down
111 changes: 111 additions & 0 deletions ql/src/test/queries/clientpositive/vector_decimal64_col_multiply_case_subquery_join.q
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
SET hive.support.concurrency=TRUE;
SET hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
set hive.cbo.enable=true;
Comment on lines +1 to +3

@kasakrisz kasakrisz Jul 10, 2026

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.

Are these necessary to repro the issue?

  • The tables in the test are external tables AFAIK we don't need a txn manager.
  • CBO is true by default


-- HIVE-28329: subquery + LEFT JOIN + CAST(decimal) with col * CASE returns wrong result (0.00000000)

-- Test tables
create EXTERNAL table test1(
col1 varchar(28),
col2 varchar(28),
col3 decimal(26,6),
col4 varchar(28)
);

insert into test1 values('12345678','abc','1.00','DEF');

create EXTERNAL table test2(
col1 varchar(28),
col2 varchar(28),
col3 decimal(26,6),
col4 varchar(28),
col5 varchar(28)
);

insert into test2 values('12345678','abc','1.00','DEF','22222222');

explain vectorization detail
SELECT
int_cost
FROM
(
SELECT
a.col4,
CAST(
CASE when a.col1 = '12345678' then a.col3 * case when a.col2 = '1' then 1.77 else 0.72 end / 100 / 365 * 10 else a.col3 * 10 / 365 / 100 END AS DECIMAL(26, 9)
) AS int_cost
FROM
test1 a
) aa
LEFT JOIN (
SELECT
col4
FROM
test2
WHERE
col5 = '22222222'
) bb ON trim(aa.col4) = trim(bb.col4);

@kasakrisz kasakrisz Jul 10, 2026

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
int_cost
FROM
(
SELECT
a.col4,
CAST(
CASE when a.col1 = '12345678' then a.col3 * case when a.col2 = '1' then 1.77 else 0.72 end / 100 / 365 * 10 else a.col3 * 10 / 365 / 100 END AS DECIMAL(26, 9)
) AS int_cost
FROM
test1 a
) aa
LEFT JOIN (
SELECT
col4
FROM
test2
WHERE
col5 = '22222222'
) bb ON trim(aa.col4) = trim(bb.col4);


explain vectorization detail
SELECT
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
LEFT JOIN (
SELECT
col4
FROM
test2
) bb ON trim(aa.col4) = trim(bb.col4);
Comment on lines +71 to +88

@kasakrisz kasakrisz Jul 10, 2026

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.

If this simpler query can repro the issue, the other one can be removed. This one might be simplified further: try removing the subqueries and the left join. Or use an inner join if it is still necessary.
Please rename the test file accordingly.



SELECT
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
LEFT JOIN (
SELECT
col4
FROM
test2
) bb ON trim(aa.col4) = trim(bb.col4);

DROP TABLE test1;
DROP TABLE test2;

Loading
Loading