Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12132 )

Change subject: Fix some warnings on GCC7
......................................................................


Patch Set 2:

(1 comment)

Thanks for doing this, it would be great to get things working on gcc7.

My one thought is that the ambiguous else fixes are non-obvious and break from 
our usual style. Should we have comments explaining why they are the way they 
are?

Beyond that, I'm ready to +2 this.

http://gerrit.cloudera.org:8080/#/c/12132/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/12132/2/be/src/exprs/decimal-operators-ir.cc@585
PS2, Line 585:   DCHECK(result == StringParser::PARSE_SUCCESS
             :       || result == StringParser::PARSE_UNDERFLOW);
This one is particularly non-obvious.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I39a12bc5ed6957c147b7f0dba85c7687cc989439
Gerrit-Change-Number: 12132
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Comment-Date: Thu, 03 Jan 2019 17:55:14 +0000
Gerrit-HasComments: Yes

Reply via email to