-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
Implement SQL standard FORMAT clause for CAST between string types and datetime types [CORE6507] #2388
Comments
Commented by: @mrotteveel @dmitry, personally I read the original request of CORE1314 more like a request for a message formatter like the following: ``` results in ``` The dateformatting was tacked on CORE1314 when CORE1341 was closed, while it should have remained a separate ticket IMHO. These are two different concerns that should be handled separately (string interpolation vs dateformatting). |
It would be nice to see this feature in 5.0. |
@omachtandras You can use a thumbs-up response on the initial comment (through the smiley icon button). |
I made a PR #7629 with this feature, and it would be nice if someone would look into it. |
…2388 (#7629) * Add FORMAT clause to convert datetime types to string and vice versa * Add tests for FORMAT clause * Fixes after review * Change TZD to TZR * Change inline variables back to static * Add README documentation * Add ability to use " in raw string and ... Use session timezone if timezone is not specified. Add ability to use + sign in timezone offset. Add truncating string exception. * Move util methods from BOOST_AUTO_TEST_SUITE * Switch back to inline variables * Consider charset in the format string * Add ability to write patterns without separators * Use printf to add extra zeros Also add extra zeros to the year patterns. * Replace template exception with a plain function * Clean code after review * Fix bug with TZH:TZM when TZH is 0 * Add TZR to STRING to DATE --------- Co-authored-by: Artyom Ivanov <artyom.ivanov@red-soft.ru>
Is it possible to use format pattern (specified here as "<cast template>") from variable instead of literal one ? Suppose that i have following table and put one row in it:
Following query runs OK:
But if i try to do similar using PSQL block then i get error:
Output:
|
No, the SQL standard doesn't support that. It only specifies support for a character string literal (see the quoted syntax rule To be honest, I'm not really happy how the implementation turned out. It has far too many non-standard extension as it is IMHO. |
IMHO it's better to have a standard function with non-standard extensions than implement a second non-standard function with these extensions. We have standard EXTRACT with non-standard QUARTER / MILLISECOND / WEEK / WEEKDAY / YEARDAY and nobody complained so far ;-) |
Sure, but I think some of the extensions that have now been added to the cast-format make it more complicated than necessary, and should only have been implemented when there is a clear demand, not as the initial implementation. |
One more Q.
Why presence of |
That looks like a bug to me. |
It seems so. I'm looking at the |
There are things in this implementation that without access to the standard I cannot say they are correct or not, for example, missing parts:
Also note that |
This is covered by 9.51 Converting a formatted character string to a datetime (SQL:2023), to paraphrase the rule: missing date parts take the value of CURRENT_DATE, for missing time parts values take value zero (0) (note if SSSS (second of day) is specified, all other time parts are irrelevant), for missing time zone parts, also: use zero (0). |
Correction: the default values for the date parts are taken from @asfernandes As an aside: I highly recommend buying a copy of ISO/IEC 9075-2:2023 (or maybe ask the Firebird Foundation to buy a copy for you). |
I asked and the subject didn't moved. |
QA issue: see #2388 (comment) |
…#2388 (#7835) * Convert time with time zone to UTC Also return an exception that was "lost" in previous commits * Fix incorrect timezone conversion Also change behavior of "YY" and "YYY" the way it is done in the standard conversion. * Convert tm year to real year for calculations in "YY" pattern --------- Co-authored-by: Artyom Ivanov <artyom.ivanov@red-soft.ru>
Am i right in guess that this patch soon will be applied and i could resume testing ? |
I re-read this issue, and @mrotteveel said that |
Maybe, but last two examples are about HH24, not HH12.
How should source data look when we have to specify it together with time zone? Can you provide an example ? |
Something like this, I guess: |
Well, i will wait for commit to make overall test. |
That template should trigger an error when parsing the string '4:24:20 +01:00' as it doesn't contain And to reiterate, the template part Note that the SQL specification requires that both
(from: ISO/IEC 9075-2:2023(E) 9.52 Datetime templates) I have to repeat that I find it worrisome to see this much guessing about how this should be implemented: read the standard and implement it accordingly, don't just guess at things. |
…ake it more similar to the SQL standard #2388 (#7881) * Use current TimeStamp for data in stringToDate conversion if it's not specify Also fix RM pattern and change (A/P)M to (A/P).M. * Add more tests * Add TimeStamp validation Also move duplicated code to functions. * Add more unit tests for "YY" and "YYY" patterns * Use Callback for getting current date It's better because we can mock Callback for unit tests. * Fix exception and README description * Add ability to print blr_cast_format * Put a comment about new BLR in the right place * Add information about behavior of string to datetime conversion * Rework old patterns and add new ones Add A.M, P.M., RR and RRRR patterns. Rework YY, YYY, HH and HH12 patterns due to new patterns. Add restriction from SQL standard to format. Fix incorrect error message for mismatched pattern. Fix bug with 0 hours in HH12. * Add more unit tests * Update doc for cast format * Allow specification of log_level for BOOST_TESTS in make * Change enum class to enum in namespace * Switch from plain enum to constexpr values --------- Co-authored-by: Artyom Ivanov <artyom.ivanov@red-soft.ru>
21 results found in 6.0.0.301 that can not be explained. Example-1:
Example-2:
Please see attachment with .sql and its log. gh-2388-wrong-results-when-convert-time_tz-to-str-and-back.zip |
PS.
Where can these tests be found (if, of course, they aren't private) ? |
https://github.com/FirebirdSQL/firebird/blob/master/src/common/tests/CvtTest.cpp
https://github.com/FirebirdSQL/firebird/blob/master/doc/README.cast.format.md#2-string-to-datetime |
@pavel-zotov Out of curiosity, why are you putting the A.M./P.M. format marker in such an illogical place? I'm not aware of anyone (or any locale) who in practice would want to generate a value like |
It's not me. The script that generates [almost] all possible combinations has "found" that :-) |
PS. |
The problem with that is that the SQL template for datetime doesn't prohibit it as far as I can tell from the rules in the standard. |
I cannot reproduce this behavior in 6.0.0.305. |
Hm-m... it looks weird but i'm also can not reproduce that, on same 6.0.0.301 which i checked 31-mar-2024 :-/ |
I have one more Q.
-- issues:
AFAIU, final part of every line ("-04-04" ) is from current year (2024). |
To be more precise, it's from current date, first 04 - current month and second 04 - current day.
I didn't add such validation because I thought it looked more like "shooting yourself in a foot" problem. Maybe I'm wrong about that.
It's using last pattern for YEAR part (YY), so we get these values. |
QA-run for snapshot 'WI-T6.0.0.313 Firebird 6.0 aaf5faf' shows that some results are still wrong.
Output:
It seems that problem can raise not only for some time zones but also for concrete values of timestamps (test selects randomly both parameters). |
According to the SQL standard (9.52 Datetime templates of SQL:2023-2) you do need to validate that:
|
Comparison for timezones 'Africa/Casablanca' vs 'Africa/Algiers' shows that first of them definitely has a problem. Please consider two scripts below (yes, some formats look very strange - but they do not affect on result): Timezone = 'Africa/Casablanca':
Timezone = 'Africa/Algiers':
Time values in both scripts are identical (for appropriate statements), scripts differ only in TZ suffix ('Casablanca' vs 'Algiers'). |
I've done somewhat like 'brute force attack' in order to get full list of time values + time zones which have a problem with converting from string to time with timezone using FORMAT clause in the CAST(). There are only TWO problematic time zones:
And both of them have only ONE SCOPE of time values which have a problem with CAST(): they start with 02:00 and finish with 02:59. All results for that scope are wrong (one hour ahead). No other time zones and also no other time scopes in above mentioned TZ (Casablanca and El_Aaiun) caused any problem. SQL script with all statements that have wrong result see in attached .zip |
I did not verified if @pavel-zotov tests are correct, I hope @TreeHunter9 does, but look at this:
You have found some timezones that were switching offsets today! |
Hmm... it seems that you're right: Latest clock change for both is: TODAY, 14-APR-2024, from 02:00 AM forward to 03:00 AM |
PS. |
I have one more Q.
IMO, this corresponds to
And - note - clock must be changed (increased by 1 hour) for BOTH timezones ("countrywide"), but:
Now let's look on result of following statements:
|
It seems like there will be many more questions :-) From this: https://www.worlddata.info/america/mexico/timezones.php - we can see that:
Also, let's note there is Monterrey for which:
Now - let's check following:
So, we can see that there are NO changed clock, at all. |
Same question about Canada (Blanc-Sablon vs Toronto), clock must be changed at 02:00 AM 03-NOV-2024:
|
Submitted by: @mrotteveel
Implement SQL standard FORMAT clause for CAST between string types and datetime types, to allow custom formatting of datetime values and conversion from string values with a specific format to datetime values.
"""
<cast specification> ::=
CAST <left paren>
<cast operand> AS <cast target>
[ FORMAT <cast template> ]
<right paren>
<cast operand> ::=
<value expression>
| <implicitly typed value specification>
<cast target> ::=
<domain name>
| <data type>
<cast template> ::=
<character string literal>
"""
Where <cast template> follows the rules of Subclause 9.42, "Converting a datetime to a formatted character string" or Subclause 9.43, "Converting a formatted character string to a datetime". Specific syntax rules defined in Subclause 9.44, "Datetime templates":
"""
<datetime template> ::=
{ <datetime template part> }...
<datetime template part> ::=
<datetime template field>
| <datetime template delimiter>
<datetime template field> ::=
<datetime template year>
| <datetime template rounded year>
| <datetime template month>
| <datetime template day of month>
| <datetime template day of year>
| <datetime template 12-hour>
| <datetime template 24-hour>
| <datetime template minute>
| <datetime template second of minute>
| <datetime template second of day>
| <datetime template fraction>
| <datetime template am/pm>
| <datetime template time zone hour>
| <datetime template time zone minute>
<datetime template delimiter> ::=
<minus sign>
| <period>
| <solidus>
| <comma>
| <apostrophe>
| <semicolon>
| <colon>
| <space>
<datetime template year> ::=
YYYY | YYY | YY | Y
<datetime template rounded year> ::=
RRRR | RR
<datetime template month> ::=
MM
<datetime template day of month> ::=
DD
<datetime template day of year> ::=
DDD
<datetime template 12-hour> ::=
HH | HH12
<datetime template 24-hour> ::=
HH24
<datetime template minute> ::=
MI
<datetime template second of minute> ::=
SS
<datetime template second of day> ::=
SSSSS
<datetime template fraction> ::=
FF1 | FF2 | FF3 | FF4 | FF5 | FF6 | FF7 | FF8 | FF9
<datetime template am/pm> ::=
A.M. | P.M.
<datetime template time zone hour> ::=
TZH
<datetime template time zone minute> ::=
TZM
"""
The text was updated successfully, but these errors were encountered: