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 [email protected] or file a JIRA ticket
with INFRA.
---
