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

Disable timestamp with time zone type for ddl #21096

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

gupteaj
Copy link
Contributor

@gupteaj gupteaj commented Oct 10, 2023

Description

Presto issue #17891

Motivation and Context

When "timestamp with time zone" type used in create statement for iceberg tables, it maps data type to "timestamp". In this case, data insert and query returns misleading/errors based on timestamp type.
In createColumnIdentity(), iceberg data type for "timestamptz" is retuned as timestamp and then in toPrestoType(), it maps to TIMESTAMP.
Similar issue exists with ALTER TABLE ADD COLUMN case.

presto:test> alter table tab2 add column c1 timestamp with time zone;

Presto should disallow create table and alter table cases. Following column description is misleading to the create statement type.

presto:test> CREATE TABLE tab1 (a timestamp with time zone, b integer);
CREATE TABLE
presto:test> show columns from tab1;


 Column |   Type    | Extra | Comment 
--------+-----------+-------+---------
 a      | timestamp |       |         
 b      | integer   |       |         
(2 rows)

Impact

create table & alter table statement having "timestamp with time zone" will return error for iceberg tables.
Iceberg column type timestamptz is not supported

Test Plan

  • Add a new test in IcebergDistributedSmokeTestBase
  • Manual testing
presto:test> create table tab1 (a timestamp with time zone, b integer);
Query 20231010_190153_00002_tjs3h failed: Iceberg column type timestamptz is not supported

presto:test> create table tab2 (a integer, b integer);
CREATE TABLE
presto:test> alter table tab2 add column c timestamp with time zone;
Query 20231011_185736_00005_ae5bc failed: Iceberg column type timestamptz is not supported

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Iceberg Changes
* Timestamp with time zone data type is not allowed in create table and alter table statements. 


@tdcmeehan tdcmeehan self-assigned this Oct 25, 2023
@tdcmeehan
Copy link
Contributor

@ChunxuTang any comments?

Copy link
Member

@ChunxuTang ChunxuTang left a comment

Choose a reason for hiding this comment

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

LGTM. @gupteaj Thanks for the patch.

@ChunxuTang
Copy link
Member

Re-running the failed tests.

@mathrajesh
Copy link

Since the ICEBERG timestamptz feature is disabled from version 0.285, we are stuck with PrestoDB version 0.284 and unable to upgrade. Instead of disabling this feature, which would break our product upgrades, you could make it an optional feature that users can enable if needed, allowing us to continue with upgrades.

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

Successfully merging this pull request may close these issues.

4 participants