Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
......................................................................


Patch Set 8: Code-Review+1

(2 comments)

> Thrift is still changing this operator<< in 0.11, adding enum
 > types. If we patch it now we may need to rework it on every thrift
 > upgrade. Generally I think it's unnecessary to patch a library only
 > for a name conflict.

Works for me.

http://gerrit.cloudera.org:8080/#/c/9168/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9168/8//COMMIT_MSG@13
PS8, Line 13:
Since this touches a lot of stuff, have you done a full test run with it?


http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc@64
PS8, Line 64:     DCHECK(host_ == instance_params->host);  // all hosts must be 
the same
> DCHECK_EQ uses operator << to print the value of lhs and rhs. Operator << i
Could you mention this in the commit message?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Feb 2018 23:03:41 +0000
Gerrit-HasComments: Yes

Reply via email to