HIVE-28329: Fix vectorized col * CASE decimal multiply returning zero#6575
HIVE-28329: Fix vectorized col * CASE decimal multiply returning zero#6575kokila-19 wants to merge 1 commit into
Conversation
kokila-19
changed the title
asf-ci-hive
added
tests pending
tests failed
and removed
tests passed
tests pending
labels
asf-ci-hive
added
tests pending
tests passed
and removed
tests failed
tests pending
labels
Preserve inputColumnNum column refs when re-instantiating DECIMAL parents after DECIMAL_64 child conversion and refresh projectedOutputColumns.
asf-ci-hive
added
tests pending
tests unstable
and removed
tests passed
tests pending
tests unstable
labels
|
| test2 | ||
| WHERE | ||
| col5 = '22222222' | ||
| ) bb ON trim(aa.col4) = trim(bb.col4); |
There was a problem hiding this comment.
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?
| SET hive.support.concurrency=TRUE; | ||
| SET hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager; | ||
| set hive.cbo.enable=true; |
There was a problem hiding this comment.
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
| 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); |
There was a problem hiding this comment.
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.
| for (int i = 0; i < size; i++) { | ||
| int exprIndex = vectorSelectExprIndexForCol[i]; | ||
| if (exprIndex >= 0) { | ||
| projectedOutputColumns[i] = vectorSelectExprs[exprIndex].getOutputColumnNum(); | ||
| } |
There was a problem hiding this comment.
Why is this change necessary? I found that projectedOutputColumns is set a few lines earlier here
| } | ||
| } | ||
|
|
||
| int[] updatedInputCols = new int[inputCount]; |
There was a problem hiding this comment.
nit. Why is this called updatedInputCols?
IIUC these are the non -1 column numbers, the extracted ones, right?
| return updatedInputCols; | ||
| } | ||
|
|
||
| private static void replaceConvertedChildColumns(int[] updatedInputCols, |
There was a problem hiding this comment.
nit. It is not clear what is replaced to what from the method signature.
3 participants
What changes were proposed in this pull request?
Fix HIVE-28329 in Vectorizer.java: preserve column inputs when re-instantiating DECIMAL multiply after DECIMAL_64→DECIMAL conversion and refresh projectedOutputColumns.
Added qtest vector_decimal64_col_multiply_case_subquery_join.q.
Why are the changes needed?
With CBO and vectorization enabled, expressions like col3 * CASE(...) are vectorized with the CASE as DECIMAL_64 and the multiply expecting DECIMAL. When fixDecimalDataTypePhysicalVariations() wraps the CASE with ConvertDecimal64ToDecimal and re-builds the multiply, the old code only used child expressions. So, col3 in inputColumnNum[] was dropped and a stale scratch column (often 0) was used instead.
That led to wrong multiply inputs and stale projectedOutputColumns, so HIVE-28329 returned 0.000000000 instead of 0.000197260.
Disabling vectorization avoided this broken path.
Does this PR introduce any user-facing change?
Yes, a bug fix. Affected queries now return correct decimal results. No new config or syntax.
How was this patch tested?
qtest: vector_decimal64_col_multiply_case_subquery_join.q
Manual test: EXPLAIN before/after to confirm multiply wiring col2, col19 vs col19, col9