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 2021/12/16 15:48:21 UTC
[GitHub] [hive] kasakrisz opened a new pull request #2888: HIVE-25818: Values query with order by position clause fails
kasakrisz opened a new pull request #2888:
URL: https://github.com/apache/hive/pull/2888
<!--
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?
When verifying the column index defined in the order by clause use the column count stored in the project's input row resolver if the ouput RR is null
### Why are the changes needed?
`NullPointerException` was thrown because in case of `values` queries there is no output row resolver of the project.
### Does this PR introduce _any_ user-facing change?
Such queries don't fail with exception but provides a result.
### How was this patch tested?
```
mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestMiniLlapLocalCliDriver -Dqfile=order_by_pos.q -pl itests/qtest -Pitests
```
--
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] kasakrisz closed pull request #2888: HIVE-25818: Values query with order by position clause fails
Posted by GitBox <gi...@apache.org>.
kasakrisz closed pull request #2888:
URL: https://github.com/apache/hive/pull/2888
--
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] kasakrisz commented on a change in pull request #2888: HIVE-25818: Values query with order by position clause fails
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on a change in pull request #2888:
URL: https://github.com/apache/hive/pull/2888#discussion_r773246018
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -4025,7 +4025,9 @@ private OBLogicalPlanGenState beginGenOBLogicalPlan(
// replace each of the position alias in ORDERBY with the actual column
if (ref != null && ref.getToken().getType() == HiveParser.Number) {
if (isObyByPos) {
- fieldIndex = getFieldIndexFromColumnNumber(selectOutputRR, ref);
Review comment:
Here is a different approach: return the output RR of the UDTF when transforming to `HiveTableFunctionScan`
Could you please review? #2899
--
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] kasakrisz commented on a change in pull request #2888: HIVE-25818: Values query with order by position clause fails
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on a change in pull request #2888:
URL: https://github.com/apache/hive/pull/2888#discussion_r772311718
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -4025,7 +4025,9 @@ private OBLogicalPlanGenState beginGenOBLogicalPlan(
// replace each of the position alias in ORDERBY with the actual column
if (ref != null && ref.getToken().getType() == HiveParser.Number) {
if (isObyByPos) {
- fieldIndex = getFieldIndexFromColumnNumber(selectOutputRR, ref);
Review comment:
HIVE-24830 doesn't solve this.
When generating the Calcite plan of values queries the source of the `Order by` is a `HiveTableFunctionScan` instead of a `Project`.
The `values` clause is converted to a function call and as a result we end up with a `HiveTableFunctionScan`
https://github.com/apache/hive/blob/c39cc56e28172397d573ca9072f55551fef86a56/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java#L4606
This case the output RR is null:
https://github.com/apache/hive/blob/c39cc56e28172397d573ca9072f55551fef86a56/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java#L4870
--
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] kgyrtkirk commented on a change in pull request #2888: HIVE-25818: Values query with order by position clause fails
Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #2888:
URL: https://github.com/apache/hive/pull/2888#discussion_r772325885
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -4025,7 +4025,9 @@ private OBLogicalPlanGenState beginGenOBLogicalPlan(
// replace each of the position alias in ORDERBY with the actual column
if (ref != null && ref.getToken().getType() == HiveParser.Number) {
if (isObyByPos) {
- fieldIndex = getFieldIndexFromColumnNumber(selectOutputRR, ref);
Review comment:
yes; I know that will not solve this - the general problem is that going to the `input` instead of the current one could be a mistake
Then I suppose we are getting the `outputRel` from [here](https://github.com/apache/hive/blob/c39cc56e28172397d573ca9072f55551fef86a56/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java#L4794) can't we fix the RR of that UDTF node - instead of returning `null` there and falling back to the `input` 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
[GitHub] [hive] kasakrisz commented on pull request #2888: HIVE-25818: Values query with order by position clause fails
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on pull request #2888:
URL: https://github.com/apache/hive/pull/2888#issuecomment-999283160
Solved by #2899
--
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] kgyrtkirk commented on a change in pull request #2888: HIVE-25818: Values query with order by position clause fails
Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #2888:
URL: https://github.com/apache/hive/pull/2888#discussion_r772241455
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -4025,7 +4025,9 @@ private OBLogicalPlanGenState beginGenOBLogicalPlan(
// replace each of the position alias in ORDERBY with the actual column
if (ref != null && ref.getToken().getType() == HiveParser.Number) {
if (isObyByPos) {
- fieldIndex = getFieldIndexFromColumnNumber(selectOutputRR, ref);
Review comment:
is it ok that `selectOutputRR` is null?
these RR things are a bit messed up...I wanted to fix things in the vicinity in https://issues.apache.org/jira/browse/HIVE-24830; so I think that could become a source of problems...
--
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] kgyrtkirk commented on a change in pull request #2888: HIVE-25818: Values query with order by position clause fails
Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #2888:
URL: https://github.com/apache/hive/pull/2888#discussion_r772242865
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -4025,7 +4025,9 @@ private OBLogicalPlanGenState beginGenOBLogicalPlan(
// replace each of the position alias in ORDERBY with the actual column
if (ref != null && ref.getToken().getType() == HiveParser.Number) {
if (isObyByPos) {
- fieldIndex = getFieldIndexFromColumnNumber(selectOutputRR, ref);
Review comment:
if you see no other hazards which may come out from that being `null` we can get this in - but is it true that if the RR is `null` - then the select is an identity?
--
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