impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pranay Singh (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-1649 Pass precision to Print TCounterType::DOUBLE VALUE
Date Thu, 14 Sep 2017 01:11:47 GMT
Pranay Singh has posted comments on this change.

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

Patch Set 2:

(1 comment)
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
To unsubscribe, visit

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 <>
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

View raw message