You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "ryukobayashi (via GitHub)" <gi...@apache.org> on 2026/05/15 06:58:20 UTC
[PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
ryukobayashi opened a new pull request, #6486:
URL: https://github.com/apache/hive/pull/6486
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
In vectorized outer join, `generateOuterNulls()` and `generateOuterNullsRepeatedAll()` set `isNull[i] = true` on scratch columns but leave `vector[i]` untouched. When `hive.vectorized.reuse.scratch.columns=true` (the default), a scratch column slot
freed after an expression evaluation (e.g. `CastStringToLong`) can be reused for the outer join's null-marking column. After `reset()` clears `isNull[]`, the expression overwrites `vector[i]` with a fresh value (e.g. 2025). Later, `generateOuterNulls()`
sets `isNull[i] = true` without clearing `vector[i]`, leaving a stale non-zero value.
Downstream operators such as `ColOrCol` read `vector[i]` directly to distinguish "false" (`== 0`) from "null" (`!= 0`). The stale value causes null rows to be misinterpreted as "true", producing wrong OR/AND/CASE WHEN results.
The fix adds `clearVectorValue()`, called whenever `isNull[i]` is set to `true` in the outer join null-marking paths, zeroing `vector[i]` for all supported column vector types (`LongColumnVector`, `DoubleColumnVector`, `BytesColumnVector`,
`TimestampColumnVector`, `IntervalDayTimeColumnVector`).
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Without the fix, vectorized outer joins silently return wrong results when scratch column reuse is enabled (the default). The bug is non-obvious because it only triggers when a specific combination of conditions is met: a type-casting expression allocates a scratch column that is later reused for the outer join's null-marking column, and the join result is consumed by a boolean operator that reads the raw vector value for null discrimination. Users have no indication that results are wrong; workarounds require disabling vectorization entirely (`hive.vectorization.enabled=false`) or disabling scratch column reuse (`hive.vectorized.reuse.scratch.columns=false`), both of which carry a significant performance cost.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
The existing TestMapJoinOperator suite (17 tests) passes without regression. The bug can also be verified manually with the minimal SQL reproducer below; with the fix applied, the result matches the expected output (C 2026 new, D 2026 new) that was previously only obtainable by disabling vectorization.
```
CREATE TABLE src (k STRING, v STRING);
INSERT INTO src VALUES
('p','1'),('p','2'),('p','3'),
('q','2'),('q','3'),
('r','3'),
('s','3');
WITH base AS (
SELECT k, v FROM src GROUP BY k, v
),
classified AS (
SELECT t1.k, t1.v,
CASE WHEN COALESCE(t2.k,'') = '' THEN 'new'
WHEN COALESCE(t3.k,'') = '' THEN 'two_step'
ELSE 'three_step' END AS status
FROM base t1
LEFT JOIN base t2
ON t1.k = t2.k
AND CAST(t1.v AS INT) - 1 = CAST(t2.v AS INT)
LEFT JOIN base t3
ON t1.k = t3.k
AND CAST(t1.v AS INT) - 2 = CAST(t3.v AS INT)
WHERE CAST(t1.v AS INT) >= 3
GROUP BY t1.k, t1.v,
CASE WHEN COALESCE(t2.k,'') = '' THEN 'new'
WHEN COALESCE(t3.k,'') = '' THEN 'two_step'
ELSE 'three_step' END
)
SELECT * FROM classified WHERE status = 'new';
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4494883167
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_coverage&view=list)
 [1.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3275164445
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
Review Comment:
SQ feedback: empty method body.
```suggestion
public void processBatch(VectorizedRowBatch batch) {
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "soumyakanti3578 (via GitHub)" <gi...@apache.org>.
soumyakanti3578 commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3277782818
##########
storage-api/src/test/org/apache/hadoop/hive/ql/exec/vector/TestBytesColumnVector.java:
##########
@@ -220,4 +222,21 @@ private static byte[] writeToBytesColumnVector(int rowIdx, BytesColumnVector col
col.setValPreallocated(rowIdx, writeSize);
return bytes;
}
+
+ @Test
+ public void testClearValue() {
+ BytesColumnVector cv = new BytesColumnVector(4);
+ byte[] data = "hello".getBytes(StandardCharsets.UTF_8);
+ cv.vector[0] = data;
+ cv.start[0] = 1;
+ cv.length[0] = 3;
+
+ cv.clearValue(0);
+
+ assertTrue(cv.isNull[0]);
+ assertFalse(cv.noNulls);
+ assertNull(cv.vector[0]);
+ assertEquals(0, cv.start[0]);
+ assertEquals(0, cv.length[0]);
+ }
Review Comment:
Neighbor slot was not tested here, whereas it has been tested for other column vectors. Please add it wherever it makes sense.
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
+ }
+
+ /**
+ * Records {@code clearSlotValue} invocations to verify the operator dispatches
+ * through {@code clearValue}, not just produces the slot-clearing side effect.
+ */
+ private static class TrackingLongColumnVector extends LongColumnVector {
+ final List<Integer> clearedIndices = new ArrayList<>();
+
+ TrackingLongColumnVector(int size) {
+ super(size);
+ }
+
+ @Override
+ protected void clearSlotValue(int elementNum) {
+ super.clearSlotValue(elementNum);
+ clearedIndices.add(elementNum);
+ }
+ }
+
+ @Test
+ void generateOuterNullsCallsClearValueOnEachMappedColumnForEachUnmatchedRow() throws HiveException, IOException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1, 2};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(3, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol1 = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol2 = new TrackingLongColumnVector(4);
+ keyCol.vector[1] = 99L;
+ valCol1.vector[1] = 88L;
+ valCol2.vector[3] = 77L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol1;
+ batch.cols[2] = valCol2;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertEquals(Arrays.asList(1, 3), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol1.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol2.clearedIndices);
+
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[1]);
+ assertTrue(keyCol.isNull[3]);
+ assertFalse(keyCol.isNull[0]);
+ assertFalse(keyCol.isNull[2]);
+
+ assertEquals(0L, keyCol.vector[1]);
+ assertEquals(0L, valCol1.vector[1]);
+ assertEquals(0L, valCol2.vector[3]);
+ }
+
+ @Test
+ void generateOuterNullsRepeatedAllCallsClearValueAtIndexZeroForEachMappedColumn() throws HiveException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(2, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol = new TrackingLongColumnVector(4);
+ keyCol.vector[0] = 42L;
+ valCol.vector[0] = 84L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol;
+
+ op.generateOuterNullsRepeatedAll(batch);
+
+ assertEquals(Arrays.asList(0), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(0), valCol.clearedIndices);
+
+ // isRepeating is set by the operator, not by clearValue.
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[0]);
+ assertTrue(keyCol.isRepeating);
+ assertFalse(valCol.noNulls);
+ assertTrue(valCol.isNull[0]);
+ assertTrue(valCol.isRepeating);
+
+ assertEquals(0L, keyCol.vector[0]);
+ assertEquals(0L, valCol.vector[0]);
+ }
+
+ @Test
+ void generateOuterNullsSetsBookkeepingOnTypeWithNoClearSlotValueOverride() throws HiveException, IOException {
+ // VoidColumnVector inherits the base no-op clearSlotValue — verifies the
+ // operator still drives the null-marking through clearValue() on a type
+ // without a per-slot value to zero.
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ VoidColumnVector voidCol = new VoidColumnVector(4);
+ batch.cols[0] = voidCol;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertFalse(voidCol.noNulls);
+ assertTrue(voidCol.isNull[1]);
+ assertTrue(voidCol.isNull[3]);
+ assertFalse(voidCol.isNull[0]);
+ assertFalse(voidCol.isNull[2]);
+ }
+
+ /**
+ * For each {@link ColumnVector} subclass whose {@code clearSlotValue} is
+ * overridden, verifies the operator's call through {@code clearValue} reaches
+ * the override and clears the slot to the type's cleared state.
+ */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("modifiedColumnVectorTypes")
+ void generateOuterNullsClearsSlotForEachModifiedType(
+ String typeName,
+ ColumnVector cv,
+ Runnable preLoad,
+ Runnable assertSlotCleared) throws HiveException, IOException {
+
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ preLoad.run();
+ batch.cols[0] = cv;
+
+ int[] noMatchs = new int[] {2};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertTrue(cv.isNull[2]);
+ assertFalse(cv.noNulls);
+ assertSlotCleared.run();
+ }
+
+ static Stream<Arguments> modifiedColumnVectorTypes() {
+ final LongColumnVector longCv = new LongColumnVector(4);
+ final DoubleColumnVector doubleCv = new DoubleColumnVector(4);
+ final BytesColumnVector bytesCv = new BytesColumnVector(4);
+ final DecimalColumnVector decCv = new DecimalColumnVector(4, 18, 4);
+ final TimestampColumnVector tsCv = new TimestampColumnVector(4);
+ final IntervalDayTimeColumnVector ivCv = new IntervalDayTimeColumnVector(4);
+
Review Comment:
I saw that there's also the `Decimal64ColumnVector` class. Maybe we need to include it here and other places where it makes sense?
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
+ }
+
+ /**
+ * Records {@code clearSlotValue} invocations to verify the operator dispatches
+ * through {@code clearValue}, not just produces the slot-clearing side effect.
+ */
+ private static class TrackingLongColumnVector extends LongColumnVector {
+ final List<Integer> clearedIndices = new ArrayList<>();
+
+ TrackingLongColumnVector(int size) {
+ super(size);
+ }
+
+ @Override
+ protected void clearSlotValue(int elementNum) {
+ super.clearSlotValue(elementNum);
+ clearedIndices.add(elementNum);
+ }
+ }
+
+ @Test
+ void generateOuterNullsCallsClearValueOnEachMappedColumnForEachUnmatchedRow() throws HiveException, IOException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1, 2};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(3, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol1 = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol2 = new TrackingLongColumnVector(4);
+ keyCol.vector[1] = 99L;
+ valCol1.vector[1] = 88L;
+ valCol2.vector[3] = 77L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol1;
+ batch.cols[2] = valCol2;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertEquals(Arrays.asList(1, 3), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol1.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol2.clearedIndices);
+
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[1]);
+ assertTrue(keyCol.isNull[3]);
+ assertFalse(keyCol.isNull[0]);
+ assertFalse(keyCol.isNull[2]);
+
+ assertEquals(0L, keyCol.vector[1]);
+ assertEquals(0L, valCol1.vector[1]);
+ assertEquals(0L, valCol2.vector[3]);
+ }
+
+ @Test
+ void generateOuterNullsRepeatedAllCallsClearValueAtIndexZeroForEachMappedColumn() throws HiveException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(2, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol = new TrackingLongColumnVector(4);
+ keyCol.vector[0] = 42L;
+ valCol.vector[0] = 84L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol;
+
+ op.generateOuterNullsRepeatedAll(batch);
+
+ assertEquals(Arrays.asList(0), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(0), valCol.clearedIndices);
+
+ // isRepeating is set by the operator, not by clearValue.
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[0]);
+ assertTrue(keyCol.isRepeating);
+ assertFalse(valCol.noNulls);
+ assertTrue(valCol.isNull[0]);
+ assertTrue(valCol.isRepeating);
+
+ assertEquals(0L, keyCol.vector[0]);
+ assertEquals(0L, valCol.vector[0]);
+ }
+
+ @Test
+ void generateOuterNullsSetsBookkeepingOnTypeWithNoClearSlotValueOverride() throws HiveException, IOException {
+ // VoidColumnVector inherits the base no-op clearSlotValue — verifies the
+ // operator still drives the null-marking through clearValue() on a type
+ // without a per-slot value to zero.
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ VoidColumnVector voidCol = new VoidColumnVector(4);
+ batch.cols[0] = voidCol;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertFalse(voidCol.noNulls);
+ assertTrue(voidCol.isNull[1]);
+ assertTrue(voidCol.isNull[3]);
+ assertFalse(voidCol.isNull[0]);
+ assertFalse(voidCol.isNull[2]);
+ }
+
+ /**
+ * For each {@link ColumnVector} subclass whose {@code clearSlotValue} is
+ * overridden, verifies the operator's call through {@code clearValue} reaches
+ * the override and clears the slot to the type's cleared state.
+ */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("modifiedColumnVectorTypes")
+ void generateOuterNullsClearsSlotForEachModifiedType(
+ String typeName,
+ ColumnVector cv,
+ Runnable preLoad,
+ Runnable assertSlotCleared) throws HiveException, IOException {
+
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ preLoad.run();
+ batch.cols[0] = cv;
+
+ int[] noMatchs = new int[] {2};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
Review Comment:
I think we need a similar parameterized test for `generateOuterNullsRepeatedAll`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "ryukobayashi (via GitHub)" <gi...@apache.org>.
ryukobayashi commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4483975360
@konstantinb Thanks, I accept your proposal because it makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "soumyakanti3578 (via GitHub)" <gi...@apache.org>.
soumyakanti3578 commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3250455968
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/VectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -749,13 +754,32 @@ protected void generateOuterNullsRepeatedAll(VectorizedRowBatch batch) throws Hi
colVector.noNulls = false;
colVector.isNull[0] = true;
colVector.isRepeating = true;
+ clearVectorValue(colVector, 0);
}
for (int column : smallTableValueColumnMap) {
ColumnVector colVector = batch.cols[column];
colVector.noNulls = false;
colVector.isNull[0] = true;
colVector.isRepeating = true;
+ clearVectorValue(colVector, 0);
+ }
+ }
+
+ private static void clearVectorValue(ColumnVector colVector, int index) {
+ if (colVector instanceof LongColumnVector) {
+ ((LongColumnVector) colVector).vector[index] = 0L;
+ } else if (colVector instanceof DoubleColumnVector) {
+ ((DoubleColumnVector) colVector).vector[index] = 0.0;
+ } else if (colVector instanceof BytesColumnVector) {
+ BytesColumnVector bcv = (BytesColumnVector) colVector;
+ bcv.vector[index] = null;
+ bcv.start[index] = 0;
+ bcv.length[index] = 0;
+ } else if (colVector instanceof TimestampColumnVector) {
+ ((TimestampColumnVector) colVector).setNullValue(index);
+ } else if (colVector instanceof IntervalDayTimeColumnVector) {
+ ((IntervalDayTimeColumnVector) colVector).setNullValue(index);
Review Comment:
Can you please confirm that these are the only `ColumnVector`s that can appear in `smallTableValueColumnMap` and `outerSmallTableKeyColumnMap`?
Maybe we should do the same for all `ColumnVector` types, and it would be better to do this in the individual classes instead of handling it here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3275082551
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
Review Comment:
SQ: empty method body.
```suggestion
protected String getLoggingPrefix() {
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
}
```
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
Review Comment:
SQ: empty method body.
```suggestion
public void processBatch(VectorizedRowBatch batch) {
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3275082588
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
Review Comment:
SQ: empty method body.
```suggestion
public void processBatch(VectorizedRowBatch batch) {
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
}
```
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
Review Comment:
SQ: empty method body.
```suggestion
protected String getLoggingPrefix() {
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3275158846
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
Review Comment:
SQ feedback: empty/trivial stub body.
```suggestion
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "ryukobayashi (via GitHub)" <gi...@apache.org>.
ryukobayashi commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3279425003
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
+ }
+
+ /**
+ * Records {@code clearSlotValue} invocations to verify the operator dispatches
+ * through {@code clearValue}, not just produces the slot-clearing side effect.
+ */
+ private static class TrackingLongColumnVector extends LongColumnVector {
+ final List<Integer> clearedIndices = new ArrayList<>();
+
+ TrackingLongColumnVector(int size) {
+ super(size);
+ }
+
+ @Override
+ protected void clearSlotValue(int elementNum) {
+ super.clearSlotValue(elementNum);
+ clearedIndices.add(elementNum);
+ }
+ }
+
+ @Test
+ void generateOuterNullsCallsClearValueOnEachMappedColumnForEachUnmatchedRow() throws HiveException, IOException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1, 2};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(3, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol1 = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol2 = new TrackingLongColumnVector(4);
+ keyCol.vector[1] = 99L;
+ valCol1.vector[1] = 88L;
+ valCol2.vector[3] = 77L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol1;
+ batch.cols[2] = valCol2;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertEquals(Arrays.asList(1, 3), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol1.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol2.clearedIndices);
+
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[1]);
+ assertTrue(keyCol.isNull[3]);
+ assertFalse(keyCol.isNull[0]);
+ assertFalse(keyCol.isNull[2]);
+
+ assertEquals(0L, keyCol.vector[1]);
+ assertEquals(0L, valCol1.vector[1]);
+ assertEquals(0L, valCol2.vector[3]);
+ }
+
+ @Test
+ void generateOuterNullsRepeatedAllCallsClearValueAtIndexZeroForEachMappedColumn() throws HiveException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(2, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol = new TrackingLongColumnVector(4);
+ keyCol.vector[0] = 42L;
+ valCol.vector[0] = 84L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol;
+
+ op.generateOuterNullsRepeatedAll(batch);
+
+ assertEquals(Arrays.asList(0), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(0), valCol.clearedIndices);
+
+ // isRepeating is set by the operator, not by clearValue.
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[0]);
+ assertTrue(keyCol.isRepeating);
+ assertFalse(valCol.noNulls);
+ assertTrue(valCol.isNull[0]);
+ assertTrue(valCol.isRepeating);
+
+ assertEquals(0L, keyCol.vector[0]);
+ assertEquals(0L, valCol.vector[0]);
+ }
+
+ @Test
+ void generateOuterNullsSetsBookkeepingOnTypeWithNoClearSlotValueOverride() throws HiveException, IOException {
+ // VoidColumnVector inherits the base no-op clearSlotValue — verifies the
+ // operator still drives the null-marking through clearValue() on a type
+ // without a per-slot value to zero.
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ VoidColumnVector voidCol = new VoidColumnVector(4);
+ batch.cols[0] = voidCol;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertFalse(voidCol.noNulls);
+ assertTrue(voidCol.isNull[1]);
+ assertTrue(voidCol.isNull[3]);
+ assertFalse(voidCol.isNull[0]);
+ assertFalse(voidCol.isNull[2]);
+ }
+
+ /**
+ * For each {@link ColumnVector} subclass whose {@code clearSlotValue} is
+ * overridden, verifies the operator's call through {@code clearValue} reaches
+ * the override and clears the slot to the type's cleared state.
+ */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("modifiedColumnVectorTypes")
+ void generateOuterNullsClearsSlotForEachModifiedType(
+ String typeName,
+ ColumnVector cv,
+ Runnable preLoad,
+ Runnable assertSlotCleared) throws HiveException, IOException {
+
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ preLoad.run();
+ batch.cols[0] = cv;
+
+ int[] noMatchs = new int[] {2};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
Review Comment:
I don't think it's absolutely necessary, but I'll add it anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4508570434
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_coverage&view=list)
 [1.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4483165061
@ryukobayashi could you please take a look at my suggestions to your approach: https://github.com/ryukobayashi/hive/pull/1 ?
The PR is a bit better reproducing the actual problem and incorporates multiple suggestions from @soumyakanti3578
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "ryukobayashi (via GitHub)" <gi...@apache.org>.
ryukobayashi commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4494977772
@konstantinb Merged. If the corrections are minor, I think it's okay to do them just to be safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4484218008
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_coverage&view=list)
 [1.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "ryukobayashi (via GitHub)" <gi...@apache.org>.
ryukobayashi commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3279610056
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
+ }
+
+ /**
+ * Records {@code clearSlotValue} invocations to verify the operator dispatches
+ * through {@code clearValue}, not just produces the slot-clearing side effect.
+ */
+ private static class TrackingLongColumnVector extends LongColumnVector {
+ final List<Integer> clearedIndices = new ArrayList<>();
+
+ TrackingLongColumnVector(int size) {
+ super(size);
+ }
+
+ @Override
+ protected void clearSlotValue(int elementNum) {
+ super.clearSlotValue(elementNum);
+ clearedIndices.add(elementNum);
+ }
+ }
+
+ @Test
+ void generateOuterNullsCallsClearValueOnEachMappedColumnForEachUnmatchedRow() throws HiveException, IOException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1, 2};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(3, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol1 = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol2 = new TrackingLongColumnVector(4);
+ keyCol.vector[1] = 99L;
+ valCol1.vector[1] = 88L;
+ valCol2.vector[3] = 77L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol1;
+ batch.cols[2] = valCol2;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertEquals(Arrays.asList(1, 3), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol1.clearedIndices);
+ assertEquals(Arrays.asList(1, 3), valCol2.clearedIndices);
+
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[1]);
+ assertTrue(keyCol.isNull[3]);
+ assertFalse(keyCol.isNull[0]);
+ assertFalse(keyCol.isNull[2]);
+
+ assertEquals(0L, keyCol.vector[1]);
+ assertEquals(0L, valCol1.vector[1]);
+ assertEquals(0L, valCol2.vector[3]);
+ }
+
+ @Test
+ void generateOuterNullsRepeatedAllCallsClearValueAtIndexZeroForEachMappedColumn() throws HiveException {
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {0};
+ op.smallTableValueColumnMap = new int[] {1};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(2, 4);
+ TrackingLongColumnVector keyCol = new TrackingLongColumnVector(4);
+ TrackingLongColumnVector valCol = new TrackingLongColumnVector(4);
+ keyCol.vector[0] = 42L;
+ valCol.vector[0] = 84L;
+ batch.cols[0] = keyCol;
+ batch.cols[1] = valCol;
+
+ op.generateOuterNullsRepeatedAll(batch);
+
+ assertEquals(Arrays.asList(0), keyCol.clearedIndices);
+ assertEquals(Arrays.asList(0), valCol.clearedIndices);
+
+ // isRepeating is set by the operator, not by clearValue.
+ assertFalse(keyCol.noNulls);
+ assertTrue(keyCol.isNull[0]);
+ assertTrue(keyCol.isRepeating);
+ assertFalse(valCol.noNulls);
+ assertTrue(valCol.isNull[0]);
+ assertTrue(valCol.isRepeating);
+
+ assertEquals(0L, keyCol.vector[0]);
+ assertEquals(0L, valCol.vector[0]);
+ }
+
+ @Test
+ void generateOuterNullsSetsBookkeepingOnTypeWithNoClearSlotValueOverride() throws HiveException, IOException {
+ // VoidColumnVector inherits the base no-op clearSlotValue — verifies the
+ // operator still drives the null-marking through clearValue() on a type
+ // without a per-slot value to zero.
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ VoidColumnVector voidCol = new VoidColumnVector(4);
+ batch.cols[0] = voidCol;
+
+ int[] noMatchs = new int[] {1, 3};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertFalse(voidCol.noNulls);
+ assertTrue(voidCol.isNull[1]);
+ assertTrue(voidCol.isNull[3]);
+ assertFalse(voidCol.isNull[0]);
+ assertFalse(voidCol.isNull[2]);
+ }
+
+ /**
+ * For each {@link ColumnVector} subclass whose {@code clearSlotValue} is
+ * overridden, verifies the operator's call through {@code clearValue} reaches
+ * the override and clears the slot to the type's cleared state.
+ */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("modifiedColumnVectorTypes")
+ void generateOuterNullsClearsSlotForEachModifiedType(
+ String typeName,
+ ColumnVector cv,
+ Runnable preLoad,
+ Runnable assertSlotCleared) throws HiveException, IOException {
+
+ TestableOuterOp op = new TestableOuterOp();
+ op.outerSmallTableKeyColumnMap = new int[] {};
+ op.smallTableValueColumnMap = new int[] {0};
+
+ VectorizedRowBatch batch = new VectorizedRowBatch(1, 4);
+ preLoad.run();
+ batch.cols[0] = cv;
+
+ int[] noMatchs = new int[] {2};
+ op.generateOuterNulls(batch, noMatchs, noMatchs.length);
+
+ assertTrue(cv.isNull[2]);
+ assertFalse(cv.noNulls);
+ assertSlotCleared.run();
+ }
+
+ static Stream<Arguments> modifiedColumnVectorTypes() {
+ final LongColumnVector longCv = new LongColumnVector(4);
+ final DoubleColumnVector doubleCv = new DoubleColumnVector(4);
+ final BytesColumnVector bytesCv = new BytesColumnVector(4);
+ final DecimalColumnVector decCv = new DecimalColumnVector(4, 18, 4);
+ final TimestampColumnVector tsCv = new TimestampColumnVector(4);
+ final IntervalDayTimeColumnVector ivCv = new IntervalDayTimeColumnVector(4);
+
Review Comment:
Decimal64ColumnVector extends LongColumnVector and inherits its clearSlotValue override (which zeros vector[elementNum]), so the production code already handles it correctly without changes. Added it to both modifiedColumnVectorTypes and modifiedColumnVectorTypesAtSlotZero parameterized test streams to make the coverage explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4478732262
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "soumyakanti3578 (via GitHub)" <gi...@apache.org>.
soumyakanti3578 merged PR #6486:
URL: https://github.com/apache/hive/pull/6486
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3275164445
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
Review Comment:
SQ feedback: empty method body.
```suggestion
public void processBatch(VectorizedRowBatch batch) {
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4458257879
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4493242164
@ryukobayashi one more tweak: https://github.com/ryukobayashi/hive/pull/2 which addresses SQ feedback and trims excessive comments/hive config flags the test uses.
The main module still seems to have at least three code paths unfixed, but those are, apparently, impossible to hit with current CBO rewrites. I am unsure if we should leave those as they are now or fix preemptively
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "konstantinb (via GitHub)" <gi...@apache.org>.
konstantinb commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3275171038
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/TestVectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.
+ */
+
+package org.apache.hadoop.hive.ql.exec.vector.mapjoin;
+
+import org.apache.hadoop.hive.common.type.HiveIntervalDayTime;
+import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.IntervalDayTimeColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
+import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.VoidColumnVector;
+import org.apache.hadoop.hive.ql.metadata.HiveException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * HIVE-29598: verifies {@link VectorMapJoinOuterGenerateResultOperator} clears
+ * every small-table slot for unmatched rows, so stale values cannot carry over
+ * past the null marking.
+ */
+class TestVectorMapJoinOuterGenerateResultOperator {
+
+ /** Concrete subclass that exposes the generateOuterNulls* methods to tests. */
+ private static final class TestableOuterOp extends VectorMapJoinOuterGenerateResultOperator {
+ @Override
+ protected String getLoggingPrefix() {
+ return "test";
+ }
+
+ @Override
+ public void processBatch(VectorizedRowBatch batch) {
+ }
Review Comment:
SQ feedback: empty method body.
```suggestion
throw new UnsupportedOperationException("stubbed only to instantiate abstract class under test");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6486:
URL: https://github.com/apache/hive/pull/6486#issuecomment-4506091943
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6486&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_coverage&view=list)
 [1.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6486&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6486)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "ryukobayashi (via GitHub)" <gi...@apache.org>.
ryukobayashi commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3279624077
##########
storage-api/src/test/org/apache/hadoop/hive/ql/exec/vector/TestBytesColumnVector.java:
##########
@@ -220,4 +222,21 @@ private static byte[] writeToBytesColumnVector(int rowIdx, BytesColumnVector col
col.setValPreallocated(rowIdx, writeSize);
return bytes;
}
+
+ @Test
+ public void testClearValue() {
+ BytesColumnVector cv = new BytesColumnVector(4);
+ byte[] data = "hello".getBytes(StandardCharsets.UTF_8);
+ cv.vector[0] = data;
+ cv.start[0] = 1;
+ cv.length[0] = 3;
+
+ cv.clearValue(0);
+
+ assertTrue(cv.isNull[0]);
+ assertFalse(cv.noNulls);
+ assertNull(cv.vector[0]);
+ assertEquals(0, cv.start[0]);
+ assertEquals(0, cv.length[0]);
+ }
Review Comment:
Added pre-loaded data to the neighboring slot (index 1) and asserted after clearValue(0) that its vector, start, length, and isNull are all unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29598: Fix vectorized outer join wrong results due to stale scratch column values [hive]
Posted by "ryukobayashi (via GitHub)" <gi...@apache.org>.
ryukobayashi commented on code in PR #6486:
URL: https://github.com/apache/hive/pull/6486#discussion_r3257671809
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/VectorMapJoinOuterGenerateResultOperator.java:
##########
@@ -749,13 +754,32 @@ protected void generateOuterNullsRepeatedAll(VectorizedRowBatch batch) throws Hi
colVector.noNulls = false;
colVector.isNull[0] = true;
colVector.isRepeating = true;
+ clearVectorValue(colVector, 0);
}
for (int column : smallTableValueColumnMap) {
ColumnVector colVector = batch.cols[column];
colVector.noNulls = false;
colVector.isNull[0] = true;
colVector.isRepeating = true;
+ clearVectorValue(colVector, 0);
+ }
+ }
+
+ private static void clearVectorValue(ColumnVector colVector, int index) {
+ if (colVector instanceof LongColumnVector) {
+ ((LongColumnVector) colVector).vector[index] = 0L;
+ } else if (colVector instanceof DoubleColumnVector) {
+ ((DoubleColumnVector) colVector).vector[index] = 0.0;
+ } else if (colVector instanceof BytesColumnVector) {
+ BytesColumnVector bcv = (BytesColumnVector) colVector;
+ bcv.vector[index] = null;
+ bcv.start[index] = 0;
+ bcv.length[index] = 0;
+ } else if (colVector instanceof TimestampColumnVector) {
+ ((TimestampColumnVector) colVector).setNullValue(index);
+ } else if (colVector instanceof IntervalDayTimeColumnVector) {
+ ((IntervalDayTimeColumnVector) colVector).setNullValue(index);
Review Comment:
Thanks. I added qtest and unit test.
The investigation confirms that `DecimalColumnVector` can also appear (for DECIMAL columns without DECIMAL_64 physical variation), so we've added handling for it. Regarding moving the logic into individual classes: ColumnVector has 15 concrete subclasses including container types (`StructColumnVector`, `ListColumnVector`, `MapColumnVector`, `UnionColumnVecto`r) and `VoidColumnVector`, for which per-slot clearing has no well-defined semantics. Only two classes currently define `setNullValue()`. Adding an abstract method to the base class would be a breaking change to storage-api with disproportionate scope. We prefer to keep the dispatch self-contained in `clearVectorValue` where it directly addresses the bug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org