Skip to content

Basic date/time support #99

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

Merged
merged 1 commit into from
Jun 4, 2019
Merged

Basic date/time support #99

merged 1 commit into from
Jun 4, 2019

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Jun 3, 2019

This adds support for parsing date/time types as string literals. It doesn't try to make sense of the data inside the string literals by e.g. returning chrono objects; that's left to downstream libraries for now.

Note that this conflicts with #90 and #96. To reduce the merge conflicts, I've simply based this PR off the merge of those two PRs.

@benesch benesch requested a review from nickolay June 3, 2019 04:53
@coveralls
Copy link

Pull Request Test Coverage Report for Build 252

  • 262 of 272 (96.32%) changed or added relevant lines in 5 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage remained the same at ?%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/sqlast/sqltype.rs 0 1 0.0%
src/sqlast/value.rs 47 49 95.92%
src/sqlparser.rs 65 68 95.59%
tests/sqlparser_common.rs 143 147 97.28%
Files with Coverage Reduction New Missed Lines %
src/sqlast/value.rs 1 95.83%
src/sqlast/sqltype.rs 1 46.81%
Totals Coverage Status
Change from base Build 234: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 3, 2019

Pull Request Test Coverage Report for Build 286

  • 32 of 32 (100.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.07%) to 91.482%

Files with Coverage Reduction New Missed Lines %
src/sqlast/value.rs 1 96.77%
Totals Coverage Status
Change from base Build 284: 0.07%
Covered Lines: 3748
Relevant Lines: 4097

💛 - Coveralls

@benesch
Copy link
Contributor Author

benesch commented Jun 3, 2019

Oh, I guess this is #62.

@nickolay
Copy link
Contributor

nickolay commented Jun 3, 2019

The DATE/TIME/TIMESTAMP commit looks good to me, though I wonder why you chose to deviate from the SCREAMING CASE we use for SQL code in this repo. (At work I prefer lower-case too, but we've got to value consistency.)

The INTERVAL commit (and the remaining PRs) is more complex, and I'm not sure when I'll have the time to give it the attention it deserves. Please feel free to merge any of them without waiting for me, if it makes it any easier for you.

@benesch
Copy link
Contributor Author

benesch commented Jun 4, 2019

The DATE/TIME/TIMESTAMP commit looks good to me, though I wonder why you chose to deviate from the SCREAMING CASE we use for SQL code in this repo. (At work I prefer lower-case too, but we've got to value consistency.)

Oh, shoot, I fixed that oversight but amended it into the wrong commit. Updated!

The INTERVAL commit (and the remaining PRs) is more complex, and I'm not sure when I'll have the time to give it the attention it deserves. Please feel free to merge any of them without waiting for me, if it makes it any easier for you.

No worries at all! I'll extract it into its own PR so you can review it at your leisure, and merge this PR in the meantime.

@benesch benesch merged commit 1f87083 into apache:master Jun 4, 2019
@benesch benesch deleted the dates branch June 4, 2019 04:30
@benesch benesch mentioned this pull request Jun 4, 2019
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.

3 participants