[ 
https://issues.apache.org/jira/browse/DRILL-4834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16321054#comment-16321054
 ] 

ASF GitHub Bot commented on DRILL-4834:
---------------------------------------

Github user daveoshinsky commented on a diff in the pull request:

    https://github.com/apache/drill/pull/570#discussion_r160792114
  
    --- Diff: 
exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java ---
    @@ -159,9 +159,20 @@ public static BigDecimal 
getBigDecimalFromSparse(DrillBuf data, int startIndex,
         }
     
         public static BigDecimal getBigDecimalFromDrillBuf(DrillBuf bytebuf, 
int start, int length, int scale) {
    +      if (length <= 0) {
    +        // if the length is somehow non-positive, interpret this as zero
    +        //System.out.println("getBigDecimal forces 0 with start " + start 
+ " len " + length);
    +          try {
    +              throw new Exception("hi there");
    --- End diff --
    
    Yes, I'll remove the friendly "hi there".  It was for debugging, I guess.


> decimal implementation is vulnerable to overflow errors, and extremely complex
> ------------------------------------------------------------------------------
>
>                 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 casted 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 destination 
> precision is too small to represent the input integer that is being casted, 
> 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 efficient 
> use of memory (when the fixed precision is specified significantly larger 
> than is actually needed to represent the numbers), and it results in a 
> tremendous mushrooming of the complexity of the code.  For each fixed 
> precision (and there are only a limited set of selections, 9, 18, 28, 38, 
> which itself leads to memory inefficiency), there is a separate set of code 
> generated from templates.  For each pairwise combination of decimal or 
> non-decimal numeric types, there are multiple places in the code where 
> conversions must be handled, or conditions must be included to handle the 
> difference in precision between the two types.  A one-size-fits-all approach 
> (using a variable width vector to represent any decimal precision) would 
> usually be more memory-efficient (since precisions are often over-specified), 
> and would greatly simplify the code.
> Also see the DRILL-4184 issue, which is related.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to