Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 7DD99200C39 for ; Thu, 2 Mar 2017 00:28:47 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 7C73B160B78; Wed, 1 Mar 2017 23:28:47 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id C407F160B70 for ; Thu, 2 Mar 2017 00:28:46 +0100 (CET) Received: (qmail 24812 invoked by uid 500); 1 Mar 2017 23:28:46 -0000 Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list reviews@impala.incubator.apache.org Received: (qmail 24798 invoked by uid 99); 1 Mar 2017 23:28:45 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 01 Mar 2017 23:28:45 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 5E402C05AA for ; Wed, 1 Mar 2017 23:28:45 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.362 X-Spam-Level: X-Spam-Status: No, score=0.362 tagged_above=-999 required=6.31 tests=[RDNS_DYNAMIC=0.363, SPF_PASS=-0.001] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id bfXo1UlYoSb9 for ; Wed, 1 Mar 2017 23:28:43 +0000 (UTC) Received: from ip-10-146-233-104.ec2.internal (ec2-75-101-130-251.compute-1.amazonaws.com [75.101.130.251]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id 7CD7D5F1EE for ; Wed, 1 Mar 2017 23:28:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by ip-10-146-233-104.ec2.internal (8.14.4/8.14.4) with ESMTP id v21NSfvF001226; Wed, 1 Mar 2017 23:28:41 GMT Message-Id: <201703012328.v21NSfvF001226@ip-10-146-233-104.ec2.internal> Date: Wed, 1 Mar 2017 23:28:41 +0000 From: "Zach Amsden (Code Review)" To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org CC: Dan Hecht , Michael Ho Reply-To: zamsden@cloudera.com X-Gerrit-MessageType: comment Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4813=3A_Round_on_divide_and_multiply=0A?= X-Gerrit-Change-Id: Ie6bfcbe37555b74598d409c6f84f06b0ae5c4312 X-Gerrit-ChangeURL: X-Gerrit-Commit: 3353115a7cc17add5a5c643e9c4995c16109bd9b In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/2.12.7 archived-at: Wed, 01 Mar 2017 23:28:47 -0000 Zach Amsden has posted comments on this change. Change subject: IMPALA-4813: Round on divide and multiply ...................................................................... Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/6132/10/be/src/runtime/types.h File be/src/runtime/types.h: Line 147: int min_scale = std::min(scale, +MIN_ADJUSTED_SCALE); I decided to copy this from the Java code in the frontend as it looked useful. In the spirit of letting no good deed go unpunished, the compiler has rewarded me with the little gift of converting MIN_ADJUSTED_SCALE into an lvalue, as std::min takes its values by reference. What is our standard approach to tackling this? Provide definitions in the .cc file? (Fortunately in this cases, there is one). -- To view, visit http://gerrit.cloudera.org:8080/6132 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie6bfcbe37555b74598d409c6f84f06b0ae5c4312 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes