You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "samuelcolvin (via GitHub)" <gi...@apache.org> on 2024/09/12 18:21:40 UTC
[I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
samuelcolvin opened a new issue, #6390:
URL: https://github.com/apache/arrow-rs/issues/6390
**Describe the bug**
While working on https://github.com/apache/datafusion/pull/12448 I noticed that the string `5 day hour` is not interpreted correctly by `parse_interval_month_day_nano_config`, it should be considered the same as `5 day 0 hour`, instead it return an error.
**To Reproduce**
run that function, or look at https://github.com/apache/datafusion/pull/12448
**Expected behavior**
`parse_interval_month_day_nano_config` should match postgres behaviour in general
**Additional context**
Related to #6211 where I improved interval parsing a lot, although apparently not enough.
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2372841637
In that case we need to decide if we wanna keep this one open or not.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Make Interval Parsing More Permissive [arrow-rs]
Posted by "ax1s-x1zz (via GitHub)" <gi...@apache.org>.
ax1s-x1zz commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-5559285843
I'd like to pick this up. Here's what I found and the scope I propose.
## Root cause
`"5 day hour"` is split by `split_interval_components` into `[("5", Some("day")), ("", Some("hour"))]`. The empty amount string `""` then fails `IntervalAmount::from_str`, producing `Invalid input syntax for type interval` (see the DataFusion slt case `interval '5 day' hour`, which becomes `"5 day HOUR"`).
## Proposed change
In `parse_interval_components` (arrow-cast/src/parse.rs), treat a unit with no amount as an amount of 0 — i.e. `"5 day hour"` parses like `"5 day 0 hour"`:
- empty amount string → `IntervalAmount { integer: 0, frac: 0 }`
- everything else unchanged: unknown units and repeated units still error
## Scope
This only relaxes the *bare unit* case (amount omitted). Unit-before-number ordering (e.g. `"hour 5"`) is a separate concern and is left alone.
Regression tests will cover `Interval::parse("5 day hour", ...)` == 5 days and the public `parse_interval_month_day_nano_config("5 day hour", ...)` path.
Does this match what you had in mind, or should the bare-unit-as-zero rule be restricted to trailing units only?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2365234381
I meant to ask if we plan to handle more cases in our parsing logic? To be closer to parsing abilities of Postgres? As @samuelcolvin pointed out above.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Make Interval Parsing More Permissive [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey closed issue #6390: Make Interval Parsing More Permissive
URL: https://github.com/apache/arrow-rs/issues/6390
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2350102349
I think `5 day hour` is a pretty uncommon usecase . Thank you for filing this
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2374731115
Let's leave it open so it is easier to find if someone else hits the problem
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Make Interval Parsing More Permissive [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-5674604859
ill close as unplanned to avoid confusion (i dont think we really intend to support this); its a pretty niche case and duckdb + spark dont support this, see
- https://github.com/apache/arrow-rs/pull/11006#issuecomment-5560459866
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2365189415
@alamb do we have a plan on integrating this inside Arrow?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2365227911
> @alamb do we have a plan on integrating this inside Arrow?
I am not sure what you mean by this question @ByteBaker
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2372001798
> I meant to ask if we plan to handle more cases in our parsing logic? To be closer to parsing abilities of Postgres? As @samuelcolvin pointed out above.
I guess in general I think it would be driven by contributor needs -- if someone wants needs more exotic parsing support, we can definitely consider it, but if it gets too specific, it probably makes sense to keep the parsing in the application
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Interval parsing interprets `5 day hour` wrongly [arrow-rs]
Posted by "samuelcolvin (via GitHub)" <gi...@apache.org>.
samuelcolvin commented on issue #6390:
URL: https://github.com/apache/arrow-rs/issues/6390#issuecomment-2350940100
The most likely scenario where it could come up is `select interval '5 day' hour`, but I agree even that seems like either allowing a common mistake, or a side effect of their very permissive parser.
---
While we're talking about weird things postgres supports, as per https://github.com/sqlparser-rs/sqlparser-rs/issues/1345#issuecomment-2334514705, the following are also supported:
```sql
select interval '1 month + 2 day'
select interval '1 minute + 2 s' -- makes sense, all units are supported in arithmetic expressions
select interval '1 minute - 2 s'
select interval '1 minute + 2' -- makes sense since raw numbers are interpreted as seconds
select interval '1 minute * 2s' -- this is crazy! the "*" operator is interpretted as "+" so this evaluates as 0:01:02
select interval '1 minute minute' -- repeated units are ignored
select interval '1 month + 2 day' day -- same thing with the units just concatenated
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org