Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201702222159.v1MLxDgB004706@ip-10-146-233-104.ec2.internal>
Date: Wed, 22 Feb 2017 21:59:13 +0000
From: "anujphadke (Code Review)" <gerrit@cloudera.org>
To: anujphadke <aphadke@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Alex Behm <alex.behm@cloudera.com>
Reply-To: aphadke@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4848=3A_Add_WIDHT_BUCKET=28=29_function=0A?=
In-Reply-To: <gerrit.1487202122000.I081bc916b1bef7b929ca161a9aade3b54c6b858f@gerrit.cloudera.org>
References: <gerrit.1487202122000.I081bc916b1bef7b929ca161a9aade3b54c6b858f@gerrit.cloudera.org>
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, 22 Feb 2017 21:59:17 -0000

anujphadke has posted comments on this change.

Change subject: IMPALA-4848: Add WIDHT_BUCKET() function
......................................................................


Patch Set 2:

(7 comments)

Please review this change after  I push a new patch which addresses some overflow bugs.

http://gerrit.cloudera.org:8080/#/c/6023/1/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

Line 427:     const IntVal& num_buckets) {
> do all null checks in one if
Done


Line 429:     return IntVal::null();
> use min_range.val directly because the == operator will check for null agai
Done


Line 430:   }
> Lower...
Done


Line 434:   }
> should -> must
Done


http://gerrit.cloudera.org:8080/#/c/6023/2/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

Line 455:   double bucket_width = (max_range.val - min_range.val) / num_buckets.val;
This can overflow. Working on handling this.
select width_bucket(1.5e+200, -1.7e+308, 1.2e+308, 900);


Line 457:   int64_t result = static_cast<int64_t>((expr.val - min_range.val) / bucket_width) + 1;
This can overflow. Working on handling this.


http://gerrit.cloudera.org:8080/#/c/6023/1/be/src/exprs/math-functions.h
File be/src/exprs/math-functions.h:

Line 128:   /// Returns true if no parse_res == PARSE_SUCCESS || parse_res == PARSE_OVERFLOW.
> Move in the public section like the other functions.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <aphadke@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: anujphadke <aphadke@cloudera.com>
Gerrit-HasComments: Yes