impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-6210: Add query id to lineage graph logging
Date Wed, 22 Nov 2017 00:11:21 GMT
Philip Zeyliger has posted comments on this change. (

Change subject: IMPALA-6210: Add query id to lineage graph logging

Patch Set 2: Code-Review+1


This looks good to me. Thanks for adding the tests.

The Thrift comments below are largely philosophical, though they wouldn't be in the case of
the query profile.
File common/thrift/LineageGraph.thrift:
PS1, Line 55:   3: required string user
> Done. Does this mean ideally we shouldn't change those numbers or removing 
In the strictest sense, you'd never re-use the numbers. You can remove an existing field today,
just don't re-use the number. See
for more on this stuff, but we're not as affected by it here.

(In something like the Impala Runtime Profile, we are affected by this: we serialize that
to log files which tools read, and it'd be bad to break these rules there.)
File common/thrift/LineageGraph.thrift:
PS2, Line 69:   8: required Types.TUniqueId query_id
It doesn't actually matter because we don't serialize these, but 'required' typically means
that this will fail to parse an old serialized TLineageGraph from previous versions of Impala.
I think your usage here is consistent with how we use Thrift here, so I think it's likely

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4adbd02df37a234dbb79f58b7c46ca11a914229f
Gerrit-Change-Number: 8589
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-Reviewer: Tianyi Wang <>
Gerrit-Comment-Date: Wed, 22 Nov 2017 00:11:21 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message