Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201702211810.v1LIAvKL015262@ip-10-146-233-104.ec2.internal>
Date: Tue, 21 Feb 2017 18:10:57 +0000
From: "Dan Hecht (Code Review)" <gerrit@cloudera.org>
To: Michael Ho <kwho@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Alex Behm <alex.behm@cloudera.com>
Reply-To: dhecht@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4821=3A_Update_AVG=28=29_for_DECIMAL_V2=0A?=
In-Reply-To: <gerrit.1487265008000.I28f5ef0370938440eb5b1c6d29b2f24e6f88499f@gerrit.cloudera.org>
References: <gerrit.1487265008000.I28f5ef0370938440eb5b1c6d29b2f24e6f88499f@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: Tue, 21 Feb 2017 18:11:01 -0000

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4821: Update AVG() for DECIMAL_V2
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6038/4//COMMIT_MSG
Commit Message:

PS4, Line 25: types
values

i.e. we can compute AVG() over DECIMAL(38,0) as long as the absolute values are less than 10^32. I think this is more common than actually having values larger than 10^32 when the column was the result of some decimal arithmetic that produced a type that's "too precise".


http://gerrit.cloudera.org:8080/#/c/6038/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

PS4, Line 338: Our implementation is similar to MS SQL which takes the max of the input's scale
             :       // and MIN_ADJUSTED_SCALE.
How about explaining why we deviate from SQL Server and from SUM()/COUNT():

Scale is set the same as MS SQL Server, which takes the max of the input scale and MIN_ADJUST_SCALE.  For precision, MS SQL always sets it to 38. Instead, trim it down to the size that's needed given that the absolute value of the result must be less than the absolute value of the largest input. Using a smaller precision allows for better DECIMAL types to be chosen for the overall expression when AVG() is a subexpression.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I28f5ef0370938440eb5b1c6d29b2f24e6f88499f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-HasComments: Yes