-
Notifications
You must be signed in to change notification settings - Fork 0
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
Added Correctness Tests For Date And Time Functions #211
Added Correctness Tests For Date And Time Functions #211
Conversation
Signed-off-by: Matthew Wells <[email protected]>
Signed-off-by: Matthew Wells <[email protected]>
Codecov Report
@@ Coverage Diff @@
## integ-date-time-correctness #211 +/- ##
==============================================================
Coverage 98.35% 98.35%
Complexity 3617 3617
==============================================================
Files 343 343
Lines 8927 8927
Branches 569 569
==============================================================
Hits 8780 8780
Misses 142 142
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
What about rest functions? Try to pick all checked functions from opensearch-project#722, e.g. |
@@ -0,0 +1,13 @@ | |||
DATE '2001-05-07' | |||
TIME '12:30:00' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about nanoseconds, does that work?
TIME '12:30:00.004'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly TIME '12:30:00.004
does not work but TIMESTAMP '2001-05-07 12:30:00.1234'
does work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After investigating I have found that this does not work because of a known bug opensearch-project/sql-jdbc#16
All functions that I didn't include in the test fail because they are not in either H2 or SQLite. Because of the fact that tests need to pass on at least one database to pass and SQLite doesn't really have any date and time functions I went through the list of all H2 functions and and compared them to OpenSearch if it had a matching function. Every function that I didn't include either doesn't exist in H2 so it automatically fails and also in the case of ADD_DATE and MAKE_TIME they don't appear to work in OpenSearch. |
I see. Please, add this ^ to the PR description to make things clear, thanks. |
I have updated the description to mention the reason for functions that haven't been included |
Signed-off-by: Matthew Wells [email protected]
Description
Wrote correctness tests for date and time functions, some tests were not including due to them failing.
All string functions that have no been included in the test is because they either aren't implemented in OpenSearch, or they aren't implemented in H2 and SQLite. Because of the small number of string functions in SQLite and the fact that all correctness tests need to pass against at least one database all functions are taken from the overlap of functions between OpenSearch and H2. All functions that are implemented in both OpenSearch and H2 that do not pass have their specific reasons for failure mentioned below.
TIME WITH TIME ZONE
whereas OpenSearch returns aTIME
as well as a different time due to different timezoneTIME WITH TIME ZONE
whereas OpenSearch returns aTIME
as well as a different time and sometimes date due to different timezoneDATE
parameters and returns the difference in days while H2 takes in a third parameter for time period to be returnedIssue
opensearch-project#722
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.