[peco-engineer-bot]

Summary

Automated fix for #103 — Fetching structs containing datetimes fails silently.

The server writes STRUCT/MAP/ARRAY values as JSON-like strings, but it leaves nested DATE and TIMESTAMP values unquoted. JSON.parse rejects that, and convertJSON (lib/result/utils.ts) then quietly returned {} or []. Now, when JSON.parse fails, convertJSON puts quotes around any bare date or timestamp outside string literals and parses again. It still returns the default value if that second parse also fails. Results: the 3 new e2e tests in tests/e2e/complex_datetime.test.ts pass against the live warehouse, I added a unit test for edge cases (date-like text inside strings, negative numbers, exponent numbers), and npm test (1528 passing), lint, prettier and type-check all pass.

Why fix this in the client

Runtime uses the legacy Hive string representation for complex values, which intentionally emits nested dates and timestamps without quotes. This format is not guaranteed to be valid JSON; the client’s JSON-only parsing assumption causes the silent data loss. Handling the existing representation in the client fixes already deployed Runtime versions without requiring a server upgrade. Changing Runtime to emit JSON would alter the established wire representation and would still leave older versions needing client handling.

Root cause & plan

Root cause: convertJSON in lib/result/utils.ts (used by convertThriftValue for STRUCT/MAP/ARRAY columns in both JsonResultHandler (arrowEnabled:false) and ArrowResultConverter (useArrowNativeTypes:false, complex types serialized as strings)) calls JSON.parse on the server's Hive-style string serialization of the complex value. That serialization emits nested DATE/TIMESTAMP values unquoted (e.g. {"created_at":2021-12-21 21:33:59.339}), which is not valid JSON, so JSON.parse throws and the catch silently returns the default ({} or []), dropping the whole value.
Files: lib/result/utils.ts, tests/e2e/complex_datetime.test.ts, tests/unit/result/utils.test.ts
Planned coverage:

• tests/e2e/complex_datetime.test.ts (new): open a session with arrowEnabled:false (stubbed getConfig, as in data_types.test.ts) and run SELECT named_struct('id', 1, 'ts', TIMESTAMP'2021-12-21 21:33:59.339', 'd', DATE'2021-12-21', 's', 'x') AS st, CAST(TIMESTAMP'2021-12-21 21:33:59.339' AS STRING) AS ts_str, CAST(DATE'2021-12-21' AS STRING) AS d_str. Assert row.st deep-equals {id:1, ts: row.ts_str, d: row.d_str, s:'x'} (server's own string rendering is the authority for the datetime text). Today st is {} so it fails. (struct with nested timestamp/date, JSON (non-Arrow) path)
• Same spec, arrowEnabled:false: SELECT array(TIMESTAMP'2021-12-21 21:33:59.339', TIMESTAMP'2022-01-01 00:00:00') AS arr, map('a', TIMESTAMP'2021-12-21 21:33:59.339') AS m plus CAST-to-string companions; assert arr equals the two timestamp strings and m equals {a: }. Today arr is [] and m is {}. (array and map with nested timestamps, JSON path)
• Same struct query with useArrowNativeTypes:false (arrowEnabled true) — ArrowResultConverter also goes through convertThriftValue/convertJSON; assert the struct is fully populated with timestamp/date strings instead of {}. (Arrow path without native types (complex types as strings))

Test plan

• tests/e2e/complex_datetime.test.ts: struct with nested timestamp/date is parsed (JSON results) — fails (red) against the original code, passes (green) after the fix
• tests/e2e/complex_datetime.test.ts: array and map with nested timestamps are parsed (JSON results) — fails (red) against the original code, passes (green) after the fix
• tests/e2e/complex_datetime.test.ts: struct with nested timestamp/date is parsed (Arrow results without native types) — fails (red) against the original code, passes (green) after the fix

🤖 Generated by engineer-bot (bug-fix flow) — review before merge.

Not changed in this PR (out of scope)

These facets could not be fixed here. Reason given for each — please follow up as needed:

• Log a warning when complex-value parsing falls back to the default — The issue asks for this as a separate suggestion. convertThriftValue/convertJSON have no logger/context, so adding one would mean changing signatures at every call site (JsonResultHandler, ArrowResultConverter), which goes beyond the minimal fix.

  Pass IClientContext (or a logger) into convertThriftValue and log at warn level in the final catch of convertJSON in lib/result/utils.ts.

[peco-review-bot]

Verdict: 1 Nit

Looks good, no blocking issues. The fallback in quoteBareDateTimes runs only after JSON.parse has already failed, and it skips string literals and escapes. Its regex can't match plain integers, negative numbers or exponent numbers, so the only behavior change is that values which used to come back as {}/[] are now parsed. The missing warn-log when parsing falls back to the default was already there before this PR and can be done as a separate follow-up.

Other findings

• ⚪ Nit — Optional: add a unit case for a MAP keyed by TIMESTAMP/DATE (e.g. {2021-12-21 21:33:59.339:"a"}) and a top-level array ([2021-12-21], where prevSignificant is ''). Both work under the current scanner, but neither is covered by a test.