Skip to content
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

Forward-compatible expressions LOCALTIME and LOCALTIMESTAMP [CORE5853] #6113

Closed
firebird-automations opened this issue Jun 20, 2018 · 16 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: @asfernandes

In version 4.0, Firebird will support time zones.

Within this support will come an incompatibility with previous versions' `CURRENT_TIME` and `CURRENT_TIMESTAMP` expressions.

In v4, `CURRENT_TIME` and `CURRENT_TIMESTAMP` will respectively return expressions of data type `TIME WITH TIME ZONE` and `TIMESTAMP WITH TIME ZONE`.

To make your queries and database code compatible with future versions, from v3.0.4 you can instead use `LOCALTIME` and `LOCALTIMESTAMP`. In v3, these `LOCAL*` expressions will work indenticaly to their correspondents `CURRENT_*` expressions.

In v4, `LOCAL*` expressions will continue to work identically as now, but the `CURRENT_*` expressions will change.

You should not start using `LOCALTIME` and `LOCALTIMESTAMP` if your database may be downgraded to v3.0.3 or another version, as the old versions will not recognize the new expressions.

Commits: 05ebc02 15e25b3

====== Test Details ======

Empty code remains for 4.0 section in .fbt: waiting for merging this new feature in master.

@firebird-automations
Copy link
Collaborator Author

Modified by: @asfernandes

assignee: Adriano dos Santos Fernandes [ asfernandes ]

@firebird-automations
Copy link
Collaborator Author

Commented by: Sean Leyne (seanleyne)

Adriano,

For consistency, would "LOCAL_", instead of "LOCAL" be a more appropriate prefix?

IMO it would align with the existing "CURRENT_".

@firebird-automations
Copy link
Collaborator Author

Commented by: @livius2

Why not CURRENT_TIME_TZ and CURRENT_TIMESTAMP_TZ for new timezone support
instead braking old working codes?

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

SQL standard defined as LOCALTIME and LOCALTIMESTAMP.

@firebird-automations
Copy link
Collaborator Author

Commented by: Nick (nick)

Maybe we can create new sql dialect for sql-standard?
And another one for legacy?
And another one, say it trunk dialect, without legacy and sql-standard brain-breaking stuff? :)

Maybe we can have a plugin for working with dialects? :)

@firebird-automations
Copy link
Collaborator Author

Modified by: @asfernandes

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: 3.0.4 [ 10863 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @sim1984

Will there be a backport in 2.5.x? Many developers have not yet migrated to 3.0 and are waiting for release 4.0. If I understand correctly with release 4.0, support 2.5 will stop. Give them a chance to migrate to 4.0 without intermediate migrations 2.5 -> 3.0 -> 4.0.

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

Backported to 2.5.9.

@firebird-automations
Copy link
Collaborator Author

Modified by: @asfernandes

Fix Version: 2.5.9 [ 10862 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

What is the proper syntax in 4.0 for these new keywords ( `LOCALTIME` and `LOCALTIMESTAMP`) ?
The following script:

set list on;
set echo on;

show version;

select current_time, current_timestamp from rdb$database;

select localtime from rdb$database;

select localtimestamp from rdb$database;

-- works OK on 2.5.9.27115 and 3.0.4.33019, but on WI-T4.0.0.1142 i get:

select current_time, current_timestamp from rdb$database;

CURRENT_TIME 14:18:16.0000
CURRENT_TIMESTAMP 2018-07-31 14:18:16.3690

select localtime from rdb$database;
Statement failed, SQLSTATE = 42S22
Dynamic SQL Error
-SQL error code = -206
-Column unknown
-LOCALTIME
-At line 1, column 8
At line 7 in file c5853.sql

select localtimestamp from rdb$database;
Statement failed, SQLSTATE = 42S22
Dynamic SQL Error
-SQL error code = -206
-Column unknown
-LOCALTIMESTAMP
-At line 1, column 8
At line 9 in file c5853.sql

PS.
Firebird/Windows/Intel/i386 (access method), version "WI-T4.0.0.1142 Firebird 4.0 Alpha 1"
Firebird/Windows/Intel/i386 (remote server), version "WI-T4.0.0.1142 Firebird 4.0 Alpha 1/tcp (csprog)/P16"
Firebird/Windows/Intel/i386 (remote interface), version "WI-T4.0.0.1142 Firebird 4.0 Alpha 1/tcp (csprog)/P16"
on disk structure version 13.0

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: No test => Deferred

Test Details: Waiting for reply on note 31/Jul/18 11:19 AM

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

The changes are not merged in master yet.

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: Deferred => Done with caveats

Test Details: Waiting for reply on note 31/Jul/18 11:19 AM => Empty code remains for 4.0 section in .fbt: waiting for merging this new feature in master.

@firebird-automations
Copy link
Collaborator Author

Commented by: @dyemanov

I'd suggest making LOCALTIME / LOCALTIMESTAMP non-reserved in v2.5 / v3.0, regardless of the standard requirements. I'm afraid making existing objects with the same names unusable is too radical for a point release. This has already appeared in our test suite (test bugs.core_4470 contains a packaged function named LOCALTIME and now throws a parsing error), so the issue is not purely theoretical.

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

LOCALTIME / LOCALTIMESTAMP (without the precision) are very equivalent to a uncontexted column access and with the precision syntax are identical to a function call.

So how can we make them non-reserved?

It's already in keyword_or_column, but non_reserved_word will not work for them.

@firebird-automations
Copy link
Collaborator Author

Commented by: @dyemanov

I missed that point, sorry. Agreed it's not possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants