Skip to content

Conversation

@Lordworms
Copy link
Contributor

Which issue does this PR close?

Closes #9353

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Mar 1, 2024
@alamb alamb changed the title Supporting Date type for range and generate_series Support Date32, Date64, Interval(MonthDayNano) argumnet types in and generate_series Mar 2, 2024
@alamb alamb changed the title Support Date32, Date64, Interval(MonthDayNano) argumnet types in and generate_series Support Date32, argument types in and generate_series Mar 2, 2024
@alamb alamb changed the title Support Date32, argument types in and generate_series Support Date32 arguments for generate_series Mar 2, 2024
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Lordworms -- this looks great

I also tried it locally and it worked great (I found using unnest was easier to see the results)

select unnest(range(DATE '1992-09-01', DATE '1993-03-01', INTERVAL '1' MONTH));
+------------------------------------------------------------------------------------------------------------+
| unnest(range(Utf8("1992-09-01"),Utf8("1993-03-01"),IntervalMonthDayNano("79228162514264337593543950336"))) |
+------------------------------------------------------------------------------------------------------------+
| 1992-09-01                                                                                                 |
| 1992-10-01                                                                                                 |
| 1992-11-01                                                                                                 |
| 1992-12-01                                                                                                 |
| 1993-01-01                                                                                                 |
| 1993-02-01                                                                                                 |
+------------------------------------------------------------------------------------------------------------+
6 rows in set. Query took 0.007 seconds.

❯ select unnest(range(DATE '1992-09-01', DATE '1993-03-01', INTERVAL '1' DAY));
+---------------------------------------------------------------------------------------------------+
| unnest(range(Utf8("1992-09-01"),Utf8("1993-03-01"),IntervalMonthDayNano("18446744073709551616"))) |
+---------------------------------------------------------------------------------------------------+
| 1992-09-01                                                                                        |
| 1992-09-02                                                                                        |
| 1992-09-03                                                                                        |
| 1992-09-04                                                                                        |
...

I think it just needs a few more tests and this PR will be ready to go

[dependencies]
arrow = { workspace = true }
datafusion-common = { workspace = true, default-features = true }
arrow-array = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we need to add arrow-array explicilty? Isn't everything we need already exported via arrow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove it. I was trying to use some methods in that crate so imported that, I will remove it

range(1, -5, -1),
range(DATE '1992-09-01', DATE '1993-03-01', INTERVAL '1' MONTH),
range(DATE '1993-02-01', DATE '1993-01-01', INTERVAL '-1' DAY),
range(DATE '1989-04-01', DATE '1993-03-01', INTERVAL '1' YEAR)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please also add tests for null handling?

For example

       range(DATE '1992-09-01', DATE '1993-03-01', NULL,
       range(DATE '1992-09-01', NULL , INTERVAL '1' MONTH),
       range(NULL, DATE '1993-03-01', INTERVAL '1' MONTH),

Also, can you add negative tests (like when start is > stop by the the interval is positive (I think the code will error in this case, but it would be good to verify this

       range(DATE '1993-03-01', DATE '1989-04-01', INTERVAL '1' YEAR)
       range(, DATE '1989-04-01', DATE '1993-03-01', INTERVAL '-1' YEAR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will do it now

@alamb
Copy link
Contributor

alamb commented Mar 2, 2024

Bonus points for updating the docs to mention the new support for date: https://arrow.apache.org/datafusion/user-guide/sql/scalar_functions.html#range

(we can do this as a follow on PR as well)

@Lordworms
Copy link
Contributor Author

Bonus points for updating the docs to mention the new support for date: https://arrow.apache.org/datafusion/user-guide/sql/scalar_functions.html#range

(we can do this as a follow on PR as well)

Yes, I will do it in next PR, along with finishing port of regx function today

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Lordworms

@alamb
Copy link
Contributor

alamb commented Mar 3, 2024

Screenshot 2024-03-03 at 3 38 15 AM

🤔 looks like there are some conflicts to resolve

@Lordworms
Copy link
Contributor Author

Screenshot 2024-03-03 at 3 38 15 AM 🤔 looks like there are some conflicts to resolve

got it

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again @Lordworms

@alamb alamb merged commit ac27428 into apache:main Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support select range from datetime

2 participants