Skip to content

[Oracle] Fix handling timestamp with time zone #6012

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

Closed
wants to merge 1 commit into from

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Apr 16, 2023

Q A
Type bug
Fixed issues n/a

Summary

  • Fix format produced at InitializeSession::connect(), as it was not compatible with what is declared at OraclePlatform::getDateTimeTzFormatString();
  • Add missing declaration for "timestamptz" type at OraclePlatform::getNowExpression().

This bug was detected after adding the functional test for the timezone aware fields in #6006.

@phansys phansys changed the title [Oracle] Fix handling timestamp with time zone in Oracle [Oracle] Fix handling timestamp with time zone Apr 16, 2023
@phansys phansys marked this pull request as ready for review April 16, 2023 04:26
@phansys phansys force-pushed the oracle_tz branch 4 times, most recently from e0b4cbc to 1a7930c Compare April 16, 2023 18:51
@@ -28,7 +28,7 @@ public function connect(
. " NLS_TIME_FORMAT = 'HH24:MI:SS'"
. " NLS_DATE_FORMAT = 'YYYY-MM-DD HH24:MI:SS'"
. " NLS_TIMESTAMP_FORMAT = 'YYYY-MM-DD HH24:MI:SS'"
. " NLS_TIMESTAMP_TZ_FORMAT = 'YYYY-MM-DD HH24:MI:SS TZH:TZM'"
. " NLS_TIMESTAMP_TZ_FORMAT = 'YYYY-MM-DD HH24:MI:SSTZH:TZM'"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change certainly makes sense, but I fear that we break existing applications that rely on the current behavior.

Let's add a new constructor parameter to opt in to the correct format and trigger a deprecation if the app does not opt in. And that also means that we need to target 3.7.x because we don't introduce deprecations in a bugfix release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@phansys phansys changed the base branch from 3.6.x to 3.7.x May 10, 2023 01:41
@phansys phansys force-pushed the oracle_tz branch 3 times, most recently from 6a4e988 to 45ba3d5 Compare May 10, 2023 01:54
@github-actions
Copy link

github-actions bot commented Aug 8, 2023

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

@github-actions github-actions bot added the Stale label Aug 8, 2023
@github-actions
Copy link

This pull request was closed due to inactivity.

@github-actions github-actions bot closed this Aug 15, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants