HIVE-29495: [HPLSQL]Setting hive configs in hplsql causing console logs to be lost#6361
HIVE-29495: [HPLSQL]Setting hive configs in hplsql causing console logs to be lost#6361mdayakar wants to merge 1 commit intoapache:masterfrom
Conversation
|
I'm new to HPL, wondering why |
Here all statements present inside 'scriptText' variable will be consider as one statement so only once logging context will be registred for all and at the end the logging context will be unregistered. For HPLSQL the delimiter is /(forward slash) compared to ;(semicolon) for normal SQL statements so for executing HPLSQL statements in beeline we need to pass / at the end so that all statements will be passed to HPLSQL engine once. |
soumyakanti3578
left a comment
soumyakanti3578
left a comment
There was a problem hiding this comment.
I am approving this as the PR can be merged without addressing the nit (since it was not introduced in this PR).
| } | ||
|
|
||
| public ExecuteStatementOperation(HiveSession parentSession, String statement, Map<String, String> confOverlay, boolean runInBackground, boolean generateNewQueryId) { | ||
| public ExecuteStatementOperation(HiveSession parentSession, String statement, Map<String, String> confOverlay, boolean generateNewQueryId) { |
There was a problem hiding this comment.
nit: Do you think the variable generateNewQueryId is confusing? Everywhere else in its hierarchy the variable is named embedded, i.e., in Operation and in this PR you're adding embedded to HiveCommandOperation and ShowProcessListOperation too. Although, I think the name embedded itself is quite confusing, but at least it is used consistently, except here.
I am adding just a nit here because this was not introduced in this PR, but if this bugs you too please rename this.
P.S.: What do you think about renaming embedded to isHplSqlMode?
There was a problem hiding this comment.
Thanks for the review @soumyakanti3578 . Changed the variable generateNewQueryId to embedded to match with other places. I feel embedded is ok here as it denotes the current operation is getting executed as embedded operation of another operation so don't want to change to isHplSqlMode even though its added as a part of HPLSQL integration within HS2. Hope that is ok.
mdayakar
force-pushed
the
HIVE-29495-HPLSQLIssue
branch
from
179c2f0 to
ab4623a
Compare
|
5 participants
HIVE-29495: [HPLSQL]Setting hive configs in hplsql causing console logs to be lost.
What changes were proposed in this pull request?
When SET hive config is executed, it is executed as a separate HiveCommand which is registrering a new logging context so it is creating a new operation log and after executing SET hive config it is unregistering the logging context which closes the operation log so for all the statements executed after this are not getting logged into operation log so in beeline console nothing is getting showed. Now we are not registering logging context as a part of SET hive config statement, which results in logging into the existing operation log so the logs are coming for all statements executed.
Why are the changes needed?
Without fix, after executing SET hive config there will not be any console logs on the beeline console.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Testcase (testPrintMessageAfterExecuteSetHiveConfig) is added in the TestHplSqlViaBeeLine class
mvn test -Dtest=TestHplSqlViaBeeLine#testPrintMessageAfterExecuteSetHiveConfig -pl itests/hive-unit -Pitests