fix: timestamp decode in nested value did not consider timezone

bindings/python/tests/blocking/steps/binding.py

@@ -154,24 +154,34 @@ def _(context):
         context.conn.exec(f"set timezone='{tz}'")
         row = context.conn.query_row("select to_datetime('2024-04-16 12:34:56.789')")
         exp = datetime(2024, 4, 16, 12, 34, 56, 789000, tzinfo=tz_expected)
-        assert row.values()[0] == exp, f"Tuple: {row.values()}"
+        assert row.values()[0] == exp, f"datetime(session level tz): {row.values()}"
         context.conn.exec("set timezone='UTC'")
 
-        # wait for release 1.2.839
-        # if DB_VERSION >= (1, 2, 839):
-        #     row = context.conn.query_row(
-        #         f"settings(timezone='{tz}') select to_datetime('2024-04-16 12:34:56.789')"
-        #     )
-        #     exp = datetime(2024, 4, 16, 12, 34, 56, 789000, tzinfo=tz_expected)
-        #     assert row.values()[0] == exp, f"Tuple: {row.values()}"
+        if DB_VERSION >= (1, 2, 839):
+            row = context.conn.query_row(
+                f"settings(timezone='{tz}') select to_datetime('2024-04-16 12:34:56.789')"
+            )
+            assert row.values()[0] == exp, f"datetime(query level tz): {row.values()}"
+
+            row = context.conn.query_row(
+                f"settings(timezone='{tz}') select to_datetime('2024-04-16 12:34:56.789'), 10"
+            )
+            assert row.values()[0] == exp, (
+                f"datetime in Tuple: {row.values()[0]} != {exp}"
+            )
 
         tz_expected = timezone(timedelta(hours=6))
         row = context.conn.query_row(
             f"settings(timezone='{tz}') select to_timestamp_tz('2024-04-16 12:34:56.789 +0600')"
         )
         exp = datetime(2024, 4, 16, 12, 34, 56, 789000, tzinfo=tz_expected)
         exp_bug = datetime(2024, 4, 16, 18, 34, 56, 789000, tzinfo=tz_expected)
-        assert row.values()[0] in (exp, exp_bug), f"Tuple: {row.values()[0]} {exp}"
+        if DB_VERSION >= (1, 2, 840) and os.getenv("BODY_FORMAT") == "json":
+            assert row.values()[0] == exp, f"timestamp_tz: {row.values()[0]} {exp}"
+        else:
+            assert row.values()[0] == exp_bug, (
+                f"timestamp_tz: {row.values()[0]} {exp_bug}"
+            )
 
 
 @then("Select numbers should iterate all rows")

sql/src/value/arrow_decoder.rs

@@ -38,6 +38,33 @@ use crate::error::{ConvertError, Error, Result};
 
 use super::{Interval, NumberValue, Value};
 
+/// The in-memory representation of the MonthDayMicros variant of the "Interval" logical type.
+#[allow(non_camel_case_types)]
+#[repr(C)]
+struct months_days_micros(pub i128);
+
+/// Mask for extracting the lower 64 bits (microseconds).
+const MICROS_MASK: i128 = 0xFFFFFFFFFFFFFFFF;
+/// Mask for extracting the middle 32 bits (days or months).
+const DAYS_MONTHS_MASK: i128 = 0xFFFFFFFF;
+
+impl months_days_micros {
+    #[inline]
+    pub fn months(&self) -> i32 {
+        ((self.0 >> 96) & DAYS_MONTHS_MASK) as i32
+    }
+
+    #[inline]
+    pub fn days(&self) -> i32 {
+        ((self.0 >> 64) & DAYS_MONTHS_MASK) as i32
+    }
+
+    #[inline]
+    pub fn microseconds(&self) -> i64 {
+        (self.0 & MICROS_MASK) as i64
+    }
+}
+
 impl TryFrom<(&ArrowField, &Arc<dyn ArrowArray>, usize, Tz)> for Value {
     type Error = Error;
     fn try_from(
@@ -386,31 +413,3 @@ fn parse_geometry(raw_data: &[u8]) -> Result<String> {
     let wkt = Ewkt::from_wkb(&mut data, WkbDialect::Ewkb)?;
     Ok(wkt.0)
 }
-
-/// The in-memory representation of the MonthDayMicros variant of the "Interval" logical type.
-#[derive(Debug, Copy, Clone, Default, PartialEq, PartialOrd, Ord, Eq, Hash)]
-#[allow(non_camel_case_types)]
-#[repr(C)]
-pub struct months_days_micros(pub i128);
-
-/// Mask for extracting the lower 64 bits (microseconds).
-pub const MICROS_MASK: i128 = 0xFFFFFFFFFFFFFFFF;
-/// Mask for extracting the middle 32 bits (days or months).
-pub const DAYS_MONTHS_MASK: i128 = 0xFFFFFFFF;
-
-impl months_days_micros {
-    #[inline]
-    pub fn months(&self) -> i32 {
-        ((self.0 >> 96) & DAYS_MONTHS_MASK) as i32
-    }
-
-    #[inline]
-    pub fn days(&self) -> i32 {
-        ((self.0 >> 64) & DAYS_MONTHS_MASK) as i32
-    }
-
-    #[inline]
-    pub fn microseconds(&self) -> i64 {
-        (self.0 & MICROS_MASK) as i64
-    }
-}

sql/src/value/format.rs

@@ -45,7 +45,8 @@ impl std::fmt::Display for Value {
     }
 }
 
-// Compatible with Databend, inner values of nested types are quoted.
+// Used as output of cli
+// Compatible with Databend, strings inside nested types are quoted.
 fn encode_value(f: &mut std::fmt::Formatter<'_>, val: &Value, raw: bool) -> std::fmt::Result {
     match val {
         Value::Null => write!(f, "NULL"),
@@ -222,6 +223,8 @@ pub fn display_decimal_256(num: i256, scale: u8) -> String {
 }
 
 impl Value {
+    // for now only used in ORM to fmt values to insert,
+    // for Params, rust use Param::as_sql_string, and py/js bindings are handled in binding code
     pub fn to_sql_string(&self) -> String {
         match self {
             Value::Null => "NULL".to_string(),

sql/src/value/string_decoder.rs

@@ -27,9 +27,9 @@ use std::io::{BufRead, Cursor};
 
 use super::{NumberValue, Value, DAYS_FROM_CE, TIMESTAMP_TIMEZONE_FORMAT};
 
-pub(crate) const NULL_VALUE: &str = "NULL";
-pub(crate) const TRUE_VALUE: &str = "1";
-pub(crate) const FALSE_VALUE: &str = "0";
+const NULL_VALUE: &str = "NULL";
+const TRUE_VALUE: &str = "1";
+const FALSE_VALUE: &str = "0";
 
 impl TryFrom<(&DataType, Option<String>, Tz)> for Value {
     type Error = Error;
@@ -97,20 +97,7 @@ impl TryFrom<(&DataType, String, Tz)> for Value {
                 let d = parse_decimal(v.as_str(), *size)?;
                 Ok(Self::Number(d))
             }
-            DataType::Timestamp => {
-                let naive_dt = NaiveDateTime::parse_from_str(v.as_str(), "%Y-%m-%d %H:%M:%S%.6f")?;
-                let dt_with_tz = match tz.from_local_datetime(&naive_dt) {
-                    LocalResult::Single(dt) => dt,
-                    LocalResult::None => {
-                        return Err(Error::Parsing(format!(
-                            "time {v} not exists in timezone {tz}"
-                        )))
-                    }
-                    LocalResult::Ambiguous(dt1, _dt2) => dt1,
-                };
-                let ts = dt_with_tz.timestamp_micros();
-                Ok(Self::Timestamp(ts, tz))
-            }
+            DataType::Timestamp => parse_timestamp(v.as_str(), tz),
             DataType::TimestampTz => {
                 let t =
                     DateTime::<FixedOffset>::parse_from_str(v.as_str(), TIMESTAMP_TIMEZONE_FORMAT)?;
@@ -127,7 +114,7 @@ impl TryFrom<(&DataType, String, Tz)> for Value {
             DataType::Interval => Ok(Self::Interval(v)),
             DataType::Array(_) | DataType::Map(_) | DataType::Tuple(_) | DataType::Vector(_) => {
                 let mut reader = Cursor::new(v.as_str());
-                let decoder = ValueDecoder {};
+                let decoder = ValueDecoder { timezone: tz };
                 decoder.read_field(t, &mut reader)
             }
             DataType::Nullable(inner) => match inner.as_ref() {
@@ -146,7 +133,9 @@ impl TryFrom<(&DataType, String, Tz)> for Value {
     }
 }
 
-pub(super) struct ValueDecoder {}
+struct ValueDecoder {
+    pub timezone: Tz,
+}
 
 impl ValueDecoder {
     pub(super) fn read_field<R: AsRef<[u8]>>(
@@ -310,16 +299,7 @@ impl ValueDecoder {
         let mut buf = Vec::new();
         reader.read_quoted_text(&mut buf, b'\'')?;
         let v = unsafe { std::str::from_utf8_unchecked(&buf) };
-        let ts = NaiveDateTime::parse_from_str(v, "%Y-%m-%d %H:%M:%S%.6f")?
-            .and_utc()
-            .timestamp_micros();
-        Ok(Value::Timestamp(ts, Tz::UTC))
-    }
-
-    fn read_interval<R: AsRef<[u8]>>(&self, reader: &mut Cursor<R>) -> Result<Value> {
-        let mut buf = Vec::new();
-        reader.read_quoted_text(&mut buf, b'\'')?;
-        Ok(Value::Interval(unsafe { String::from_utf8_unchecked(buf) }))
+        parse_timestamp(v, self.timezone)
     }
 
     fn read_timestamp_tz<R: AsRef<[u8]>>(&self, reader: &mut Cursor<R>) -> Result<Value> {
@@ -330,6 +310,12 @@ impl ValueDecoder {
         Ok(Value::TimestampTz(t))
     }
 
+    fn read_interval<R: AsRef<[u8]>>(&self, reader: &mut Cursor<R>) -> Result<Value> {
+        let mut buf = Vec::new();
+        reader.read_quoted_text(&mut buf, b'\'')?;
+        Ok(Value::Interval(unsafe { String::from_utf8_unchecked(buf) }))
+    }
+
     fn read_bitmap<R: AsRef<[u8]>>(&self, reader: &mut Cursor<R>) -> Result<Value> {
         let mut buf = Vec::new();
         reader.read_quoted_text(&mut buf, b'\'')?;
@@ -467,6 +453,21 @@ impl ValueDecoder {
     }
 }
 
+fn parse_timestamp(ts_string: &str, tz: Tz) -> Result<Value> {
+    let naive_dt = NaiveDateTime::parse_from_str(ts_string, "%Y-%m-%d %H:%M:%S%.6f")?;
+    let dt_with_tz = match tz.from_local_datetime(&naive_dt) {
+        LocalResult::Single(dt) => dt,
+        LocalResult::None => {
+            return Err(Error::Parsing(format!(
+                "time {ts_string} not exists in timezone {tz}"
+            )))
+        }
+        LocalResult::Ambiguous(dt1, _dt2) => dt1,
+    };
+    let ts = dt_with_tz.timestamp_micros();
+    Ok(Value::Timestamp(ts, tz))
+}
+
 fn parse_decimal(text: &str, size: DecimalSize) -> Result<NumberValue> {
     let mut start = 0;
     let bytes = text.as_bytes();