Normalize source string when parsing #609
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The regex used to parse dates does not account for the fact that new dates created by SQLite's CURRENT_TIMESTAMP do not include millisecond information. The regex used by the date parser tries to find a string matching "yyyy-MM-dd HH:mm:ss.SSS" but the dates created by SQLite do not include the milliseconds (.SSS) so the date strings are not matching and this is causing the parser to return null. The solution is to detect this exact case (string length is exactly 19) and then append .000 to the source string. This causes the regex to match on the string. allowing the date string to be parsed, and the Date object to be returned. The source object is not mutated, and the extra characters do not change the DATETIME represented by the date string, so I think this is a safe fix.
|
No unit test to catch possible future regression? |
|
@bendem I'm a little outside my comfort zone here, but I gave it a shot. I added a unit test to the StatementTest class. Let me know what you think. |
|
Looks good to me, but I'm not maintainer :) |
|
Thanks for the fix. It looks good to me |
xerial
added a commit
that referenced
this pull request
Jun 27, 2021
|
@davidjpfeiffer Sorry. It's once merged, but the CI is failing because of this change. Could you submit a change again? |
|
@xerial sure, should I create a new PR with the exact same changes? |
|
Yes. In another PR, GitHub Action CI will run to find any issue in advance |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request fixes the bug described by issue #88
This bug can be reproduced by using
ResultSet.getDate()to parse aDATETIMEcreated usingCURRENT_TIMESTAMPThe exception thrown will look something like this:
java.text.ParseException: Unparseable date: "2021-06-08 02:00:47" does not match (\p{Nd}++)\Q-\E(\p{Nd}++)\Q-\E(\p{Nd}++)\Q \E(\p{Nd}++)\Q:\E(\p{Nd}++)\Q:\E(\p{Nd}++)\Q.\E(\p{Nd}++)The regex used to parse dates does not account for the fact that new dates created by SQLite's
CURRENT_TIMESTAMPdo not include millisecond information. The regex used by the date parser tries to find a string matchingyyyy-MM-dd HH:mm:ss.SSSbut the dates created by SQLite do not include the milliseconds.SSSso the date strings are not matching and this is causing the parser to return null. The solution implemented in this pull request detects this exact case (string length is exactly 19) and then appends.000to the source string. This causes the regex to match on the string allowing the date string to be parsed and the Date object to be returned. The source object is not mutated, and the extra characters do not change theDATETIMErepresented by the date string, so I think this is a safe fix.Please let me know if you have any questions about this.