Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in 42.0.0: Parsing fractional intervals without leading 0 is not supported #4424

Closed
alamb opened this issue Jun 16, 2023 · 2 comments · Fixed by #4425
Closed

Regression in 42.0.0: Parsing fractional intervals without leading 0 is not supported #4424

alamb opened this issue Jun 16, 2023 · 2 comments · Fixed by #4425
Assignees
Labels
arrow Changes to the arrow crate bug

Comments

@alamb
Copy link
Contributor

alamb commented Jun 16, 2023

Describe the bug
in version 41.0.0 an interval such as .5 months was parsed correctly

In version 42.0.0 (not yet released at the time of this writing) it fails with an error: NotYetImplemented("Unsupported Interval Expression with value ".5 months")

To Reproduce
Try to parse .5

Here is a unit test:

diff --git a/arrow-cast/src/parse.rs b/arrow-cast/src/parse.rs
index accce99b4..7984aeb9a 100644
--- a/arrow-cast/src/parse.rs
+++ b/arrow-cast/src/parse.rs
@@ -1798,6 +1798,16 @@ mod tests {
             Interval::parse("-1.5 months -3.2 days", &config).unwrap(),
         );
 
+        assert_eq!(
+            Interval::new(0i32, 15i32, 0),
+            Interval::parse("0.5 months", &config).unwrap(),
+        );
+
+        assert_eq!(
+            Interval::new(0i32, 15i32, 0),
+            Interval::parse(".5 months", &config).unwrap(),
+        );
+
         assert_eq!(
             Interval::new(2i32, 10i32, 9 * NANOS_PER_HOUR),
             Interval::parse("2.1 months 7.25 days 3 hours", &config).unwrap(),

The second case fails like this:

---- parse::tests::test_parse_interval stdout ----
thread 'parse::tests::test_parse_interval' panicked at 'called `Result::unwrap()` on an `Err` value: NotYetImplemented("Unsupported Interval Expression with value \".5 months\"")', arrow-cast/src/parse.rs:1808:51
stack backtrace:

Expected behavior
Both new cases should pass correctly

Additional context
Found while testing this upgrade in DataFusion

@alamb
Copy link
Contributor Author

alamb commented Jun 16, 2023

I believe this was introduced by #4291

at ac9c6fa, these two (equivalent tests) pass:

        assert_eq!(
            (0i32, 15i32, 0),
            parse_interval("months", "0.5 months").unwrap(),
        );

        assert_eq!(
            (0i32, 15i32, 0),
            parse_interval("months", ".5 months").unwrap(),
        );
 

at 72f84b2 , (with #4291), the reproducer fails:

thread 'parse::tests::test_parse_interval' panicked at 'called `Result::unwrap()` on an `Err` value: NotYetImplemented("Unsupported Interval Expression with value \".5 months\"")', arrow-cast/src/parse.rs:1808:51
stack backtrace:

@alamb
Copy link
Contributor Author

alamb commented Jun 16, 2023

For better (likely for worse) .5 is supported by postgres:

postgres=# select interval '0.5 minutes';
 interval
----------
 00:00:30
(1 row)

postgres=# select interval '.5 minutes';
 interval
----------
 00:00:30
(1 row)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant