alamb commented on code in PR #5806: URL: https://github.com/apache/arrow-datafusion/pull/5806#discussion_r1157493781
########## datafusion/core/tests/sqllogictests/test_files/interval.slt: ########## @@ -23,7 +23,113 @@ select arrow_typeof(interval '5 months'), arrow_typeof(interval '5 days 3 nanoseconds') ---- -Interval(YearMonth) Interval(MonthDayNano) +Interval(MonthDayNano) Interval(MonthDayNano) + + +## This is incredibly confusing but document it in tests: +# +# years is parsed as a column name +# year is parsed as part of the interval type. +# +# postgres=# select interval '5' year; +# interval +# ---------- +# 5 years +# (1 row) +# +# postgres=# select interval '5' years; +# years +# ---------- +# 00:00:05 +# (1 row) +query ? +select interval '5' years +---- +0 years 0 mons 0 days 0 hours 0 mins 5.000000000 secs + + +# check all different kinds of intervals +query ? +select interval '5' year +---- +0 years 60 mons 0 days 0 hours 0 mins 0.000000000 secs Review Comment: Yeah, I agree it is a little strange. I can't figure out how important it is to to mirror back exactly what the user said. For some things like `5 years` we could pick `Interval(YearMonth)` and encode exactly what the user specified For things like `5 years 3 days` to express that properly I think we need to use `Interval(MonthDayNano)` which is going to be formatted like `60 months ... 3 days` Or maybe we should just make the interval printing prettier 🤔 like convert `60 months` to 5 years... -- 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]
