alamb commented on PR #6199:
URL: https://github.com/apache/arrow-rs/pull/6199#issuecomment-2275522669

   > Thanks for your comments @alamb (and patience 😅 ) ... I think I got 
anchored to the series generation code in datafusion that uses a date32 for 
generating series for dates.
   
   Sorry for the delay in responding. Things have been very busy for me the 
last few days
   
   > I think maybe the proper path for improving this handling looks something 
like:
   > 
   > 1. Close this PR, to your point about `Date64` being increments of day, vs 
timestamp being semantically correct
   > 2. (optional) Temp fix to datafusion to throw an error when something 
smaller than a day is used in `generate_series` with a date.
   
   Yes I think this sounds like a good idea
   
   > 3. Better fix to coerce the date into a timestamp (this would seem to 
match postgres and duckdb) and support timestamps general (not yet supported in 
the `GenSeries` UDF (I think there's a ticket for the latter piece of work).
   
   This (using timestamps in gen sereis) also seems reasonable to me -- I 
suggest we treat it as a separate ticket though as the infinite loop seems like 
a bug and this part seems more like a feature / improvement. 
   
   
   > 4. (optional) Comments and/or fallible addition/subtraction between 
`Date*` and intervals when they aren't sufficient. E.g. adding a `Date32` with 
an interval a `MonthDayNano` when only the nano part is present and less than a 
day, doesn't align with a day, etc. I can also do a bit of digging against the 
C++ implementation to see if there's anything similar there.
   
   Sounds also reasonable
   
   Thanks again for the help. 
   


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to