From issues-return-48545-archive-asf-public=cust-asf.ponee.io@drill.apache.org Tue Jan 9 19:53:06 2018 Return-Path: X-Original-To: archive-asf-public@eu.ponee.io Delivered-To: archive-asf-public@eu.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by mx-eu-01.ponee.io (Postfix) with ESMTP id 615A5180718 for ; Tue, 9 Jan 2018 19:53:06 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 562AB160C3F; Tue, 9 Jan 2018 18:53:06 +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 9C3D1160C17 for ; Tue, 9 Jan 2018 19:53:05 +0100 (CET) Received: (qmail 95480 invoked by uid 500); 9 Jan 2018 18:53:04 -0000 Mailing-List: contact issues-help@drill.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@drill.apache.org Delivered-To: mailing list issues@drill.apache.org Received: (qmail 95471 invoked by uid 99); 9 Jan 2018 18:53:04 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 09 Jan 2018 18:53:04 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 67D5C1A0E44 for ; Tue, 9 Jan 2018 18:53:04 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -99.911 X-Spam-Level: X-Spam-Status: No, score=-99.911 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, USER_IN_WHITELIST=-100] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id IyUs793cs5I2 for ; Tue, 9 Jan 2018 18:53:03 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id EA2495F24E for ; Tue, 9 Jan 2018 18:53:02 +0000 (UTC) Received: from jira-lw-us.apache.org (unknown [207.244.88.139]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 6D164E0F4F for ; Tue, 9 Jan 2018 18:53:02 +0000 (UTC) Received: from jira-lw-us.apache.org (localhost [127.0.0.1]) by jira-lw-us.apache.org (ASF Mail Server at jira-lw-us.apache.org) with ESMTP id 23A88212F8 for ; Tue, 9 Jan 2018 18:53:02 +0000 (UTC) Date: Tue, 9 Jan 2018 18:53:02 +0000 (UTC) From: "ASF GitHub Bot (JIRA)" To: issues@drill.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Commented] (DRILL-4834) decimal implementation is vulnerable to overflow errors, and extremely complex MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/DRILL-4834?page=3Dcom.atlassian= .jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D1631= 8924#comment-16318924 ]=20 ASF GitHub Bot commented on DRILL-4834: --------------------------------------- Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/570#discussion_r160458242 =20 --- Diff: exec/java-exec/src/main/codegen/templates/Decimal/CastDecimal= Varchar.java --- @@ -150,6 +150,14 @@ public void setup() { =20 public void eval() { =20 +<#if type.from.contains("VarDecimal")> + java.math.BigDecimal bigDecimal =3D org.apache.drill.exec.util= .DecimalUtility.getBigDecimalFromDrillBuf(in.buffer, in.start, in.end - in.= start, in.scale); + String str =3D bigDecimal.toString(); + out.buffer =3D buffer; + out.start =3D 0; + out.end =3D Math.min((int)len.value, str.length()); + out.buffer.setBytes(0, str.getBytes()); --- End diff -- =20 What about the case when `str` has a length greater than `len`? I think= it would be better to use `setBytes(int index, byte[] src, int srcIndex, i= nt length)` method here. > decimal implementation is vulnerable to overflow errors, and extremely co= mplex > -------------------------------------------------------------------------= ----- > > Key: DRILL-4834 > URL: https://issues.apache.org/jira/browse/DRILL-4834 > Project: Apache Drill > Issue Type: Bug > Components: Execution - Data Types > Affects Versions: 1.6.0 > Environment: Drill 1.7 on any platform > Reporter: Dave Oshinsky > Fix For: Future > > > While working on a fix for DRILL-4704, logic was added to CastIntDecimal.= java template to handle the situation where a precision is not supplied (i.= e., the supplied precision is zero) for an integer value that is to be cast= ed to a decimal. The Drill decimal implementation uses a limited selection= of fixed decimal precision data types (the total number of decimal digits,= i.e., Decimal9, 18, 28, 38) to represent decimal values. If the destinati= on precision is too small to represent the input integer that is being cast= ed, there is no clean way to deal with the overflow error properly. > While using fixed decimal precisions as is being done currently can lead = to more efficient use of memory, it often will actually lead to less effici= ent use of memory (when the fixed precision is specified significantly larg= er than is actually needed to represent the numbers), and it results in a t= remendous mushrooming of the complexity of the code. For each fixed precis= ion (and there are only a limited set of selections, 9, 18, 28, 38, which i= tself leads to memory inefficiency), there is a separate set of code genera= ted from templates. For each pairwise combination of decimal or non-decima= l numeric types, there are multiple places in the code where conversions mu= st be handled, or conditions must be included to handle the difference in p= recision between the two types. A one-size-fits-all approach (using a vari= able width vector to represent any decimal precision) would usually be more= memory-efficient (since precisions are often over-specified), and would gr= eatly simplify the code. > Also see the DRILL-4184 issue, which is related. -- This message was sent by Atlassian JIRA (v6.4.14#64029)