Change TIMESTAMP and TIME parsing so that time zone information is preserved (#641)

* 640 Fixing time zone printing format for TIMESTAMP and TIME

* 640 Removing unnecessary changes

* Update src/ast/data_type.rs

Fix comment typo

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

src/ast/data_type.rs

@@ -99,13 +99,11 @@ pub enum DataType {
     /// Date
     Date,
     /// Time
-    Time,
+    Time(TimezoneInfo),
     /// Datetime
     Datetime,
-    /// Timestamp [Without Time Zone]
+    /// Timestamp
-    Timestamp,
+    Timestamp(TimezoneInfo),
-    /// Timestamp With Time Zone
-    TimestampTz,
     /// Interval
     Interval,
     /// Regclass used in postgresql serial
@@ -190,10 +188,9 @@ impl fmt::Display for DataType {
             DataType::DoublePrecision => write!(f, "DOUBLE PRECISION"),
             DataType::Boolean => write!(f, "BOOLEAN"),
             DataType::Date => write!(f, "DATE"),
-            DataType::Time => write!(f, "TIME"),
+            DataType::Time(timezone_info) => write!(f, "TIME{}", timezone_info),
             DataType::Datetime => write!(f, "DATETIME"),
-            DataType::Timestamp => write!(f, "TIMESTAMP"),
+            DataType::Timestamp(timezone_info) => write!(f, "TIMESTAMP{}", timezone_info),
-            DataType::TimestampTz => write!(f, "TIMESTAMPTZ"),
             DataType::Interval => write!(f, "INTERVAL"),
             DataType::Regclass => write!(f, "REGCLASS"),
             DataType::Text => write!(f, "TEXT"),
@@ -240,3 +237,50 @@ fn format_type_with_optional_length(
     }
     Ok(())
 }
+
+/// Timestamp and Time data types information about TimeZone formatting.
+///
+/// This is more related to a display information than real differences between each variant. To
+/// guarantee compatibility with the input query we must maintain its exact information.
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub enum TimezoneInfo {
+    /// No information about time zone. E.g., TIMESTAMP
+    None,
+    /// Temporal type 'WITH TIME ZONE'. E.g., TIMESTAMP WITH TIME ZONE, [standard], [Oracle]
+    ///
+    /// [standard]: https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#datetime-type
+    /// [Oracle]: https://docs.oracle.com/en/database/oracle/oracle-database/12.2/nlspg/datetime-data-types-and-time-zone-support.html#GUID-3F1C388E-C651-43D5-ADBC-1A49E5C2CA05
+    WithTimeZone,
+    /// Temporal type 'WITHOUT TIME ZONE'. E.g., TIME WITHOUT TIME ZONE, [standard], [Postgresql]
+    ///
+    /// [standard]: https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#datetime-type
+    /// [Postgresql]: https://www.postgresql.org/docs/current/datatype-datetime.html
+    WithoutTimeZone,
+    /// Postgresql specific `WITH TIME ZONE` formatting, for both TIME and TIMESTAMP. E.g., TIMETZ, [Postgresql]
+    ///
+    /// [Postgresql]: https://www.postgresql.org/docs/current/datatype-datetime.html
+    Tz,
+}
+
+impl fmt::Display for TimezoneInfo {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            TimezoneInfo::None => {
+                write!(f, "")
+            }
+            TimezoneInfo::WithTimeZone => {
+                write!(f, " WITH TIME ZONE")
+            }
+            TimezoneInfo::WithoutTimeZone => {
+                write!(f, " WITHOUT TIME ZONE")
+            }
+            TimezoneInfo::Tz => {
+                // TZ is the only one that is displayed BEFORE the precision, so the datatype display
+                // must be aware of that. Check <https://www.postgresql.org/docs/14/datatype-datetime.html>
+                // for more information
+                write!(f, "TZ")
+            }
+        }
+    }
+}

src/ast/mod.rs

@@ -29,6 +29,7 @@ use core::fmt;
 use serde::{Deserialize, Serialize};
 
 pub use self::data_type::DataType;
+pub use self::data_type::TimezoneInfo;
 pub use self::ddl::{
     AlterColumnOperation, AlterTableOperation, ColumnDef, ColumnOption, ColumnOptionDef,
     ReferentialAction, TableConstraint,

src/keywords.rs

@@ -528,6 +528,7 @@ define_keywords!(
     TIME,
     TIMESTAMP,
     TIMESTAMPTZ,
+    TIMETZ,
     TIMEZONE,
     TIMEZONE_HOUR,
     TIMEZONE_MINUTE,

src/parser.rs

@@ -3412,22 +3412,27 @@ impl<'a> Parser<'a> {
                 Keyword::TIMESTAMP => {
                     if self.parse_keyword(Keyword::WITH) {
                         self.expect_keywords(&[Keyword::TIME, Keyword::ZONE])?;
-                        Ok(DataType::TimestampTz)
+                        Ok(DataType::Timestamp(TimezoneInfo::WithTimeZone))
                     } else if self.parse_keyword(Keyword::WITHOUT) {
                         self.expect_keywords(&[Keyword::TIME, Keyword::ZONE])?;
-                        Ok(DataType::Timestamp)
+                        Ok(DataType::Timestamp(TimezoneInfo::WithoutTimeZone))
                     } else {
-                        Ok(DataType::Timestamp)
+                        Ok(DataType::Timestamp(TimezoneInfo::None))
                     }
                 }
-                Keyword::TIMESTAMPTZ => Ok(DataType::TimestampTz),
+                Keyword::TIMESTAMPTZ => Ok(DataType::Timestamp(TimezoneInfo::Tz)),
                 Keyword::TIME => {
-                    // TBD: we throw away "with/without timezone" information
+                    if self.parse_keyword(Keyword::WITH) {
-                    if self.parse_keyword(Keyword::WITH) || self.parse_keyword(Keyword::WITHOUT) {
                         self.expect_keywords(&[Keyword::TIME, Keyword::ZONE])?;
+                        Ok(DataType::Time(TimezoneInfo::WithTimeZone))
+                    } else if self.parse_keyword(Keyword::WITHOUT) {
+                        self.expect_keywords(&[Keyword::TIME, Keyword::ZONE])?;
+                        Ok(DataType::Time(TimezoneInfo::WithoutTimeZone))
+                    } else {
+                        Ok(DataType::Time(TimezoneInfo::None))
                     }
-                    Ok(DataType::Time)
                 }
+                Keyword::TIMETZ => Ok(DataType::Time(TimezoneInfo::Tz)),
                 // Interval types can be followed by a complicated interval
                 // qualifier that we don't currently support. See
                 // parse_interval for a taste.
@@ -5265,23 +5270,78 @@ mod tests {
     }
 
     // TODO add tests for all data types? https://github.com/sqlparser-rs/sqlparser-rs/issues/2
+    // TODO when we have dialect validation by data type parsing, split test
     #[test]
     fn test_parse_data_type() {
-        test_parse_data_type("BLOB", "BLOB");
+        // BINARY data type
-        test_parse_data_type("BLOB(50)", "BLOB(50)");
+        test_parse_data_type("BINARY", DataType::Binary(None), "BINARY");
-        test_parse_data_type("CLOB", "CLOB");
+        test_parse_data_type("BINARY(20)", DataType::Binary(Some(20)), "BINARY(20)");
-        test_parse_data_type("CLOB(50)", "CLOB(50)");
-        test_parse_data_type("DOUBLE PRECISION", "DOUBLE PRECISION");
-        test_parse_data_type("DOUBLE", "DOUBLE");
-        test_parse_data_type("VARBINARY", "VARBINARY");
-        test_parse_data_type("VARBINARY(20)", "VARBINARY(20)");
-        test_parse_data_type("BINARY", "BINARY");
-        test_parse_data_type("BINARY(20)", "BINARY(20)");
 
-        fn test_parse_data_type(input: &str, expected: &str) {
+        // BLOB data type
+        test_parse_data_type("BLOB", DataType::Blob(None), "BLOB");
+        test_parse_data_type("BLOB(50)", DataType::Blob(Some(50)), "BLOB(50)");
+
+        // CLOB data type
+        test_parse_data_type("CLOB", DataType::Clob(None), "CLOB");
+        test_parse_data_type("CLOB(50)", DataType::Clob(Some(50)), "CLOB(50)");
+
+        // Double data type
+        test_parse_data_type(
+            "DOUBLE PRECISION",
+            DataType::DoublePrecision,
+            "DOUBLE PRECISION",
+        );
+        test_parse_data_type("DOUBLE", DataType::Double, "DOUBLE");
+
+        // Time data type
+        test_parse_data_type("TIME", DataType::Time(TimezoneInfo::None), "TIME");
+        test_parse_data_type(
+            "TIME WITH TIME ZONE",
+            DataType::Time(TimezoneInfo::WithTimeZone),
+            "TIME WITH TIME ZONE",
+        );
+        test_parse_data_type(
+            "TIME WITHOUT TIME ZONE",
+            DataType::Time(TimezoneInfo::WithoutTimeZone),
+            "TIME WITHOUT TIME ZONE",
+        );
+        test_parse_data_type("TIMETZ", DataType::Time(TimezoneInfo::Tz), "TIMETZ");
+
+        // Timestamp data type
+        test_parse_data_type(
+            "TIMESTAMP",
+            DataType::Timestamp(TimezoneInfo::None),
+            "TIMESTAMP",
+        );
+        test_parse_data_type(
+            "TIMESTAMP WITH TIME ZONE",
+            DataType::Timestamp(TimezoneInfo::WithTimeZone),
+            "TIMESTAMP WITH TIME ZONE",
+        );
+        test_parse_data_type(
+            "TIMESTAMP WITHOUT TIME ZONE",
+            DataType::Timestamp(TimezoneInfo::WithoutTimeZone),
+            "TIMESTAMP WITHOUT TIME ZONE",
+        );
+        test_parse_data_type(
+            "TIMESTAMPTZ",
+            DataType::Timestamp(TimezoneInfo::Tz),
+            "TIMESTAMPTZ",
+        );
+
+        // VARBINARY data type
+        test_parse_data_type("VARBINARY", DataType::Varbinary(None), "VARBINARY");
+        test_parse_data_type(
+            "VARBINARY(20)",
+            DataType::Varbinary(Some(20)),
+            "VARBINARY(20)",
+        );
+
+        fn test_parse_data_type(input: &str, expected_type: DataType, expected_str: &str) {
             all_dialects().run_parser_method(input, |parser| {
-                let data_type = parser.parse_data_type().unwrap().to_string();
+                let data_type = parser.parse_data_type().unwrap();
-                assert_eq!(data_type, expected);
+                assert_eq!(data_type, expected_type);
+                assert_eq!(expected_type.to_string(), expected_str.to_string());
             });
         }
     }

tests/sqlparser_common.rs

@@ -2933,7 +2933,7 @@ fn parse_literal_time() {
     let select = verified_only_select(sql);
     assert_eq!(
         &Expr::TypedString {
-            data_type: DataType::Time,
+            data_type: DataType::Time(TimezoneInfo::None),
             value: "01:23:34".into()
         },
         expr_from_projection(only(&select.projection)),
@@ -2959,16 +2959,13 @@ fn parse_literal_timestamp_without_time_zone() {
     let select = verified_only_select(sql);
     assert_eq!(
         &Expr::TypedString {
-            data_type: DataType::Timestamp,
+            data_type: DataType::Timestamp(TimezoneInfo::None),
             value: "1999-01-01 01:23:34".into()
         },
         expr_from_projection(only(&select.projection)),
     );
 
-    one_statement_parses_to(
+    one_statement_parses_to("SELECT TIMESTAMP '1999-01-01 01:23:34'", sql);
-        "SELECT TIMESTAMP WITHOUT TIME ZONE '1999-01-01 01:23:34'",
-        sql,
-    );
 }
 
 #[test]
@@ -2977,16 +2974,13 @@ fn parse_literal_timestamp_with_time_zone() {
     let select = verified_only_select(sql);
     assert_eq!(
         &Expr::TypedString {
-            data_type: DataType::TimestampTz,
+            data_type: DataType::Timestamp(TimezoneInfo::Tz),
             value: "1999-01-01 01:23:34Z".into()
         },
         expr_from_projection(only(&select.projection)),
     );
 
-    one_statement_parses_to(
+    one_statement_parses_to("SELECT TIMESTAMPTZ '1999-01-01 01:23:34Z'", sql);
-        "SELECT TIMESTAMP WITH TIME ZONE '1999-01-01 01:23:34Z'",
-        sql,
-    );
 }
 
 #[test]

tests/sqlparser_postgres.rs

@@ -139,7 +139,7 @@ fn parse_create_table_with_defaults() {
                     },
                     ColumnDef {
                         name: "last_update".into(),
-                        data_type: DataType::Timestamp,
+                        data_type: DataType::Timestamp(TimezoneInfo::WithoutTimeZone),
                         collation: None,
                         options: vec![
                             ColumnOptionDef {
@@ -212,7 +212,7 @@ fn parse_create_table_from_pg_dump() {
             activebool BOOLEAN DEFAULT true NOT NULL, \
             create_date DATE DEFAULT CAST(now() AS DATE) NOT NULL, \
             create_date1 DATE DEFAULT CAST(CAST('now' AS TEXT) AS DATE) NOT NULL, \
-            last_update TIMESTAMP DEFAULT now(), \
+            last_update TIMESTAMP WITHOUT TIME ZONE DEFAULT now(), \
             release_year public.year, \
             active INT\
         )");