Return-Path: X-Original-To: apmail-drill-dev-archive@www.apache.org Delivered-To: apmail-drill-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7DBE818C43 for ; Sun, 6 Mar 2016 21:23:27 +0000 (UTC) Received: (qmail 34386 invoked by uid 500); 6 Mar 2016 21:23:27 -0000 Delivered-To: apmail-drill-dev-archive@drill.apache.org Received: (qmail 34330 invoked by uid 500); 6 Mar 2016 21:23:27 -0000 Mailing-List: contact dev-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 dev@drill.apache.org Received: (qmail 34318 invoked by uid 99); 6 Mar 2016 21:23:27 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 06 Mar 2016 21:23:27 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id CA679DFC56; Sun, 6 Mar 2016 21:23:26 +0000 (UTC) From: daveoshinsky To: dev@drill.apache.org Reply-To: dev@drill.apache.org References: In-Reply-To: Subject: [GitHub] drill pull request: DRILL-4184: support variable length decimal fi... Content-Type: text/plain Message-Id: <20160306212326.CA679DFC56@git1-us-west.apache.org> Date: Sun, 6 Mar 2016 21:23:26 +0000 (UTC) Github user daveoshinsky commented on the pull request: https://github.com/apache/drill/pull/372#issuecomment-192999951 Regarding the overall intent of the fix, as the "TODO" comment on decimalLengths implies, it's intended only as a short-term fix. More long-term, I would suggest that decimal values should be stored in a VariableWidthVector (which was assumed by VarLenghValuesColumn, hence the class cast exception). This would use memory more efficiently when most values are far smaller than full precision, as is often the case (think java.math.BigDecimal, which operates this way). Moreover, there would be no need to have a whole bunch of separate (generated) classes for different decimal precisions. Just one class, variable width, handling any precision. I also suggest that some other "special cases" could be combined. Fixed width is a special case of variable width, where there's no need to store a separate length for each value. Non-nullable is a special case of nullable, where there's no need to store a nullable boolean (or equivalent) for each value. One last bit of feedback - it wou ld be much easier to maintain the code if it did not involve generation of code (freemarker). An old-fashioned class hierarchy, with no generated code, would probably work just fine for the vectoring mechanisms. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastructure@apache.org or file a JIRA ticket with INFRA. ---