coord: allow +00:00 as a valid value for timezone

It helps with compatibility with some drivers, in particular Sequelize.

src/coord/src/error.rs

@@ -41,7 +41,7 @@ pub enum CoordError {
     ConstrainedParameter {
         parameter: &'static (dyn Var + Send + Sync),
         value: String,
-        valid_values: Vec<&'static str>,
+        valid_values: Option<Vec<&'static str>>,
     },
     /// The cursor already exists.
     DuplicateCursor(String),
@@ -238,9 +238,10 @@ impl CoordError {
                 ))
             }
             CoordError::Catalog(c) => c.hint(),
-            CoordError::ConstrainedParameter { valid_values, .. } => {
-                Some(format!("Available values: {}.", valid_values.join(", ")))
-            }
+            CoordError::ConstrainedParameter {
+                valid_values: Some(valid_values),
+                ..
+            } => Some(format!("Available values: {}.", valid_values.join(", "))),
             CoordError::Eval(e) => e.hint(),
             CoordError::InvalidAlterOnDisabledIndex(idx) => Some(format!(
                 "To perform this ALTER, first enable the index using ALTER \

src/coord/src/session/vars.rs

@@ -114,10 +114,10 @@ const STANDARD_CONFORMING_STRINGS: ServerVar<bool> = ServerVar {
     description: "Causes '...' strings to treat backslashes literally (PostgreSQL).",
 };
 
-const TIMEZONE: ServerVar<str> = ServerVar {
+const TIMEZONE: ServerVar<TimeZone> = ServerVar {
     // TimeZone has nonstandard capitalization for historical reasons.
     name: static_uncased_str!("TimeZone"),
-    value: "UTC",
+    value: &TimeZone::UTC,
     description: "Sets the time zone for displaying and interpreting time stamps (PostgreSQL).",
 };
 
@@ -167,7 +167,7 @@ pub struct Vars {
     server_version_num: ServerVar<i32>,
     sql_safe_updates: SessionVar<bool>,
     standard_conforming_strings: ServerVar<bool>,
-    timezone: ServerVar<str>,
+    timezone: SessionVar<TimeZone>,
     transaction_isolation: ServerVar<str>,
 }
 
@@ -188,7 +188,7 @@ impl Default for Vars {
             server_version_num: SERVER_VERSION_NUM,
             sql_safe_updates: SessionVar::new(&SQL_SAFE_UPDATES),
             standard_conforming_strings: STANDARD_CONFORMING_STRINGS,
-            timezone: TIMEZONE,
+            timezone: SessionVar::new(&TIMEZONE),
             transaction_isolation: TRANSACTION_ISOLATION,
         }
     }
@@ -312,7 +312,7 @@ impl Vars {
                 return Err(CoordError::ConstrainedParameter {
                     parameter: &CLIENT_MIN_MESSAGES,
                     value: value.into(),
-                    valid_values: ClientSeverity::valid_values(),
+                    valid_values: Some(ClientSeverity::valid_values()),
                 });
             }
         } else if name == DATABASE.name {
@@ -378,10 +378,14 @@ impl Vars {
                 )),
             }
         } else if name == TIMEZONE.name {
-            if UncasedStr::new(value) != TIMEZONE.value {
-                return Err(CoordError::FixedValueParameter(&TIMEZONE));
+            if let Ok(_) = TimeZone::parse(value) {
+                self.timezone.set(value, local)
             } else {
-                Ok(())
+                return Err(CoordError::ConstrainedParameter {
+                    parameter: &TIMEZONE,
+                    value: value.into(),
+                    valid_values: None,
+                });
             }
         } else if name == TRANSACTION_ISOLATION.name {
             Err(CoordError::ReadOnlyParameter(&TRANSACTION_ISOLATION))
@@ -488,8 +492,8 @@ impl Vars {
     }
 
     /// Returns the value of the `timezone` configuration parameter.
-    pub fn timezone(&self) -> &'static str {
-        self.timezone.value
+    pub fn timezone(&self) -> &TimeZone {
+        self.timezone.value()
     }
 
     /// Returns the value of the `transaction_isolation` configuration
@@ -805,3 +809,45 @@ impl Value for ClientSeverity {
         self.as_str().into()
     }
 }
+
+/// List of valid time zones.
+///
+/// Names are following the tz database, but only time zones equivalent
+/// to UTC±00:00 are supported.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum TimeZone {
+    /// UTC
+    UTC,
+    /// Fixed offset from UTC, currently only "+00:00" is supported.
+    /// A string representation is kept here for compatibility with Postgres.
+    FixedOffset(&'static str),
+}
+
+impl TimeZone {
+    fn as_str(&self) -> &'static str {
+        match self {
+            TimeZone::UTC => "UTC",
+            TimeZone::FixedOffset(s) => s,
+        }
+    }
+}
+
+impl Value for TimeZone {
+    const TYPE_NAME: &'static str = "string";
+
+    fn parse(s: &str) -> Result<Self::Owned, ()> {
+        let s = UncasedStr::new(s);
+
+        if s == TimeZone::UTC.as_str() {
+            Ok(TimeZone::UTC)
+        } else if s == "+00:00" {
+            Ok(TimeZone::FixedOffset("+00:00"))
+        } else {
+            Err(())
+        }
+    }
+
+    fn format(&self) -> String {
+        self.as_str().into()
+    }
+}

src/pgwire/src/message.rs

@@ -381,7 +381,7 @@ impl ErrorResponse {
             CoordError::TailOnlyTransaction => SqlState::INVALID_TRANSACTION_STATE,
             CoordError::Transform(_) => SqlState::INTERNAL_ERROR,
             CoordError::UnknownCursor(_) => SqlState::INVALID_CURSOR_NAME,
-            CoordError::UnknownParameter(_) => SqlState::INVALID_SQL_STATEMENT_NAME,
+            CoordError::UnknownParameter(_) => SqlState::UNDEFINED_OBJECT,
             CoordError::UnknownPreparedStatement(_) => SqlState::UNDEFINED_PSTATEMENT,
             CoordError::UnknownLoginRole(_) => SqlState::INVALID_AUTHORIZATION_SPECIFICATION,
             CoordError::UnmaterializableFunction(_) => SqlState::FEATURE_NOT_SUPPORTED,

test/pgtest-mz/vars.pt

@@ -0,0 +1,25 @@
+# Test status codes for errors related to setting session variables.
+
+# Errors shared with Postgres are covered in pgtest/vars.pt.
+
+# FixedValueParameter maps to 22023
+send
+Query {"query": "SET STANDARD_CONFORMING_STRINGS TO false"}
+----
+
+until
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"22023"},{"typ":"M","value":"parameter \"standard_conforming_strings\" can only be set to \"on\""}]}
+ReadyForQuery {"status":"I"}
+
+# InvalidParameterValue maps to 22023
+send
+Query {"query": "SET STANDARD_CONFORMING_STRINGS TO not_a_boolean"}
+----
+
+until
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"22023"},{"typ":"M","value":"parameter \"standard_conforming_strings\" requires a \"boolean\" value"}]}
+ReadyForQuery {"status":"I"}

test/pgtest/vars.pt

@@ -0,0 +1,72 @@
+# Test status codes for errors related to setting session variables.
+
+# FixedValueParameter and InvalidParameterValue are covered
+# in pgtest-mz/vars.pt
+
+# ConstrainedParameter maps to 22023
+send
+Query {"query": "SET TIMEZONE TO bad"}
+----
+
+until
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"22023"},{"typ":"M","value":"invalid value for parameter \"TimeZone\": \"bad\""}]}
+ReadyForQuery {"status":"I"}
+
+# ReadOnlyParameter maps to 55P02
+send
+Query {"query": "SET SERVER_VERSION TO 10.0"}
+----
+
+until
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"55P02"},{"typ":"M","value":"parameter \"server_version\" cannot be changed"}]}
+ReadyForQuery {"status":"I"}
+
+# InvalidParameterType maps to 22023
+send
+Query {"query": "SET STANDARD_CONFORMING_STRINGS TO nonbool"}
+----
+
+# Our message differs from Postgres, so skip it
+until err_field_typs=SC
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"22023"}]}
+ReadyForQuery {"status":"I"}
+
+
+# UnknownParameter maps to 42704
+send
+Query {"query": "SET I_DONT_EXIST TO any_value"}
+----
+
+until
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"42704"},{"typ":"M","value":"unrecognized configuration parameter \"i_dont_exist\""}]}
+ReadyForQuery {"status":"I"}
+
+# client_min_messages sends hint
+send
+Query {"query": "SET client_min_messages TO bad"}
+----
+
+until err_field_typs=SCMH
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"22023"},{"typ":"M","value":"invalid value for parameter \"client_min_messages\": \"bad\""},{"typ":"H","value":"Available values: debug5, debug4, debug3, debug2, debug1, log, notice, warning, error."}]}
+ReadyForQuery {"status":"I"}
+
+# TimeZone does not send hint
+send
+Query {"query": "SET TimeZone TO bad"}
+----
+
+until err_field_typs=SCMH
+ReadyForQuery
+----
+ErrorResponse {"fields":[{"typ":"S","value":"ERROR"},{"typ":"C","value":"22023"},{"typ":"M","value":"invalid value for parameter \"TimeZone\": \"bad\""}]}
+ReadyForQuery {"status":"I"}

test/sqllogictest/timezone.slt

@@ -21,7 +21,7 @@ SET TIME ZONE 'uTc'
 statement ok
 SET TimeZone = 'uTc'
 
-statement error parameter "TimeZone" can only be set to "UTC"
+statement error invalid value for parameter "TimeZone": "bad"
 SET TIME ZONE bad
 
 query T
@@ -34,6 +34,22 @@ SHOW TIME ZONE
 ----
 UTC
 
+statement ok
+SET TimeZone = '+00:00'
+
+statement ok
+SET TIMEZONE to '+00:00'
+
+query T
+SHOW TIMEZONE
+----
++00:00
+
+query T
+SHOW TIME ZONE
+----
++00:00
+
 query T
 SELECT TIMESTAMP '2020-12-21 18:53:49' AT TIME ZONE 'America/New_York'
 ----

test/testdrive/session.td

@@ -88,7 +88,7 @@ contains:parameter "server_version" cannot be changed
 contains:parameter "server_version_num" cannot be changed
 
 ! SET TimeZone = 'nope'
-contains:parameter "TimeZone" can only be set to "UTC"
+contains:invalid value for parameter "TimeZone": "nope"
 
 # The `transaction_isolation` variable has dedicated syntax as mandated by the
 # SQL standard.