[GitHub] [arrow] jorisvandenbossche commented on pull request #10176: ARROW-11759: [C++] Kernel to extract datetime components (year, month, day, etc) from timestamp type

[GitBox <gi...@apache.org>]

jorisvandenbossche commented on pull request #10176:
URL: https://github.com/apache/arrow/pull/10176#issuecomment-854529392


   To recap some spread inline comments (original discussion at https://github.com/apache/arrow/pull/10176#discussion_r633497393) about the sub-second / fractional components. 
   
   Assume the timestamp `2012-03-26 01:02:03.123456789`.
   
   Currently, I think the PR does:
   
   * second -> 3.123456789 (float)
   * subsecond -> 123456789 (int)
   
   Based on a naive reading of the docstrings and my biased expectations, I would have thought they return:
   
   * second -> 3 (int)
   * subsecond -> 0.123456789 (float)
   
   
   Now, in our previous discussion you proposed to implement `subsecond` as "total units since last second", so that indeed matches with what is in the PR now. But, that also means that the interpretation of this value critically depends on the unit of the timestamp type. While a float value would not have that problem.
   
   In addition, I personally think it would be useful to have some form of `second` that is just the integer the non-fractional component in " 01:02:03.123...".
   
   ---
   
   In addition, we also have the other components (for which eg Antoine questioned whether they are needed):
   
   * microsecond -> 123
   * millisecond -> 456
   * nanosecond -> 789


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org