fix(query): render named-timezone timestamps (#283)

* fix(query): render named-timezone timestamps

Results are fetched as Arrow IPC and formatted client-side, so a
timestamp arrives as a raw integer plus a timezone recorded in the
schema. Formatting one whose zone is a named IANA zone ("UTC",
"America/New_York") needs a timezone database; arrow was built with
default features off, so none was compiled in and its formatter
returned an error for those values. The cell then reported them as
null.

Only named zones were affected: a fixed offset ("+00:00") and a
zone-less timestamp both format without a timezone database, so a
neighbouring column in the same row looked correct and the loss was
easy to miss. Nothing was wrong with the data — the same query served
inline, formatted by the service, returned the values correctly.

Enable arrow's chrono-tz feature, and stop turning a formatting
failure into a null: the cell has already been checked for null above,
so reaching that arm means a present value could not be rendered, and
reporting it as null asserts the opposite. The output shape is kept
(callers rely on one value per column) and the failure is reported on
stderr instead, once per run so a large result cannot flood it.

Two tests cover it: named zones across several time units, and a
sweep over the temporal and decimal types a query can return
asserting none of them render a present value as null. Both fail
without the feature.

Regressed in 50e3439, which moved result fetching from JSON to Arrow.

* fix(query): handle per-value format errors, warn per type

Two review points.

The warn-once flag was global, so a result with two unformattable
column types reported only the first and the second went back to
nulling silently — the exact hazard this removes. Key the set on the
data type, which is what the failure is a property of.

`try_new` only reports a type Arrow cannot format at all. A single
out-of-range value inside a formattable type fails later, in the
value formatter, and the plain `Display` impl under the default
`safe: true` writes the text "ERROR: <msg>" into the cell as if it
were data (and panics when the inner write fails). Render the value
through `try_to_string` so one bad value degrades the same way a bad
type does.

Test covers it: an out-of-range Timestamp(Second) must not panic and
must not put an error message in the data. Without the change it
fails with "ERROR: Cast error: Failed to convert ... to datetime".

Author: anoop-narang

Cargo.lock

@@ -33,7 +33,15 @@ reqwest = { version = "0.13", features = ["blocking", "json"] }
 rayon = "1.10"
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
-arrow = { version = "55", default-features = false, features = ["ipc"] }
+arrow = { version = "55", default-features = false, features = [
+  "ipc",
+  # Timestamps carrying a named IANA zone ("UTC", "America/New_York")
+  # cannot be formatted without a timezone database — arrow's formatter
+  # returns an error for them, and only fixed offsets ("+00:00") parse
+  # without it. Results are fetched as Arrow and formatted here, so
+  # without this feature every named-zone timestamp fails to render.
+  "chrono-tz",
+] }
 serde_yaml = "0.9"
 base64 = "0.22"
 crossterm = "0.28"

Cargo.toml

@@ -111,6 +111,35 @@ fn value_to_string(v: &Value) -> String {
     }
 }
 
+/// Warn that a column could not be rendered, once per data type.
+///
+/// Keyed on the type rather than a single global flag: the failure is a
+/// property of the type, so a million-row result must not print a million
+/// lines — but a result with two unformattable types has to report both, or
+/// the second one goes back to nulling silently.
+fn warn_unformattable_once(data_type: &arrow::datatypes::DataType, err: &arrow::error::ArrowError) {
+    use crossterm::style::Stylize;
+    use std::collections::HashSet;
+    use std::sync::{Mutex, OnceLock};
+
+    static WARNED: OnceLock<Mutex<HashSet<arrow::datatypes::DataType>>> = OnceLock::new();
+    let seen = WARNED.get_or_init(|| Mutex::new(HashSet::new()));
+    // A poisoned lock only means another thread panicked mid-warn; recover the
+    // set rather than take the process down over a diagnostic.
+    let mut seen = match seen.lock() {
+        Ok(g) => g,
+        Err(poisoned) => poisoned.into_inner(),
+    };
+    if !seen.insert(data_type.clone()) {
+        return;
+    }
+    eprintln!(
+        "{}",
+        format!("warning: could not format a {data_type} value, so it is shown as null: {err}")
+            .yellow()
+    );
+}
+
 /// Convert one cell of an Arrow array to a `serde_json::Value`.
 fn arrow_cell(col: &dyn arrow::array::Array, row: usize) -> Value {
     use arrow::array::*;
@@ -222,9 +251,28 @@ fn arrow_cell(col: &dyn arrow::array::Array, row: usize) -> Value {
         _ => {
             use arrow::util::display::{ArrayFormatter, FormatOptions};
             let opts = FormatOptions::default();
-            ArrayFormatter::try_new(col, &opts)
-                .map(|f| Value::String(f.value(row).to_string()))
-                .unwrap_or(Value::Null)
+            // `col.is_null(row)` was checked above, so the cell *has* a value.
+            // Reporting it as `null` claims the opposite and is
+            // indistinguishable from a real null to anything reading the
+            // output — which is how a whole class of unformattable column went
+            // unnoticed. Keep the shape (callers rely on a value per column)
+            // but say so on stderr, deduplicated per type.
+            //
+            // Both failure levels are handled. `try_new` reports a type Arrow
+            // cannot format at all; `try_to_string` reports a single value it
+            // cannot format. The latter matters because the plain `Display`
+            // impl, under the default `safe: true`, writes the text
+            // "ERROR: <msg>" into the cell — an error message wearing the
+            // costume of data — and panics outright when the inner write fails.
+            let formatted =
+                ArrayFormatter::try_new(col, &opts).and_then(|f| f.value(row).try_to_string());
+            match formatted {
+                Ok(s) => Value::String(s),
+                Err(e) => {
+                    warn_unformattable_once(col.data_type(), &e);
+                    Value::Null
+                }
+            }
         }
     }
 }
@@ -723,6 +771,167 @@ mod tests {
         resp
     }
 
+    /// A timestamp carrying a *named* IANA zone must render, not come back as
+    /// null. Rendering it needs a timezone database compiled in; without one
+    /// Arrow's formatter errors, and this cell used to swallow that error and
+    /// report the value as null — silently, and only for named zones, so a
+    /// fixed-offset or zone-less timestamp in the same row looked fine.
+    #[test]
+    fn named_timezone_timestamps_render_instead_of_nulling() {
+        use arrow::array::TimestampMicrosecondArray;
+
+        // 2026-01-01T12:00:00Z
+        let micros = 1_767_268_800_000_000i64;
+
+        for zone in ["UTC", "America/New_York", "Europe/London"] {
+            let col = TimestampMicrosecondArray::from(vec![micros]).with_timezone(zone);
+            let cell = arrow_cell(&col, 0);
+            assert!(
+                cell.is_string(),
+                "zone {zone} rendered as {cell:?}, expected a formatted string"
+            );
+            assert_ne!(cell, Value::Null, "zone {zone} rendered as null");
+        }
+
+        // The two forms that worked even without a timezone database, kept here
+        // so a regression narrows to the named-zone case rather than all timestamps.
+        let offset = TimestampMicrosecondArray::from(vec![micros]).with_timezone("+00:00");
+        assert!(arrow_cell(&offset, 0).is_string());
+        let naive = TimestampMicrosecondArray::from(vec![micros]);
+        assert!(arrow_cell(&naive, 0).is_string());
+
+        // A genuine null is still a null.
+        let with_null = TimestampMicrosecondArray::from(vec![None::<i64>]).with_timezone("UTC");
+        assert_eq!(arrow_cell(&with_null, 0), Value::Null);
+    }
+
+    /// No column type we can be handed may turn a present value into `null`.
+    ///
+    /// The timezone case above is one instance of a wider hazard: every type
+    /// without an explicit arm falls through to Arrow's formatter, and a
+    /// formatter that cannot handle the type used to be reported as a null
+    /// cell. This walks the temporal and decimal types a query can return and
+    /// asserts each renders, so a future dependency or feature change that
+    /// breaks one of them fails here instead of silently blanking a column.
+    #[test]
+    fn no_column_type_silently_renders_as_null() {
+        use arrow::array::{
+            Date32Array, Date64Array, Decimal128Array, DurationMicrosecondArray, Time32SecondArray,
+            Time64MicrosecondArray, TimestampMicrosecondArray, TimestampMillisecondArray,
+            TimestampNanosecondArray, TimestampSecondArray,
+        };
+
+        let micros = 1_767_268_800_000_000i64;
+        let cases: Vec<(&str, arrow::array::ArrayRef)> = vec![
+            ("Date32", Arc::new(Date32Array::from(vec![20454]))),
+            (
+                "Date64",
+                Arc::new(Date64Array::from(vec![1_767_268_800_000])),
+            ),
+            ("Time32(s)", Arc::new(Time32SecondArray::from(vec![43200]))),
+            (
+                "Time64(µs)",
+                Arc::new(Time64MicrosecondArray::from(vec![43_200_000_000])),
+            ),
+            (
+                "Duration(µs)",
+                Arc::new(DurationMicrosecondArray::from(vec![1_000_000])),
+            ),
+            (
+                "Decimal128",
+                Arc::new(
+                    Decimal128Array::from(vec![123_456i128])
+                        .with_precision_and_scale(10, 3)
+                        .expect("valid decimal"),
+                ),
+            ),
+            (
+                "Timestamp(s, naive)",
+                Arc::new(TimestampSecondArray::from(vec![1_767_268_800])),
+            ),
+            (
+                "Timestamp(ms, naive)",
+                Arc::new(TimestampMillisecondArray::from(vec![1_767_268_800_000])),
+            ),
+            (
+                "Timestamp(µs, naive)",
+                Arc::new(TimestampMicrosecondArray::from(vec![micros])),
+            ),
+            (
+                "Timestamp(ns, naive)",
+                Arc::new(TimestampNanosecondArray::from(vec![micros * 1_000])),
+            ),
+            (
+                "Timestamp(s, UTC)",
+                Arc::new(TimestampSecondArray::from(vec![1_767_268_800]).with_timezone("UTC")),
+            ),
+            (
+                "Timestamp(ms, UTC)",
+                Arc::new(
+                    TimestampMillisecondArray::from(vec![1_767_268_800_000]).with_timezone("UTC"),
+                ),
+            ),
+            (
+                "Timestamp(µs, UTC)",
+                Arc::new(TimestampMicrosecondArray::from(vec![micros]).with_timezone("UTC")),
+            ),
+            (
+                "Timestamp(ns, UTC)",
+                Arc::new(TimestampNanosecondArray::from(vec![micros * 1_000]).with_timezone("UTC")),
+            ),
+            (
+                "Timestamp(µs, +05:30)",
+                Arc::new(TimestampMicrosecondArray::from(vec![micros]).with_timezone("+05:30")),
+            ),
+            (
+                "Timestamp(µs, Asia/Kolkata)",
+                Arc::new(
+                    TimestampMicrosecondArray::from(vec![micros]).with_timezone("Asia/Kolkata"),
+                ),
+            ),
+        ];
+
+        for (label, col) in cases {
+            let cell = arrow_cell(col.as_ref(), 0);
+            assert_ne!(
+                cell,
+                Value::Null,
+                "{label} has a value at row 0 but rendered as null"
+            );
+        }
+    }
+
+    /// A single value Arrow cannot format must not panic, and must not put an
+    /// error message into the data.
+    ///
+    /// Two distinct failure levels exist: a type Arrow cannot format at all
+    /// (caught by `try_new`) and one value inside a formattable type that is
+    /// out of range. For the latter, the plain `Display` impl under the default
+    /// `safe: true` writes the literal text "ERROR: <msg>" as the cell value,
+    /// and panics when the inner write itself fails. Neither is acceptable in a
+    /// data column, so the value path is rendered fallibly.
+    #[test]
+    fn an_unformattable_value_is_null_not_a_panic_or_an_error_string() {
+        use arrow::array::TimestampSecondArray;
+
+        // Far outside the range a second-resolution timestamp can render.
+        let col = TimestampSecondArray::from(vec![i64::MAX]);
+        let cell = arrow_cell(&col, 0);
+
+        if let Value::String(s) = &cell {
+            assert!(
+                !s.contains("ERROR"),
+                "an error message reached the data as a value: {s}"
+            );
+        } else {
+            assert_eq!(
+                cell,
+                Value::Null,
+                "expected either a rendered value or null"
+            );
+        }
+    }
+
     #[test]
     fn hint_for_missing_database_context() {
         let tip = cross_source_hint(