Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12301 )

Change subject: Logging current database context when logging analyzing queries.
......................................................................


Patch Set 1:

(1 comment)

This is a one-line change, so of course I have comments...

http://gerrit.cloudera.org:8080/#/c/12301/1/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/12301/1/fe/src/main/java/org/apache/impala/service/Frontend.java@1243
PS1, Line 1243:         + queryCtx.session.database);
In Java, a general rule is to use accessor methods, but since this is Thrift, I 
suppose it is not essential.

Verified that both fields are required, so seems no worry about NPE.

The statement can be huge. With this formatting you'll get:

<prefix> Analyzing query SELECT ...
...
... db: foo

Would it make sense to log this as two lines? Or maybe:

<prefix> Analyzing query for db: foo\n
SELECT ...
...
...



--
To view, visit http://gerrit.cloudera.org:8080/12301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70886be4685e0d3ae7ca9f6e57e8159dc39c67c7
Gerrit-Change-Number: 12301
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Paul Rogers <[email protected]>
Gerrit-Comment-Date: Thu, 31 Jan 2019 22:19:44 +0000
Gerrit-HasComments: Yes

Reply via email to