You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/08/03 00:28:20 UTC
[GitHub] [hive] yigress opened a new pull request, #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
yigress opened a new pull request, #3492:
URL: https://github.com/apache/hive/pull/3492
<!--
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.
-->
### 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.
-->
### 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'.
-->
### 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.
-->
--
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
[GitHub] [hive] yigress commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
yigress commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r938380996
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[i]: keyLength[keyOffset];
Review Comment:
added test cases of repeating multiple keys and mixed keys.
--
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
[GitHub] [hive] maswin commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by "maswin (via GitHub)" <gi...@apache.org>.
maswin commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r1200967679
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[0]: keyLength[keyOffset];
+ byte[] rowKey = isRepeating? keyVector[0]: keyVector[keyOffset];
if (StringExpr.equal(key, 0, key.length,
- keyVector[keyOffset], keyStart[keyOffset], keyLength[keyOffset])) {
+ rowKey, keyStart[keyOffset], len)) {
Review Comment:
keyStart[keyOffset] should also be assigned based on isRepeating key.
--
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
[GitHub] [hive] prasanthj commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
prasanthj commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1206664084
+1
--
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
[GitHub] [hive] prasanthj commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
prasanthj commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r938270084
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[i]: keyLength[keyOffset];
Review Comment:
Agree with Gopal. If isRepeating=true, then keyLength and keyVector will have to be looked at index = 0. I was also expecting something like below
```
final int len = isRepeating? keyLength[0]: keyLength[keyOffset];
```
--
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
[GitHub] [hive] sunchao commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
sunchao commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1206758080
Test failures unrelated. Merging now.
--
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
[GitHub] [hive] jfsii commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
jfsii commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1211284480
> sorry about this. thank you @sunchao @jfsii !
No problem @yigress - it is an easy mistake to make. Thanks for your contribution to the project!
--
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
[GitHub] [hive] jfsii commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
jfsii commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1207234052
I've attempted to fix the situation - https://github.com/apache/hive/pull/3509
--
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
[GitHub] [hive] jfsii commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
jfsii commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1207224851
Hi @sunchao / @yigress - I think some of the test failures could be related to this patch.
http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-3492/3/pipeline/439 (which is from the run on this PR)
[2022-08-05T04:08:23.465Z] [ERROR] org.apache.hadoop.hive.cli.split16.TestMiniLlapLocalCliDriver.testCliDriver[orc_map_key_repeating] Time elapsed: 3.725 s <<< FAILURE!
[2022-08-05T04:08:23.465Z] java.lang.AssertionError:
[2022-08-05T04:08:23.465Z] Client Execution succeeded but contained differences (error code = 2) after executing orc_map_key_repeating.q
[2022-08-05T04:08:23.465Z] See ./ql/target/tmp/log/hive.log or ./itests/qtest/target/tmp/log/hive.log, or check ./ql/target/surefire-reports or ./itests/qtest/target/surefire-reports/ for specific test cases logs.
[2022-08-05T04:08:23.465Z] at org.junit.Assert.fail(Assert.java:89)
This PR directly added orc_map_key_repeating.
--
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
[GitHub] [hive] yigress commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
yigress commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r938282246
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[i]: keyLength[keyOffset];
Review Comment:
Thank you for the review @prasanthj ! updated. I only see isRepeating is true for one single key, but not sure if there can be multiple keys and still have isRepeating=true
--
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
[GitHub] [hive] yigress commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
yigress commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1205952601
updated with test cases of repeating with multiple keys and mixed keys.
--
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
[GitHub] [hive] sunchao merged pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
sunchao merged PR #3492:
URL: https://github.com/apache/hive/pull/3492
--
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
[GitHub] [hive] yigress commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
yigress commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r938987928
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[i]: keyLength[keyOffset];
Review Comment:
@prasanthj appreciate your review!
--
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
[GitHub] [hive] sunchao commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
sunchao commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1207237039
@jfsii oops sorry, I'll take a look at the fix
--
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
[GitHub] [hive] prasanthj commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
prasanthj commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r938288613
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[i]: keyLength[keyOffset];
Review Comment:
Could you update the test case with multiple keys and see if that produces expected result?
--
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
[GitHub] [hive] t3rmin4t0r commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
t3rmin4t0r commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r936143948
##########
pom.xml:
##########
@@ -58,7 +58,7 @@
<module>testutils</module>
<module>packaging</module>
<module>standalone-metastore</module>
- <module>upgrade-acid</module>
Review Comment:
Remove the pom.xml from the PR
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[i]: keyLength[keyOffset];
Review Comment:
Is that supposed to be [i] or [0]
The two rows look a bit wrong there.
I would write keyOffset = isRepeating ? 0 : offset + i
if the intention was to use =0 for all repeating.
--
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
[GitHub] [hive] yigress commented on a diff in pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
yigress commented on code in PR #3492:
URL: https://github.com/apache/hive/pull/3492#discussion_r936209369
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/VectorUDFMapIndexStringScalar.java:
##########
@@ -66,10 +66,13 @@ public int findScalarInMap(MapColumnVector mapColumnVector, int mapBatchIndex) {
byte[][] keyVector = keyColVector.vector;
int[] keyStart = keyColVector.start;
int[] keyLength = keyColVector.length;
+ final boolean isRepeating = keyColVector.isRepeating;
for (int i = 0; i < count; i++) {
final int keyOffset = offset + i;
+ final int len = isRepeating? keyLength[i]: keyLength[keyOffset];
Review Comment:
I found that the key is set isRepeating only when there is a single key (count=1), but I am not sure if different orc versions may have multiple keys, if there are multiple keys in repeating, keyLength may have multiple entries. Would it be better just deal with the isRepeating and count=1 separately?
--
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
[GitHub] [hive] yigress commented on pull request #3492: HIVE-26447: Vectorization: wrong results when filter on repeating map key orc table
Posted by GitBox <gi...@apache.org>.
yigress commented on PR #3492:
URL: https://github.com/apache/hive/pull/3492#issuecomment-1209774357
sorry about this. thank you @sunchao @jfsii !
--
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