impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Taras Bobrovytsky (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-2.6.0_5.8.0) IMPALA-3163: Fix Decimal to Timestamp casting
Date Tue, 24 May 2016 03:01:43 GMT
Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-3163: Fix Decimal to Timestamp casting
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3154/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

Line 569: int64_t
> should these be int32_t in this case?  if not, why?
Yeah, it makes sense that they can be int32_t. The entire Decimal4Value is can fit into int32_t.


Line 571: LIKELY
> why is this LIKELY?
I think it's going to be very rare that someone has a decimal with scale larger than 9 that
they want to convert to timestamp. In other words, why would someone have a decimal column
that represents timestamp with scale greater than 9 (for example picosecond precision)?


Line 577:       }
> How about factoring this block into a (templated) subroutine?
Done


http://gerrit.cloudera.org:8080/#/c/3154/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 1360: 260000000000
> what i'm asking is whether the upper limit changes between the old and new 
I did some investigating, but it seems it's very close, but not exactly the same. The larger
the precision and scale, the bigger the difference is. I'll investigate some more tomorrow.


Line 3766:  We get some decimal-binary skew here
> isn't this no longer the case?
Done


Line 3799:     // We get some decimal-binary skew here
> same
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabeea9f4ab4880b2f814408add63c77916e2dba9
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message