HIVE-28129 Execute statement doesnot report the correct query string information#5155
Conversation
ramesh0201
force-pushed
the
preparequery
branch
from
9a7f3e1 to
a3941a6
Compare
asf-ci-hive
added
tests unstable
tests pending
tests passed
and removed
tests pending
tests unstable
labels
zabetak
left a comment
zabetak
left a comment
There was a problem hiding this comment.
LGTM, just some minor comments/questions but not blocking for the patch.
| Processor Tree: | ||
| ListSink | ||
|
|
||
| PREHOOK: query: prepare pcount from select count(*) from src where key > ? |
There was a problem hiding this comment.
The changes in the .q.out file are the expected ones however there seems to be something missing from the prepare statement implementation. After the changes introduced here the query: prepare pattern does not appear at all in the .q.out files.
Is this normal? Should we log a follow-up ticket?
There was a problem hiding this comment.
Prepare queries doesn't go through the execution phase as per current implementation https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/Driver.java#L166C30-L166C44
So the pre-execution and post-execution hooks are skipped as well, which is why we miss seeing those results in the q.out file. I feel it is ok. Just that we need to use compilation side hooks to test the prepare queries. explain..prepare still shows up in the q.out files.
So I am not sure if we have to create a follow up to test the prepare queries? what is your take?
There was a problem hiding this comment.
Thanks for the explanation. Let's leave it as is at the moment and if really needed we can create a JIRA later ;)
ramesh0201
force-pushed
the
preparequery
branch
from
a3941a6 to
b4b9645
Compare
|
|
Thank you @zabetak for the review |
…information (apache#5155) (Ramesh Kumar reviewed by Stamatis Zampetakis)
3 participants
What changes were proposed in this pull request?
Execute Statement should report the correct query string. Because it inherits the conf from the prepare query's sem object, we need to update the query string.
Why are the changes needed?
Because currently execute queries report the prepare statement as their query string in all the logs.
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?
mvn test -Dtest=TestMiniLlapLocalCliDriver -Dqfile=execute_query_string.q