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

sql: remove/hide support for TIMETZ #28095

Merged
merged 1 commit into from
Aug 13, 2018
Merged

Conversation

knz
Copy link
Contributor

@knz knz commented Jul 31, 2018

Fixes #25224.

This patch removes the support for TIMETZ that was previously
introduced in the 2.1 branch.

The protobuf ID for table schemas remains reserved, however any
attempt to access a TIMETZ column in existing tables will cause a node
to panic. Tables containing TIMETZ columns must be completely dropped,
they cannot be recovered.

Release note (backward-incompatible change): Support for PostgreSQL's
TIMETZ data type is removed because it was incomplete/incorrect. This
was a feature only available in previous 2.1-alpha releases but not
any stable release, and thus was experimental anyways. Tables
previously created with the TIMETZ data type must be dropped entirely
-- it is not possible to convert the data or drop a single TIMETZ
column.

cc @awoods187

@knz knz added this to Triage in (DEPRECATED) SQL Front-end, Lang & Semantics via automation Jul 31, 2018
@knz knz requested a review from a team as a code owner July 31, 2018 13:22
@knz knz requested review from a team July 31, 2018 13:22
@knz knz requested a review from a team as a code owner July 31, 2018 13:22
@knz knz requested review from a team July 31, 2018 13:22
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz
Copy link
Contributor Author

knz commented Jul 31, 2018

cc @rmloveland

@knz
Copy link
Contributor Author

knz commented Jul 31, 2018

@petermattis just suggested an alternate approach which I will try shortly.

@knz knz moved this from Triage to Candidate next milestone in (DEPRECATED) SQL Front-end, Lang & Semantics Jul 31, 2018
@knz knz moved this from Candidate next milestone to Current milestone in (DEPRECATED) SQL Front-end, Lang & Semantics Jul 31, 2018
@knz
Copy link
Contributor Author

knz commented Aug 13, 2018

So the attempt was to keep the datum type but remove the table encoding (which was blatantly wrong)

However the datum type was also wrong actually -- the package timeofday has subtle flaws that make TIMETZ very wrong, namely that the times should be converted relative to the current date, not January 1 1970. This is a rather non-trivial fix and I think the implementation of TIMETZ (and, actually, also TIME) is currently irredeemable.

So I have opted for a full removal. We can always consider a revert later if we want to restore the functionality, e.g. on the master branch after 2.1. has been cut.

This patch removes the support for TIMETZ that was previously
introduced in the 2.1 branch.

The protobuf ID for table schemas remains reserved, however any
attempt to access a TIMETZ column in existing tables will cause a node
to panic. Tables containing TIMETZ columns must be completely dropped,
they cannot be recovered.

Release note (backward-incompatible change): Support for PostgreSQL's
TIMETZ data type is removed because it was incomplete/incorrect. This
was a feature only available in previous 2.1-alpha releases but not
any stable release, and thus was experimental anyways. Tables
previously created with the TIMETZ data type must be dropped entirely
-- it is not possible to convert the data or drop a single TIMETZ
column.
@knz
Copy link
Contributor Author

knz commented Aug 13, 2018

bors r+

craig bot pushed a commit that referenced this pull request Aug 13, 2018
28095: sql: remove/hide support for TIMETZ r=knz a=knz

Fixes #25224.

This patch removes the support for TIMETZ that was previously
introduced in the 2.1 branch.

The protobuf ID for table schemas remains reserved, however any
attempt to access a TIMETZ column in existing tables will cause a node
to panic. Tables containing TIMETZ columns must be completely dropped,
they cannot be recovered.

Release note (backward-incompatible change): Support for PostgreSQL's
TIMETZ data type is removed because it was incomplete/incorrect. This
was a feature only available in previous 2.1-alpha releases but not
any stable release, and thus was experimental anyways. Tables
previously created with the TIMETZ data type must be dropped entirely
-- it is not possible to convert the data or drop a single TIMETZ
column.

cc @awoods187 

Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented Aug 13, 2018

Build succeeded

@craig craig bot merged commit 7c7e748 into cockroachdb:master Aug 13, 2018
@knz knz moved this from Current milestone to Finished (milestone 0731) in (DEPRECATED) SQL Front-end, Lang & Semantics Aug 13, 2018
@knz knz deleted the 20180731-hide-timetz branch August 13, 2018 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants