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

Support CEIL(expr TO DateTimeField) and FLOOR(expr TO DateTimeField) #635

Merged
merged 11 commits into from
Oct 6, 2022

Conversation

sarahyurick
Copy link
Contributor

@sarahyurick sarahyurick commented Sep 28, 2022

Hi, I'd like to be able to parse the following statements:

SELECT CEIL(d TO DAY) FROM df
SELECT CEIL(d TO HOUR) FROM df
SELECT CEIL(d TO MINUTE) FROM df
SELECT CEIL(d TO SECOND) FROM df
SELECT CEIL(d TO MILLISECOND) FROM df

and

SELECT FLOOR(d TO DAY) FROM df
SELECT FLOOR(d TO HOUR) FROM df
SELECT FLOOR(d TO MINUTE) FROM df
SELECT FLOOR(d TO SECOND) FROM df
SELECT FLOOR(d TO MILLISECOND) FROM df

I also added Microsecond and Millisecond in addition to the existing (plural) Microseconds and Milliseconds.

@sarahyurick sarahyurick changed the title Support CEIL(expr TO DateTimeField) and FLOOR(expr TO DateTimeField Support CEIL(expr TO DateTimeField) and FLOOR(expr TO DateTimeField) Sep 28, 2022
@coveralls
Copy link

coveralls commented Sep 28, 2022

Pull Request Test Coverage Report for Build 3199394745

  • 55 of 60 (91.67%) changed or added relevant lines in 4 files are covered.
  • 717 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.06%) to 85.663%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ast/value.rs 2 3 66.67%
src/parser.rs 20 22 90.91%
tests/sqlparser_common.rs 24 26 92.31%
Files with Coverage Reduction New Missed Lines %
src/ast/data_type.rs 11 88.04%
tests/sqlparser_common.rs 44 97.05%
src/ast/mod.rs 253 77.47%
src/parser.rs 409 83.16%
Totals Coverage Status
Change from base Build 3146760243: 0.06%
Covered Lines: 10319
Relevant Lines: 12046

💛 - Coveralls

Copy link
Collaborator

@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 for the contribution @sarahyurick -- what database supports the CEIL( as date) syntax?

Do you think this will cause issues with existing queries that use ceil as a function, for example

select ceil(float_column) from my_table;

?

@sarahyurick
Copy link
Contributor Author

Thank you for the contribution @sarahyurick -- what database supports the CEIL( as date) syntax?

Do you think this will cause issues with existing queries that use ceil as a function, for example

select ceil(float_column) from my_table;

?

In terms of other databases that support this syntax, I've had trouble finding examples, but am mostly trying to support existing tests in Dask-SQL that were originally implemented with dask-contrib/dask-sql#91.

I can create some tests with CEIL(float_column) to try to gauge possible issues there.

@alamb
Copy link
Collaborator

alamb commented Oct 3, 2022

I can create some tests with CEIL(float_column) to try to gauge possible issues there.

As long as CEIL(float_column) can be parsed as a function call to CEIL I think it would be a good addition. Thank you

@sarahyurick
Copy link
Contributor Author

Okay @alamb, let me know what you think of this implementation.

I created a special DateTimeField::NoDateTime and then, depending on whether or not the TO keyword is there, we construct Expr::Ceil/Expr::Floor accordingly.

Similarly, when we're formatting the expression, we check if we have a DateTimeField::NoDateTime, which means that it should be formatted as CEIL(x), else it should be formatted as CEIL(expr TO DateTimeField).

src/parser.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@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 @sarahyurick -- I think this PR looks almost ready to go. I had a few style suggestions, but I think the basic structure looks good as do the tests. Thank you.

src/parser.rs Outdated Show resolved Hide resolved
tests/sqlparser_common.rs Show resolved Hide resolved
tests/sqlparser_common.rs Show resolved Hide resolved
src/ast/mod.rs Outdated Show resolved Hide resolved
src/ast/mod.rs Outdated Show resolved Hide resolved
sarahyurick and others added 2 commits October 6, 2022 10:17
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@sarahyurick
Copy link
Contributor Author

OK, I've refactored parse_ceil_expr and parse_floor_expr into parse_ceil_floor_expr. Let me know what you think.

@alamb alamb merged commit cb397d1 into sqlparser-rs:main Oct 6, 2022
@sarahyurick sarahyurick deleted the ceil_floor_datetime branch May 26, 2023 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants