Github user daveoshinsky commented on a diff in the pull request:
https://github.com/apache/drill/pull/517#discussion_r70468695
--- Diff:
exec/java-exec/src/main/codegen/templates/Decimal/CastIntDecimal.java ---
@@ -64,7 +64,15 @@ public void setup() {
public void eval() {
out.scale = (int) scale.value;
- out.precision = (int) precision.value;
--- End diff --
The DRILL-4704 bug root cause was that there is no supplied precision in
this case (casting an int to a decimal), i.e., the "supplied precision" was a
bogus value of 0, and the casted decimal with precision 0 matched nothing
(unless perhaps the value was 0, but I didn't try that). Â The most direct way
I could fix this was to compute the precision, which just involves a few
integer divides by 10 basically. Â It might be possible to assume a large
enough precision (say 38, to fit most any integer), but that would waste lots
of space (to hold a much smaller value than worst case precision of 38 can
hold). Â Moreover, whenever one assumes a "worst case", eventually it's likely
to hit a "worse than worst case" (remember fixed length char buffers in C? Â
128, no 256, no 1024, and so on). Â It's been a while since I ran this (and it
was difficult to debug because of the runtime code generation and compilation),
but it may be the case that the cast from int to decimal is done only once
(to prepare a decimal for comparison, just once), in which case the divisions
by 10 really wouldn't make any difference.
On Tuesday, July 12, 2016 11:13 AM, Aman Sinha
<[email protected]> wrote:
In exec/java-exec/src/main/codegen/templates/Decimal/CastIntDecimal.java:>
@@ -64,7 +64,15 @@ public void setup() {
>
> public void eval() {
> out.scale = (int) scale.value;
> - out.precision = (int) precision.value;
I am not too familiar with the decimal type but taking a closer look at the
fix I don't think we should ignore the supplied precision. Your fix will
compute the precision in all cases, which would be a performance overhead. It
should be computing only when supplied precision is 0. Thoughts ? â
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
---
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.
---
