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>
Fixed in #7835. |
Perhaps it's more appropriate to post it here rather than in #7835 It seems that 'RM' (abbreviation for Roman numbers) can not be used for cast from string to date in '<cast template>':
Output:
|
One more weird case.
Output:
PS. |
There is no such format string defined in the SQL standard. Why is this even implemented? |
Can you be more explicit in describing the problem? You have marked it with |
The first step is already wrong to begin with. The
should result in
The "result" you expect should only be produced if you'd use the format |
In other words, garbage in = garbage out, but the bigger problem is that this is seemingly accepted without resulting in an error in your second step. |
Because it exists in PostgreSQL, Oracle and MSSQL (at least). |
That is
Should result in an error, because the |
Specifically the rule for AM/PM is (in SQL:2023-2, section 9.51 Converting a formatted character string to a datetime):
where:
|
And I think that is a problem. Why not start with the SQL:2023 format as baseline and only add things when requested (who even uses Roman numerals in months?), instead of gold-plating it from the start. |
@pavel-zotov Could you make sure that your tests verify compliance with sections 9.50, 9.51 and 9.52 of SQL:2023-2, instead of only verifying that it does what the implementation says it does (as the implementation in #7629 actually implemented |
Unfortunately, I have no SQL:2023. |
Do you have SQL:2016-2? Because I don't think it changed in 2023 compared to the introduction in 2016 (except maybe wording/clarification). |
For reference, in SQL:2016-2 it is sections 9.42, 9.43 and 9.44. |
Also no. 0xFF-1. I don't know the nuances of the license, but: may FF purchase for all involved members one copy of this SQL:20xx ? 0xFF-2. Totally agree with aafemt
|
I have started a discussion on firebird-admins regarding this. And otherwise I will just quote those three sections here. |
I see that problem with cast(... format '<pattern>') still exists if format <pattern> is either 'TZR' or 'TZH:TZM'. Example-1:
Example-2:
|
I test these queries with my patch in PR #7881, and now we get the expected results. |
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>
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: