Fix for DATENAME() gives incorrect value with TZOFFSET part#3846
Merged
forestkeeper merged 11 commits intobabelfish-for-postgresql:BABEL_5_X_DEVfrom Jul 1, 2025
Merged
Conversation
Collaborator
Pull Request Test Coverage Report for Build 16008532369Details
💛 - Coveralls |
Contributor
Author
|
TODO: Will look into version upgrade test failures |
a714a34 to
570debd
Compare
Anikait143
reviewed
Jun 19, 2025
63e74ed to
98c517f
Compare
98c517f to
ab53d08
Compare
KushaalShroff
requested changes
Jun 25, 2025
3311e14 to
97d5efa
Compare
Contributor
Author
|
Updating the DATENAME function from STABLE to IMMUTABLE fails an existing test case |
55c1745 to
46e0324
Compare
KushaalShroff
previously approved these changes
Jul 1, 2025
Contributor
|
Please check Test failures! |
Anikait143
reviewed
Jul 1, 2025
Anikait143
reviewed
Jul 1, 2025
test/JDBC/expected/datename_tzoffset-before-16_10-or-17_6-vu-prepare.out
Show resolved
Hide resolved
forestkeeper
reviewed
Jul 1, 2025
Currently, DATENAME function returns a numeric value for TZOFFSET datepart, which does not match SQL Server's text output. Root cause: The function definition of sys.datename explicitly calls the DATEPART function for most argument values, including TZOFFSET, thereby returning a numerical offset value instead of the required text format (+/-HH:MM). Fix: Modified the sys function's SQL definition to extract and return the timezone offset substring from the input - similar to how the offset_string is determined in DATETRUNC/DATEBUCKET - if the argument matches tzoffset. Furthermore, translated the function from SQL language to plpgsql in order to enable raising an exception for unsupported input datetime datatypes such as date, time, datetime and smalldatetime. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
In the datename function, before returning the tzoffset substring, called a sys helper function `babelfish_conv_string_to_datetimeoffset` to validate the date string and raise exceptions for invalid formats. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
DATENAME should support only datetime2 and datetimeoffset datetime datatypes as input when the datepart argument is TZOFFSET. SQL Server throws an unsupported error for datatypes date, time, datetime, smalldatetime when passed to datename(TZOFFSET, date). Modified a related test case to skip failing inserts for the mentioned datepart-datatype combinations. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
Moved datepart, datename inserts SQL from the verify file of datepart-before-16_10-or-17_6 to the prepare file in a view and tested the view in the verify file. This is to avoid the verify SQL from referencing the new definition of sys.DATENAME(TZOFFSET,...) and failing in the version upgrade tests. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
46e0324 to
bbcfde6
Compare
Added datename_tzoffset-before-16_10-or-17_6-vu-prepare/verify/cleanup test files to represent the difference in DATENAME output format before function update. Previously, timezone offset output was numeric, now it is in an offset string format '+/- HH:MM'. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com>
bbcfde6 to
219bfc0
Compare
forestkeeper
approved these changes
Jul 1, 2025
debf90b
into
babelfish-for-postgresql:BABEL_5_X_DEV
47 checks passed
manisha-deshpande
added a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Jul 1, 2025
…h-for-postgresql#3846) Currently, DATENAME function returns a numeric value for TZOFFSET datepart, which does not match SQL Server's text output. Root cause: The function definition of sys.datename explicitly calls the DATEPART function for most argument values, including TZOFFSET, thereby returning a numerical offset value instead of the required text format (+/-HH:MM). Fix: Modified the sys function's SQL definition to extract and return the timezone offset substring from the input - similar to how the offset_string is determined in DATETRUNC/DATEBUCKET - if the argument matches tzoffset. Furthermore, translated the function from SQL language to plpgsql in order to enable raising an exception for unsupported input datetime datatypes such as date, time, datetime and smalldatetime. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com> (cherry picked from commit debf90b)
manisha-deshpande
added a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Jul 2, 2025
…h-for-postgresql#3846) Currently, DATENAME function returns a numeric value for TZOFFSET datepart, which does not match SQL Server's text output. Root cause: The function definition of sys.datename explicitly calls the DATEPART function for most argument values, including TZOFFSET, thereby returning a numerical offset value instead of the required text format (+/-HH:MM). Fix: Modified the sys function's SQL definition to extract and return the timezone offset substring from the input - similar to how the offset_string is determined in DATETRUNC/DATEBUCKET - if the argument matches tzoffset. Furthermore, translated the function from SQL language to plpgsql in order to enable raising an exception for unsupported input datetime datatypes such as date, time, datetime and smalldatetime. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com> (cherry picked from commit debf90b)
forestkeeper
pushed a commit
that referenced
this pull request
Jul 3, 2025
* Fix for DATENAME() gives incorrect value with TZOFFSET part (#3846) Currently, DATENAME function returns a numeric value for TZOFFSET datepart, which does not match SQL Server's text output. Root cause: The function definition of sys.datename explicitly calls the DATEPART function for most argument values, including TZOFFSET, thereby returning a numerical offset value instead of the required text format (+/-HH:MM). Fix: Modified the sys function's SQL definition to extract and return the timezone offset substring from the input - similar to how the offset_string is determined in DATETRUNC/DATEBUCKET - if the argument matches tzoffset. Furthermore, translated the function from SQL language to plpgsql in order to enable raising an exception for unsupported input datetime datatypes such as date, time, datetime and smalldatetime. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com> (cherry picked from commit debf90b) * Fixed DATENAME to use datetimeoffset cast instead of sys.babelfish_conv_string_to_datetimeoffset While sys.babelfish_conv_string_to_datetimeoffset is a helpful way to validate datetimeoffset values before returning the offset string in DATENAME(TZOFFSET,...) it is crrently not present in 4_X. (BABEL-4896). Switching to a normal pg cast to datetimeoffset works as well, but does not validate out of boundary offset hour values above +/-14:00 since PG accepts upto hour 15. The incorrect output in the out-of-boundary test case in `datename+tzoffset-v-verify.sql` can be updated to the expected error (SQL Server), once the root cause jira BABEL-5827 is fixed. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com> (cherry picked from commit 3140b6c)
sharathbp
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Sep 22, 2025
…h-for-postgresql#3895) * Fix for DATENAME() gives incorrect value with TZOFFSET part (babelfish-for-postgresql#3846) Currently, DATENAME function returns a numeric value for TZOFFSET datepart, which does not match SQL Server's text output. Root cause: The function definition of sys.datename explicitly calls the DATEPART function for most argument values, including TZOFFSET, thereby returning a numerical offset value instead of the required text format (+/-HH:MM). Fix: Modified the sys function's SQL definition to extract and return the timezone offset substring from the input - similar to how the offset_string is determined in DATETRUNC/DATEBUCKET - if the argument matches tzoffset. Furthermore, translated the function from SQL language to plpgsql in order to enable raising an exception for unsupported input datetime datatypes such as date, time, datetime and smalldatetime. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com> (cherry picked from commit debf90b) * Fixed DATENAME to use datetimeoffset cast instead of sys.babelfish_conv_string_to_datetimeoffset While sys.babelfish_conv_string_to_datetimeoffset is a helpful way to validate datetimeoffset values before returning the offset string in DATENAME(TZOFFSET,...) it is crrently not present in 4_X. (BABEL-4896). Switching to a normal pg cast to datetimeoffset works as well, but does not validate out of boundary offset hour values above +/-14:00 since PG accepts upto hour 15. The incorrect output in the out-of-boundary test case in `datename+tzoffset-v-verify.sql` can be updated to the expected error (SQL Server), once the root cause jira BABEL-5827 is fixed. Task: BABEL-5846 Signed-off-by: Manisha Deshpande <mmdeshp@amazon.com> (cherry picked from commit 3140b6c)
This file contains hidden or 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
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.
Description
Currently, DATENAME function returns a numeric value for TZOFFSET datepart, which does not match SQL Server's text output.
Root cause: The function definition of sys.datename explicitly calls the DATEPART function for most argument values, including TZOFFSET, thereby returning a numerical offset value instead of the required text format (+/-HH:MM).
Fix: Modified the sys function's SQL definition to extract and return the timezone offset substring from the input - similar to how the offset_string is determined in DATETRUNC/DATEBUCKET - if the argument matches tzoffset. Furthermore, translated the function from SQL language to plpgsql in order to enable raising an exception for unsupported input datetime datatypes such as date, time, datetime and smalldatetime.
Additional changes: Updated an existing jdbc test case
datepart-vu-verify.sqlto avoid 'failing' table insertions ofDATENAME(TZOFFSET, cursor(date))for incompatible date types: datetime, smalldatetime, datetimeoffset.IF NOT (@datepart = 'tzoffset' AND @datatype NOT IN ('DATETIMEOFFSET', 'DATETIME2'))Added corresponding version upgrade tests.
Issues Resolved
BABEL-5846
Test Scenarios Covered
Minor version upgrade tests -
Major version upgrade tests -
Performance tests -
Tooling impact -
Client tests -
Check List
By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.