HIVE-29262: Incorrect column ordering output in case of different ordering of mutual columns in query & window function in vectorized PTF#6512
Conversation
62d8db8 to
ca4ac35
Compare
| LOAD DATA LOCAL INPATH '../../data/files/web_sales_2k' OVERWRITE INTO TABLE web_sales_txt; | ||
| select ws_bill_customer_sk,ws_item_sk from web_sales_txt; | ||
|
|
||
| SET hive.vectorized.execution.enabled; |
There was a problem hiding this comment.
Why is this line needed? hive.vectorized.execution.enabled is set to true on line 1.
| ws_sold_date_sk, | ||
| ws_sales_price, | ||
| LAG(ws_sales_price) OVER ( | ||
| PARTITION BY ws_item_sk,ws_bill_customer_sk |
There was a problem hiding this comment.
Please add a space between ws_item_sk,ws_bill_customer_sk
a5e3bfc to
5abfd2c
Compare
|
|
Thanks for contribution, @tanishq-chugh ! |
…ering of mutual columns in query & window function in vectorized PTF (apache#6512) (cherry picked from commit 46927a2)
|
This PR, is also fixing 1 additional issue of Attaching the q file for the repro without this PR. vector_ptf_spill_partition_order_overlap.sql Adding this here for reference if someone faces this issue on older hive version :-) |
|
@Aggarwal-Raghav is #6579 addressing the same ? |
| int[] nonKeyInputColumnMap = ArrayUtils.toPrimitive(nonKeyInputColumns.toArray(new Integer[0])); | ||
|
|
||
| if (isPartitionOrderBy && partitionKeyCount > 1) { | ||
| reorderPartitionColumnsToMatchOutputOrder(outputSignature, evaluatorCount, outputColumnProjectionMap, |
There was a problem hiding this comment.
TBH, i am not sure it's a right place for the fix.
I think the PARTITION-BY-key restoration bug should be fixed in VectorPTFOperator/VectorPTFGroupBatches — map partitionKey[i] to keyWithoutOrderColumnMap positions by column number at the point of use — rather than reordering VectorPTFInfo in the Vectorizer
There was a problem hiding this comment.
Yes, that is what I was thinking, I'll check if vector_ptf_spill_partition_order_overlap.sql and vector_ptf_cols_order.q are passing after that.
There was a problem hiding this comment.
Subject: [PATCH] patch
---
Index: ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java (revision 1b1fa6d6a00f38ff8968bea673b5006615cbc2b3)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java (date 1783678424148)
@@ -349,6 +349,7 @@
bufferedTypeInfos,
orderColumnMap,
keyWithoutOrderColumnMap,
+ partitionColumnMap,
overflowBatch);
isFirstPartition = true;
Index: ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java (revision 1b1fa6d6a00f38ff8968bea673b5006615cbc2b3)
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java (date 1783678635446)
@@ -26,6 +26,7 @@
import java.util.List;
import java.util.stream.Collectors;
+import org.apache.commons.lang3.ArrayUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
import org.apache.hadoop.hive.ql.exec.PTFPartition;
@@ -64,6 +65,7 @@
private int[] bufferedColumnMap;
private int[] orderColumnMap;
private int[] keyWithoutOrderColumnMap;
+ private int[] partitionKeyIndices;
private int spillLimitBufferedBatchCount;
private String spillLocalDirs;
@@ -289,13 +291,21 @@
public void init(VectorPTFEvaluatorBase[] evaluators, int[] outputProjectionColumnMap,
int[] bufferedColumnMap, TypeInfo[] bufferedTypeInfos, int[] orderColumnMap,
- int[] keyWithoutOrderColumnMap, VectorizedRowBatch overflowBatch) {
+ int[] keyWithoutOrderColumnMap, int[] partitionColumnMap, VectorizedRowBatch overflowBatch) {
this.evaluators = evaluators;
this.outputProjectionColumnMap = outputProjectionColumnMap;
this.bufferedColumnMap = bufferedColumnMap;
this.orderColumnMap = orderColumnMap;
this.keyWithoutOrderColumnMap = keyWithoutOrderColumnMap;
+ partitionKeyIndices = new int[keyWithoutOrderColumnMap.length];
+ for (int i = 0; i < keyWithoutOrderColumnMap.length; i++) {
+ partitionKeyIndices[i] = ArrayUtils.indexOf(partitionColumnMap, keyWithoutOrderColumnMap[i]);
+ Preconditions.checkState(partitionKeyIndices[i] != ArrayUtils.INDEX_NOT_FOUND,
+ "Key input column %s not found among partition columns %s",
+ keyWithoutOrderColumnMap[i], Arrays.toString(partitionColumnMap));
+ }
+
this.overflowBatch = overflowBatch;
bufferedBatches = new ArrayList<BufferedVectorizedRowBatch>(0);
@@ -775,6 +785,11 @@
}
}
+ /**
+ * Sets the partition key values as repeating columns in the overflowBatch. Key columns are in
+ * output projection order, partitionKey values in PARTITION BY order; partitionKeyIndices maps
+ * between the two.
+ */
private void copyPartitionColumnToOverflow(Object[] partitionKey) {
// Set partition column in overflowBatch.
// We can set by ref since our last batch is held by us.
@@ -782,7 +797,7 @@
for (int i = 0; i < keyInputColumnCount; i++) {
final int keyColumnNum = keyWithoutOrderColumnMap[i];
Preconditions.checkState(overflowBatch.cols[keyColumnNum] != null);
- setRepeatingColumn(partitionKey[i], overflowBatch, keyColumnNum);
+ setRepeatingColumn(partitionKey[partitionKeyIndices[i]], overflowBatch, keyColumnNum);
}
}
Index: ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java b/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java
--- a/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java (revision 1b1fa6d6a00f38ff8968bea673b5006615cbc2b3)
+++ b/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java (date 1783678472285)
@@ -588,6 +588,7 @@
/* bufferedTypeInfos */ new TypeInfo[] { getTypeInfo("int"), getTypeInfo("string") },
/* orderColumnMap */ new int[] { 1 }, // p_date
/* keyWithoutOrderColumnMap */ new int[] { 0 }, // p_mfgr
+ /* partitionColumnMap */ new int[] { 0 }, // p_mfgr
getFakeOperator().setupOverflowBatch(3, new String[] { "bigint", "bigint" },
outputProjectionColumnMap, outputTypeInfos));
}
There was a problem hiding this comment.
@deniskuzZ I came up with this. Will check the best of both and will raise PR.
diff --git i/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java w/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java
index bd60d215ff..1288703402 100644
--- i/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java
+++ w/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFGroupBatches.java
@@ -64,6 +64,7 @@ public class VectorPTFGroupBatches extends PTFPartition {
private int[] bufferedColumnMap;
private int[] orderColumnMap;
private int[] keyWithoutOrderColumnMap;
+ private int[] partitionColumnMap;
private int spillLimitBufferedBatchCount;
private String spillLocalDirs;
@@ -289,12 +290,13 @@ public VectorPTFGroupBatches(Configuration hconf, int vectorizedPTFMaxMemoryBuff
public void init(VectorPTFEvaluatorBase[] evaluators, int[] outputProjectionColumnMap,
int[] bufferedColumnMap, TypeInfo[] bufferedTypeInfos, int[] orderColumnMap,
- int[] keyWithoutOrderColumnMap, VectorizedRowBatch overflowBatch) {
+ int[] keyWithoutOrderColumnMap, int[] partitionColumnMap, VectorizedRowBatch overflowBatch) {
this.evaluators = evaluators;
this.outputProjectionColumnMap = outputProjectionColumnMap;
this.bufferedColumnMap = bufferedColumnMap;
this.orderColumnMap = orderColumnMap;
this.keyWithoutOrderColumnMap = keyWithoutOrderColumnMap;
+ this.partitionColumnMap = partitionColumnMap;
this.overflowBatch = overflowBatch;
bufferedBatches = new ArrayList<BufferedVectorizedRowBatch>(0);
@@ -772,8 +774,16 @@ private void copyPartitionColumnToOverflow(Object[] partitionKey) {
final int keyInputColumnCount = keyWithoutOrderColumnMap.length;
for (int i = 0; i < keyInputColumnCount; i++) {
final int keyColumnNum = keyWithoutOrderColumnMap[i];
+ int partitionKeyIndex = -1;
+ for (int j = 0; j < partitionColumnMap.length; j++) {
+ if (partitionColumnMap[j] == keyColumnNum) {
+ partitionKeyIndex = j;
+ break;
+ }
+ }
+ Preconditions.checkState(partitionKeyIndex != -1, "Could not find keyColumnNum " + keyColumnNum + " in partitionColumnMap");
Preconditions.checkState(overflowBatch.cols[keyColumnNum] != null);
- setRepeatingColumn(partitionKey[i], overflowBatch, keyColumnNum);
+ setRepeatingColumn(partitionKey[partitionKeyIndex], overflowBatch, keyColumnNum);
}
}
diff --git i/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java w/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java
index 26987a68f4..feb12617de 100644
--- i/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java
+++ w/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java
@@ -348,6 +348,7 @@ protected void initializeOp(Configuration hconf) throws HiveException {
bufferedTypeInfos,
orderColumnMap,
keyWithoutOrderColumnMap,
+ vectorPTFInfo.getPartitionColumnMap(),
overflowBatch);
isFirstPartition = true;
diff --git i/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java w/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java
index 3039585817..c74d407c14 100644
--- i/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java
+++ w/ql/src/test/org/apache/hadoop/hive/ql/exec/vector/ptf/TestVectorPTFGroupBatches.java
@@ -588,6 +588,7 @@ private void init(VectorPTFGroupBatches groupBatches) throws HiveException {
/* bufferedTypeInfos */ new TypeInfo[] { getTypeInfo("int"), getTypeInfo("string") },
/* orderColumnMap */ new int[] { 1 }, // p_date
/* keyWithoutOrderColumnMap */ new int[] { 0 }, // p_mfgr
+ /* partitionColumnMap */ new int[] { 0 },
getFakeOperator().setupOverflowBatch(3, new String[] { "bigint", "bigint" },
outputProjectionColumnMap, outputTypeInfos));
}
There was a problem hiding this comment.
@Aggarwal-Raghav looks similar to mine (i've used ArrayUtils.indexOf) :) please, go ahead. maybe reuse same jira id just add addendum in commit msg
final int partitionKeyIndex = ArrayUtils.indexOf(partitionColumnMap, keyColumnNum);
Preconditions.checkState(partitionKeyIndex != ArrayUtils.INDEX_NOT_FOUND,
"Could not find keyColumnNum %s in partitionColumnMap", keyColumnNum);
No, #6579 is different issue and still open on master branch |



What changes were proposed in this pull request?
Reorder partition columns so they match the key input column sequence during Partitioned Table Function (PTF) execution
Why are the changes needed?
To ensure, the query column ordering is maintained in case of mutual columns in query and PTF having different ordering
Does this PR introduce any user-facing change?
Yes, the column ordering in the query now matches as the input column sequence provided by the user
How was this patch tested?
Manual Testing + added Qtest