Pranay Singh has posted comments on this change.

Change subject: IMPALA-1649 Pass precision to Print TCounterType::DOUBLE_VALUE
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7725/2/be/src/util/pretty-printer.h
File be/src/util/pretty-printer.h:

PS2, Line 142:       case TUnit::DOUBLE_VALUE: {
             :         double output = static_cast<double>(value);
             :         ss << std::setprecision(precision) << output << " ";
> I think TUnit::DOUBLE_VALUE is so similar to TUnit::UNIT that we should be 
I thought of removing TUnit::DOUBLE_VALUE from the code and replacing it with 
TUnit::UNIT but was perplexed by two cases;

a) TUnit::UNIT only obeys precision for larger values: It conditionally sets 
the precision for large values greater than 1000, so how should a caller 
display a smaller value in floating point representation restricted to two 
decimal places(or any determined by precision). Say this can be done by setting 
the precision argument but the second issue is more confounding.

b) Printing different variables of TUnit::UNIT type with different precision: 
Use of common function like RuntimeProfile::PrintChildCounters() to display 
variables of same type with different precision makes it hard to use 
TUnit::UNIT type, it appears that TUnit::DOUBLE is a way to Print some 
variables with smaller value to be restricted to 2 decimal places (or any 
precision) and the others to be displayed without any restriction.

Hence I thought that TUnit::DOUBLE_VALUE cannot be removed, so  made changes 
for it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc63d8a59bdf175341097df143798fb1a957d93f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

Reply via email to