diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index eb46908..7fdde3e 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -310,8 +310,8 @@ jobs: # per layer: once a single layer exhausts it, the pull fails and takes # every other image with it. Layers that did arrive stay in the local # content store, so a second attempt refetches only what is missing and - # costs seconds. The spooling leg is the one that reaches quay.io for - # MinIO, and that is where the dropped connections have been seen. + # costs seconds. The spooling leg pulls the most, MinIO on top of the + # core images, so it is the one most exposed to a dropped connection. - name: Pull the stack images # From the stack directory, as scripts/lib.sh's `compose` wrapper does. # Compose derives the project name from the compose file's directory, diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f4aaf2..a77435a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- `timestamp with time zone` values are delivered as wall time in the session + time zone instead of in UTC: `TimeZone=` when set, otherwise the + coordinator's default, and whatever a later `SET TIME ZONE` chose. Power BI + folds the value it showed back as a plain `TIMESTAMP` literal, which Trino + reads in the session zone, so a DirectQuery slicer on such a column only + selected its rows in a UTC session. Applications reading these columns see + different values unless the session is UTC. `time with time zone` values + follow the same zone, at its current offset because a time has no date, which + is how Trino itself casts `TIME` to `TIME WITH TIME ZONE`. +- The Power BI connector reports that Trino cannot convert `TIME` to + `TIMESTAMP` (`SQL_CONVERT_TIME` without `SQL_CVT_TIMESTAMP`). A DirectQuery + slicer on a time column folded to a comparison against Power BI's base date, + 30 December 1899, while Trino anchors a cast time on the current date, so the + report silently showed no rows. Power BI now refuses that fold with a visible + error instead. The README describes a view-based workaround. + +### Fixed + +- `SQLGetTypeInfo` lists the plain type first among the rows that share a + `DATA_TYPE`: `VARCHAR` for `SQL_WVARCHAR`, `TIME` for `SQL_TYPE_TIME` and + `TIMESTAMP` for `SQL_TYPE_TIMESTAMP`, as the spec's "how closely the data type + maps" ordering requires. The rows were sorted by name, so `SQL_WVARCHAR` led + with `INTERVAL DAY TO SECOND`, and Power Query, which takes the first row as + its `CAST` target, folded a DirectQuery slicer on a text column into + `CAST(... AS INTERVAL DAY TO SECOND)`, which Trino rejected. +- The Power BI connector quotes text constants and doubles any `'` in them. + Power Query hands the constant over unquoted, so a slicer value folded into + `CAST(hello world as VARCHAR)` and failed. Together with the `SQLGetTypeInfo` + fix above this makes DirectQuery slicers on text columns work, so the driver + and the connector have to be upgraded together. +- `INTERVAL YEAR TO MONTH` and `INTERVAL DAY TO SECOND` columns read as text now + return Trino's own rendering, the same text `CAST(... AS VARCHAR)` produces + (`-1-0`, `0 00:00:00.500`). They were parsed into fields and re-rendered + (`-1-00`, `0 00:00:00.5`), so a Power BI DirectQuery slicer on an interval + column, which folds to `cast(col as VARCHAR) = ''`, silently + selected no rows. Reading these columns as `SQL_C_INTERVAL_*` still works: + stackable-odbc-core now converts interval text to those C types. + ## [0.1.2] — 2026-09-01 ### Changed diff --git a/Cargo.lock b/Cargo.lock index f5475e5..fbff71c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -83,9 +83,9 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "aws-lc-rs" -version = "1.17.3" +version = "1.18.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00bdb5da18dac48ca2cc7cd4a98e533e8635a58e2361d13a1a4ee3888e0d72f1" +checksum = "b281d307588d634de920874890732659e2e7672f72b5e10e81badc1a8a83621e" dependencies = [ "aws-lc-sys", "zeroize", @@ -93,9 +93,9 @@ dependencies = [ [[package]] name = "aws-lc-sys" -version = "0.43.0" +version = "0.45.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43103168cc76fe62678a375e722fc9cb3a0146159ac5828bc4f0dfd755c2224c" +checksum = "9bff6c3b54fad79a2e60b8102caf565819711497c1f5f092f49508e2f5c31b27" dependencies = [ "cc", "cmake", @@ -485,7 +485,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1437,7 +1437,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1659,14 +1659,14 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] name = "rustls" -version = "0.23.43" +version = "0.23.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0283386ce02abc0151e1761d08802dfe86c173b0b494af5cbc086574e453da06" +checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" dependencies = [ "aws-lc-rs", "once_cell", @@ -1716,7 +1716,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1727,9 +1727,9 @@ checksum = "f87165f0995f63a9fbeea62b64d10b4d9d8e78ec6d7d51fb2125fda7bb36788f" [[package]] name = "rustls-webpki" -version = "0.103.13" +version = "0.103.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" +checksum = "f3c3cf1d8b1e7d4927e2d154c3fcb02979afb9939629c62cd9048d4f07b60ac2" dependencies = [ "aws-lc-rs", "ring", @@ -1970,8 +1970,8 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stackable-odbc-core" -version = "0.1.0" -source = "git+https://github.com/stackabletech/stackable-odbc-core.git?tag=v0.1.0#23c924489e135d1d3da1d1664ae16bf8656d5aa3" +version = "0.1.1" +source = "git+https://github.com/stackabletech/stackable-odbc-core.git?tag=v0.1.1#4f5e802cf0cded7c75dba8afa905864b9f355302" dependencies = [ "odbc-sys", "snafu", @@ -2086,7 +2086,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -2603,7 +2603,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 663ee78..d48901c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ snafu = "0.9" # reviewable edit to this line instead of whatever the branch happens to point # at. To build against a local checkout, use a `[patch]` in your own # `.cargo/config.toml` rather than editing this line; see CONTRIBUTING.md. -stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.0" } +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.1" } # `time` is needed directly by `query_all_rows_within`, which bounds the login # round trip with `tokio::time::timeout`. It resolves without being declared, # because reqwest enables it, but a direct use must not rely on another crate's @@ -60,7 +60,7 @@ serial_test = "4" # offline FFI tests need. It is default-off because it is test code, so it is # enabled here rather than on the [dependencies] entry above -- that keeps it # out of the shipped cdylib. -stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.0", features = ["test-support"] } +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.1", features = ["test-support"] } [lints.clippy] unwrap_in_result = "deny" diff --git a/README.md b/README.md index a029391..fe6a3ce 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,11 @@ proper entry in the **Get Data** dialog instead of the generic ODBC one. 2. In **File > Options > Security**, allow any extension to load. 3. Restart Power BI Desktop. **Stackable Trino** now appears under **Get Data**. +For incremental refresh on a `timestamp with time zone` column, Trino compares +the `RangeStart` and `RangeEnd` bounds in the session time zone (`TimeZone`), +so partition boundaries follow that zone. Changing `TimeZone` on a dataset that +already has partitions moves the boundaries. + ### Your first query Assuming a Trino instance is reachable on the given host and port: @@ -148,7 +153,7 @@ The authoritative list is `src/backend/types/connect_params.rs`. | `Roles` | No | Authorisation role per catalog, `{catalog:role;catalog2:ALL}` | | `SessionUser` | No | User statements run as, while `User` still authenticates. JDBC's `sessionUser` | | `Path` | No | Default SQL path for resolving unqualified function names | -| `TimeZone` | No | IANA session time zone (`Europe/Berlin`). Unset leaves the coordinator's | +| `TimeZone` | No | IANA session time zone (`Europe/Berlin`). Unset leaves the coordinator's default. `timestamp with time zone` and `time with time zone` values are delivered as wall time in the session zone, which a later `SET TIME ZONE` changes | | `Locale` | No | Locale for locale-dependent formatting, sent as `X-Trino-Language` | | `ClientInfo` | No | Free-form client metadata Trino records against the query | | `TraceToken` | No | Correlation token Trino records against the query | @@ -259,6 +264,19 @@ ignored, so the tool can react instead of trusting a wrong answer. somewhere you never asked for. Set `Catalog` when you connect. - **Row and field size limits are not faked.** Trino can only limit a result set through `LIMIT` in the SQL you wrote. +- **Power BI cannot filter on a time column in DirectQuery.** Picking a value + in a slicer on a `time` column fails with "We couldn't fold the expression to + the data source". Power BI would filter by casting the column to a timestamp + and comparing it with that time on 30 December 1899, while Trino, like ODBC + itself, puts a cast time on today's date, so the filter could never match. + The connector declares the cast unsupported so you see an error instead of an + empty report. To slice on a time of day, expose it as text in a Trino view, + for example `CAST(col_time AS VARCHAR) AS col_time_text`, and slice on that + column. Selecting "(Blank)" still works. Users who cannot create views can + switch the table to Import mode, where the slicer filters Power BI's own copy + of the data. That copy is only as fresh as its last refresh, and Power BI + treats a blank time as equal to midnight, so selecting 12:00:00 AM also shows + rows without a time. - **One isolation level.** Trino catalogs disagree about which levels they accept, so the driver offers the one they all support and refuses the rest up front, rather than letting a query fail later for a reason nobody can see. @@ -307,6 +325,22 @@ the coordinator's chain is refused even when the machine trusts that chain. **Only the first session property applies.** Wrap the value in braces. See [Values that contain a semicolon](#values-that-contain-a-semicolon). +**A Power BI slicer misses a timestamp from the night the clocks go back.** +`timestamp with time zone` values are shown in the session time zone, and in +the hour that repeats, two instants share one wall time. Power BI filters on the +wall time it showed and Trino reads that as the later of the two instants, so a +value from the first of the repeated hours is not selected. + +**Large reads fail with `ABANDONED_QUERY` or `Query not found`.** Trino +abandons a query whose results the client has not fetched within +`query.client.timeout` (5 minutes by default), and later forgets it altogether, +after which the next fetch returns `404 Not Found: Query not found`. A client +that reads slowly enough gets there on a large result. One cause on +Windows is ODBC tracing left switched on: it writes every call to a file and +slows reads down considerably. Turn it off in the ODBC Data Source +Administrator (Tracing tab, **Stop Tracing Now**), including on an on-premises +data gateway. + **The browser login never opens.** Some tools, `pyodbc` among them, tell the driver it may not display anything. The driver reports this rather than hanging. Use `AccessToken` with those tools, or connect through one that allows a prompt. diff --git a/connector/StackableTrinoODBC.pq b/connector/StackableTrinoODBC.pq index 191a999..3940eb3 100644 --- a/connector/StackableTrinoODBC.pq +++ b/connector/StackableTrinoODBC.pq @@ -307,10 +307,10 @@ StackableTrinoODBCImpl = ( SupportsTop = false ], - // Nothing is overridden. An override here silently wins over - // SQLGetInfoW and cannot be corrected by fixing the driver, so the - // record is reserved for what the driver gets wrong, and this group it - // answers honestly: SQL_SQL92_PREDICATES, SQL_AGGREGATE_FUNCTIONS, + // One entry is overridden, SQL_CONVERT_TIME, below. An override here + // silently wins over SQLGetInfoW and cannot be corrected by fixing the + // driver, so the record is otherwise reserved for what the driver gets + // wrong, and this group it answers honestly: SQL_SQL92_PREDICATES, SQL_AGGREGATE_FUNCTIONS, // SQL_SQL92_RELATIONAL_JOIN_OPERATORS, SQL_SQL92_VALUE_EXPRESSIONS and // SQL_IDENTIFIER_QUOTE_CHAR. // @@ -327,7 +327,22 @@ StackableTrinoODBCImpl = ( // Nothing Power Query generates is lost: comparison, IN, // LIKE, BETWEEN, IS NULL, EXISTS and the four join types are all in // the driver's ungated set. - SQLGetInfo = defaultConfig[SQLGetInfo], + // + // SQL_CONVERT_TIME is the driver's answer (every SQL_CVT_* bit) minus + // SQL_CVT_TIMESTAMP, and that is a deliberate misreport: Trino can + // cast TIME to TIMESTAMP. It anchors the time on the current date, as + // ODBC's own conversion tables do ("SQL to C: Time", footnote [c]), + // while Power BI folds a time slicer to + // `cast(col as TIMESTAMP) = CAST('1899-12-30 hh:mm:ss' as TIMESTAMP)`, + // anchoring on its own base date. That filter ran and silently matched + // no row. Without the bit, Power BI refuses the fold and says so ("We + // couldn't fold the expression to the data source"), which is the + // honest outcome for a filter Trino cannot evaluate the way Power BI + // means it. test_folding_contract.py pins the value to the driver's + // answer and checks Trino's anchor is still the current date. + SQLGetInfo = defaultConfig[SQLGetInfo] & [ + SQL_CONVERT_TIME = 0x01FDFFFF + ], SQLColumns = (catalogName, schemaName, tableName, columnName, source) => source, @@ -394,12 +409,16 @@ StackableTrinoODBCImpl = ( // rendering it as Trino's `X'..'` literal // cannot be verified without Power BI Desktop. // + // Power Query hands a text-valued constant to the visitor + // bare, not quoted: confirmed in Power BI Desktop on + // 2026-10-07, where `Cast(_, "VARCHAR")` folded a slicer + // into `CAST(hello world as VARCHAR)`. A text entry therefore + // quotes the value itself and doubles any single quote in + // it, or `O'Brien` ends the literal early. + // // TODO: add UUID, JSON, TIME WITH TIME ZONE and TIMESTAMP - // WITH TIME ZONE. Each has a valid Trino cast, but the - // rendering turns on whether Power Query hands a - // text-valued constant to the visitor already quoted, which - // the VARCHAR entry below also rests on. Confirm that in - // Power BI Desktop, then add all four. + // WITH TIME ZONE, now that the quoting question above is + // settled. Each has a valid Trino cast. Visitor = [ DECIMAL = each Cast(_, "DECIMAL"), INTEGER = each Cast(_, "INTEGER"), @@ -410,7 +429,7 @@ StackableTrinoODBCImpl = ( DOUBLE = each Cast(_, "DOUBLE PRECISION"), BOOLEAN = each Cast(_, "BOOLEAN"), DATE = each Cast(Quote(Date.ToText(_, "yyyy-MM-dd")), "DATE"), - VARCHAR = each Cast(_, "VARCHAR"), + VARCHAR = each Cast(Quote(Text.Replace(_, "'", "''")), "VARCHAR"), // `fffffff`, not `sssssss`: in a custom format string // `s` is the second and `f` is the fractional second, // so the latter spelling renders the second eight times diff --git a/fuzz/README.md b/fuzz/README.md index edca93e..4347f73 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -20,8 +20,8 @@ one: a value a coordinator legitimately sent must fail safe, and where a release build has no overflow checks the same defect returns a wrong answer instead of an error. -- `json_value` covers `json_to_column_value` and the dozen temporal, interval - and decimal scanners under it. This is the half of the read path core does +- `json_value` covers `json_to_column_value` and the temporal and decimal + scanners under it. This is the half of the read path core does not see: core fuzzes `write_column_value`, which turns the resulting `ColumnValue` into the caller's buffer, and nothing covered the step that produces it. diff --git a/fuzz/fuzz_targets/json_value.rs b/fuzz/fuzz_targets/json_value.rs index 00c126e..b5c45be 100644 --- a/fuzz/fuzz_targets/json_value.rs +++ b/fuzz/fuzz_targets/json_value.rs @@ -11,8 +11,8 @@ use trino_rust_client::{TrinoFloat, TrinoInt, TrinoTy}; // // stackable-odbc-core already fuzzes the second half (`write_column_value`, // ColumnValue -> the caller's buffer). Nothing covered the step before it, -// which is where this crate's temporal, interval and decimal parsers live: -// roughly a dozen hand-written scanners over text a Trino coordinator chose. +// which is where this crate's temporal and decimal parsers live: hand-written +// scanners over text a Trino coordinator chose. // Every one of them runs on the server's side of the trust boundary. // // The property is that no input panics. A panic here is caught at the FFI diff --git a/integration-tests/scripts/gen-trino-config.sh b/integration-tests/scripts/gen-trino-config.sh index 9681822..2a6428e 100755 --- a/integration-tests/scripts/gen-trino-config.sh +++ b/integration-tests/scripts/gen-trino-config.sh @@ -19,7 +19,12 @@ set -euo pipefail source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" OUT="$GENERATED/trino" -rm -rf "$OUT" +# Emptied in place, never removed: a running coordinator bind-mounts this +# directory, and replacing it would leave the container holding the deleted +# one, empty, so a restart would find no config at all. setup.sh recreates +# Trino when the assembled content changes (TRINO_CONFIG_HASH). +mkdir -p "$OUT" +find "$OUT" -mindepth 1 -delete mkdir -p "$OUT/catalog" cp -r "$STACK_DIR/trino/base/." "$OUT/" diff --git a/integration-tests/scripts/seed-hive.sh b/integration-tests/scripts/seed-hive.sh index 9473ede..20cb19a 100755 --- a/integration-tests/scripts/seed-hive.sh +++ b/integration-tests/scripts/seed-hive.sh @@ -6,7 +6,7 @@ # suite connecting normally gets `Access Denied: Cannot create schema`. Once the # schema exists and admin owns it, everything else an ordinary connection needs # works without a role: creating tables, writing, reading and granting. So this -# seeds exactly the schema and stops. +# seeds the schema, plus one view no other catalog can provide (below). # # Idempotent, and run on every setup: the file metastore lives in the # coordinator's writable layer, so recreating the container starts it empty. @@ -49,4 +49,22 @@ trino_run() { } trino_run "CREATE SCHEMA IF NOT EXISTS hive.$HIVE_SCHEMA" admin -echo "Seeded hive.$HIVE_SCHEMA" + +# INTERVAL columns, which no table in the stack has: a Hive table cannot store +# the type, and the postgresql catalog does not expose Postgres intervals. A +# view computes them over postgresql.public.types_test, so a Power BI slicer on +# an interval column can be tested next to the text and date/time columns that +# table already has. +trino_run "CREATE OR REPLACE VIEW hive.$HIVE_SCHEMA.interval_test AS +SELECT id, col_varchar, col_integer, + col_date, col_time, col_timestamp, col_timestamptz, + CASE id WHEN 1 THEN INTERVAL '0.5' SECOND + WHEN 5 THEN INTERVAL '1' DAY + WHEN 6 THEN INTERVAL '2' HOUR + WHEN 7 THEN INTERVAL '1' DAY + INTERVAL '30' MINUTE + WHEN 8 THEN INTERVAL '-1' DAY END AS col_interval_ds, + CASE id WHEN 5 THEN INTERVAL '1' YEAR + WHEN 6 THEN INTERVAL '3' MONTH + WHEN 8 THEN INTERVAL '-1' YEAR END AS col_interval_ym +FROM postgresql.public.types_test" admin +echo "Seeded hive.$HIVE_SCHEMA and hive.$HIVE_SCHEMA.interval_test" diff --git a/integration-tests/scripts/setup.sh b/integration-tests/scripts/setup.sh index 595b515..7e20ca7 100755 --- a/integration-tests/scripts/setup.sh +++ b/integration-tests/scripts/setup.sh @@ -55,6 +55,12 @@ echo "=== Assembling the Keycloak realm ===" echo "=== Assembling Trino config ===" "$SCRIPT_DIR/gen-trino-config.sh" +# The assembled config's content, as a label on the trino service +# (compose.yaml): compose recreates a service whose definition changed, so a +# changed fragment reaches a running coordinator without a profile change. +TRINO_CONFIG_HASH="$(cd "$GENERATED/trino" && find . -type f -print0 | sort -z \ + | xargs -0 sha256sum | sha256sum | cut -c1-16)" +export TRINO_CONFIG_HASH # After every generator, before anything is mounted. make_mounts_readable diff --git a/integration-tests/stack/compose.yaml b/integration-tests/stack/compose.yaml index d7a1c2a..11dc1de 100644 --- a/integration-tests/stack/compose.yaml +++ b/integration-tests/stack/compose.yaml @@ -17,6 +17,10 @@ services: # Pinned, never `latest`. The floor is 466: the spooling protocol the # spooling profile needs did not exist before it. image: trinodb/trino:483 + labels: + # Set by scripts/setup.sh from the assembled config. A changed value is a + # changed service definition, so compose recreates the coordinator. + stackable.odbc.trino-config-hash: "${TRINO_CONFIG_HASH:-unset}" ports: - "8443:8443" volumes: @@ -83,7 +87,10 @@ services: minio: profiles: [spooling] - image: quay.io/minio/minio:RELEASE.2025-04-22T22-12-26Z + # pgsty's build of MinIO, as stackabletech/demos uses: pulls of + # quay.io/minio were refused ("unauthorized") in CI on 2026-10-09. pgsty also + # publishes `mc` (below), and both images carry /bin/sh. + image: docker.io/pgsty/minio:RELEASE.2026-08-04T00-00-00Z command: ["server", "/data"] environment: # Interpolated from scripts/lib.sh, which is the single source; the @@ -98,7 +105,7 @@ services: minio-init: profiles: [spooling] - image: quay.io/minio/mc:RELEASE.2025-04-16T18-13-26Z + image: docker.io/pgsty/mc:RELEASE.2026-09-16T00-00-00Z depends_on: minio: condition: service_started diff --git a/integration-tests/stack/postgres/init.sql b/integration-tests/stack/postgres/init.sql index 91cbb86..4dfc1d5 100644 --- a/integration-tests/stack/postgres/init.sql +++ b/integration-tests/stack/postgres/init.sql @@ -81,6 +81,15 @@ INSERT INTO types_test VALUES ( '{"unicode": "日本語"}' ); +-- Rows 5-8: ordinary text values, so a Power BI slicer on col_varchar offers +-- more than the edge cases above. `O'Brien` checks that the connector's +-- Constant visitor escapes a single quote in a folded filter literal. +INSERT INTO types_test (id, col_varchar, col_integer) VALUES + (5, 'apple', 10), + (6, 'banana', 20), + (7, 'cherry', 30), + (8, 'O''Brien', 40); + -- --------------------------------------------------------------------------- -- public schema: relational tables for PK/FK/index testing -- --------------------------------------------------------------------------- diff --git a/integration-tests/suites/registry.py b/integration-tests/suites/registry.py index 005664a..ce38129 100644 --- a/integration-tests/suites/registry.py +++ b/integration-tests/suites/registry.py @@ -109,6 +109,7 @@ def windows_skip_reason(self): # the connector travels with it. deploy=("connector/StackableTrinoODBC.pq",), ), + Suite("pbi slicer semantics", "test_pbi_slicer_semantics.py"), Suite( "tls", "test_tls.py", argv="none", # keycloak.crt is a leaf signed by the same CA, used as a trust anchor diff --git a/integration-tests/suites/test_folding_contract.py b/integration-tests/suites/test_folding_contract.py index 5fb8f5b..aa83cd4 100644 --- a/integration-tests/suites/test_folding_contract.py +++ b/integration-tests/suites/test_folding_contract.py @@ -74,6 +74,10 @@ # as a folding gap misreports a gap that cannot exist. DM_COMPAT_ONLY = {"SQL_CHAR", "SQL_VARCHAR"} +# SQL_CVT_* bits (sqlext.h): every defined conversion target, and TIMESTAMP's. +SQL_CVT_ALL = 0x01FFFFFF +SQL_CVT_TIMESTAMP = 0x00020000 + def check(label, ok, detail=""): R.check(f"{label}{detail}", ok) @@ -117,6 +121,39 @@ def parse_temporal_formats(source): ) +def parse_text_entries(source): + """Map each Constant-visitor key that casts to VARCHAR to its whole body. + + `VARCHAR = each Cast(Quote(Text.Replace(_, "'", "''")), "VARCHAR")` yields + `{"VARCHAR": 'Cast(Quote(Text.Replace(_, "\\'", "\\'\\'")), "VARCHAR")'}`. + """ + return dict( + re.findall(r'(\w+)\s*=\s*each\s+(Cast\(.*?"VARCHAR"\s*\))', source) + ) + + +def render_text_constant(body, value): + """The SQL a text visitor entry produces for `value`. + + Power Query hands the visitor the bare text, not a quoted literal: verified + in Power BI Desktop on 2026-10-07, where the entry `Cast(_, "VARCHAR")` + produced `CAST(hello world as VARCHAR)`. So the entry has to quote the value + itself, and double any single quote inside it, or a value such as `O'Brien` + ends the literal early. This mirrors exactly those two steps, read off the + entry's body, so the check below fails for an entry that skips either. + """ + quoted = "Quote(" in body + escaped = re.search(r"""Text\.Replace\(\s*_\s*,\s*"'"\s*,\s*"''"\s*\)""", body) + text = value.replace("'", "''") if escaped else value + return f"'{text}'" if quoted else text + + +# Slicer values a text constant must survive. The first three are what Power BI +# sent from `postgresql.public.types_test.col_varchar` on 2026-10-07 (the empty +# string included); the apostrophe is the one that needs escaping. +TEXT_SAMPLES = ["hello world", "日本語テスト 🎉🦀 café résumé", "", "O'Brien"] + + # The .NET custom date/time specifiers the connector is allowed to use, longest # first so `mm` is matched before `m` would be. Anything else is rejected rather # than guessed at: an unrecognised specifier is exactly the defect this looks @@ -296,6 +333,29 @@ def main(): f" {str(e)[:90]}", ) + # ------------------------------------------------------------------ + print("\n--- the text literals the Constant visitor renders round-trip ---") + # A slicer on a text column folds into `"col" = `, and the + # constant is whatever this entry renders. Unquoted, every value but NULL + # is a syntax error; quoted but unescaped, an apostrophe ends the literal. + # Round-tripped through Trino rather than pattern-matched, because what + # matters is the value Trino compares against. + text_entries = parse_text_entries(source) + check( + "the Constant visitor has a VARCHAR entry", + bool(text_entries), + "" if text_entries else " (none parsed)", + ) + for key in sorted(text_entries): + for value in TEXT_SAMPLES: + literal = render_text_constant(text_entries[key], value) + label = f"{key} renders {value!r} as {literal!r}, which CASTs back to it" + try: + got = cur.execute(f"SELECT CAST({literal} AS VARCHAR)").fetchone()[0] + check(label, got == value, "" if got == value else f" (got {got!r})") + except pyodbc.Error as e: + check(label, False, f" {str(e)[:90]}") + # ------------------------------------------------------------------ print("\n--- the row-limiting clause the AstVisitor builds parses ---") rendered = parse_limit_clause(source) @@ -349,6 +409,41 @@ def main(): "" if bindings_on else " (set false, which disables a function the driver declares)", ) + # ------------------------------------------------------------------ + print("\n--- TIME -> TIMESTAMP is withheld, so Power BI cannot fold it ---") + # A Power BI slicer on a time column folds to + # `cast("col" as TIMESTAMP) = CAST('1899-12-30 hh:mm:ss' as TIMESTAMP)`, + # anchoring the time on Power BI's base date. Trino's cast, like ODBC's own + # conversion tables ("SQL to C: Time" footnote [c]), uses the current date, + # so the filter silently matched nothing. Withholding SQL_CVT_TIMESTAMP + # from SQL_CONVERT_TIME makes Power BI refuse the fold with a visible error + # instead (measured in Power BI Desktop, 2026-10-07). The override is a + # deliberate misreport, so it must stay exactly "the driver's answer minus + # that one bit" and must be revisited if Trino's anchor ever changes. + override = re.search(r"SQL_CONVERT_TIME\s*=\s*(0x[0-9A-Fa-f]+|\d+)", source) + check( + "the connector overrides SQL_CONVERT_TIME", + override is not None, + "" if override else " (no SQL_CONVERT_TIME entry in the SQLGetInfo record)", + ) + if override: + declared = int(override.group(1), 0) + driver_answer = conn.getinfo(pyodbc.SQL_CONVERT_TIME) & SQL_CVT_ALL + expected = driver_answer & ~SQL_CVT_TIMESTAMP + check( + "SQL_CONVERT_TIME is the driver's answer without SQL_CVT_TIMESTAMP", + declared == expected, + f" (declared {declared:#010x}, expected {expected:#010x})", + ) + anchored_today = cur.execute( + "SELECT CAST(CAST(TIME '14:30:00' AS TIMESTAMP) AS DATE) = current_date" + ).fetchone()[0] + check( + "Trino still anchors TIME -> TIMESTAMP on the current date", + bool(anchored_today), + "" if anchored_today else " (it no longer does: revisit the SQL_CONVERT_TIME override)", + ) + # ------------------------------------------------------------------ print("\n--- driver types with no Constant visitor entry ---") # Not a failure: an absent key makes Power Query evaluate that constant diff --git a/integration-tests/suites/test_pbi_slicer_semantics.py b/integration-tests/suites/test_pbi_slicer_semantics.py new file mode 100644 index 0000000..cdda35b --- /dev/null +++ b/integration-tests/suites/test_pbi_slicer_semantics.py @@ -0,0 +1,145 @@ +#!/usr/bin/env python3 +""" +Power BI slicer semantics: does the filter Power BI folds select the value it showed? + +A Power BI DirectQuery slicer shows each value the driver delivers, and when one is +picked it folds a filter built from that shown value. The filter only works if Trino, +evaluating it, arrives back at the same value. Whether it does depends on the +column's type, and for several types it silently does not: the filter runs, finds no +rows, and the report shows nothing, with no error anywhere. + +This reproduces Power BI's filters through the driver and checks the row count. The +templates below are copied verbatim from the SQL Power BI Desktop sent to Trino on +2026-10-07 (DirectQuery, the Stackable connector, `hive.tx.interval_test`); the +expected count is computed here from the fetched rows, so no comparison inside Trino +is trusted to define the answer. + +Everything runs twice, in the server's default session time zone and in +`Europe/Berlin`, because a timestamp-with-time-zone filter depends on it. + +Usage: + uv run --with pyodbc python3 integration-tests/suites/test_pbi_slicer_semantics.py "" + +Requires a running Trino (integration-tests/setup.sh), whose seed-hive.sh creates the +`hive.tx.interval_test` view this reads. Needs no compose profile. +""" + +import datetime +import os +import sys + +import pyodbc + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +from harness import Results, Stack # noqa: E402 + +R = Results("pbi slicer semantics") + +VIEW = "hive.tx.interval_test" + + +def text(v): + return v.replace("'", "''") + + +def fraction7(v): + """Power BI renders seconds with seven fractional digits (100 ns ticks).""" + return f"{v.microsecond:06d}0" + + +# column -> the WHERE clause Power BI folded for a slicer value `v`, as captured. +TEMPLATES = { + "col_varchar": lambda v: f"\"col_varchar\" = CAST('{text(v)}' as VARCHAR)", + "col_date": lambda v: f"\"col_date\" = CAST('{v:%Y-%m-%d}' as DATE)", + "col_timestamp": lambda v: ( + f"\"col_timestamp\" = CAST('{v:%Y-%m-%d %H:%M:%S}.{fraction7(v)}' as TIMESTAMP)" + ), + "col_interval_ds": lambda v: ( + f"cast(\"col_interval_ds\" as VARCHAR) = CAST('{text(v)}' as VARCHAR)" + ), + "col_interval_ym": lambda v: ( + f"cast(\"col_interval_ym\" as VARCHAR) = CAST('{text(v)}' as VARCHAR)" + ), + # Power BI anchors a time of day on its base date, 1899-12-30, and folds + # `cast("col_time" as TIMESTAMP) = CAST('1899-12-30 hh:mm:ss' as TIMESTAMP)`. + # Trino's cast uses the current date, as ODBC's conversion tables do, so + # that filter can never match. The connector therefore withholds + # SQL_CVT_TIMESTAMP from SQL_CONVERT_TIME and Power BI refuses the fold with + # a visible error; test_folding_contract.py checks that. Only "(Blank)" is + # checked here. + "col_time": None, + "col_timestamptz": lambda v: ( + f"\"col_timestamptz\" = CAST('{v:%Y-%m-%d %H:%M:%S}.{fraction7(v)}' as TIMESTAMP)" + ), +} + + +def check_zone(conn_str, zone_label): + print(f"\n--- session time zone: {zone_label} ---") + conn = pyodbc.connect(conn_str, autocommit=True) + cur = conn.cursor() + for col, template in TEMPLATES.items(): + pairs = cur.execute(f"SELECT id, {col} FROM {VIEW}").fetchall() + values = [] + for _, x in pairs: + if x is not None and x not in values: + values.append(x) + if template is None: + values = [] + for v in values: + expected = sum(1 for _, x in pairs if x == v) + where = template(v) + label = f"[{zone_label}] {col} = {v!r} selects its rows" + try: + got = cur.execute(f"SELECT count(*) FROM {VIEW} WHERE {where}").fetchone()[0] + R.check(label, got == expected, "" if got == expected + else f"got {got}, expected {expected} (WHERE {where})") + except pyodbc.Error as e: + R.check(label, False, f"{str(e)[:120]} (WHERE {where})") + # A slicer's "(Blank)" folds to `is null`; that already works and must keep working. + nulls = sum(1 for _, x in pairs if x is None) + got = cur.execute(f"SELECT count(*) FROM {VIEW} WHERE \"{col}\" is null").fetchone()[0] + R.check(f"[{zone_label}] {col} (Blank) selects its rows", got == nulls, + "" if got == nulls else f"got {got}, expected {nulls}") + cur.close() + conn.close() + + +def check_dst_overlap(conn_str): + """Pin what a slicer does with a value in the autumn overlap hour. + + The driver delivers both instants of 2025-10-26 02:30 in Europe/Berlin as the + same wall time, because an instant has only one. Reading the folded literal + back, Trino has to pick one of the two instants and picks the later (CET) + one, so the earlier (CEST) row cannot be selected. That is Trino's reading of + an ambiguous local time, not the driver's, and this records it. + """ + print("\n--- DST overlap, Europe/Berlin ---") + conn = pyodbc.connect(conn_str + ";TimeZone=Europe/Berlin", autocommit=True) + cur = conn.cursor() + rows = ("(VALUES (1, from_iso8601_timestamp('2025-10-26T00:30:00Z')), " + "(2, from_iso8601_timestamp('2025-10-26T01:30:00Z'))) t(id, ts)") + shown = [v for _, v in cur.execute(f"SELECT id, ts FROM {rows} ORDER BY id").fetchall()] + want = datetime.datetime(2025, 10, 26, 2, 30) + R.check("both overlap instants are shown as 02:30", shown == [want, want], + "" if shown == [want, want] else f"shown {shown}") + where = "ts = CAST('2025-10-26 02:30:00.0000000' as TIMESTAMP)" + ids = [r[0] for r in cur.execute(f"SELECT id FROM {rows} WHERE {where} ORDER BY id")] + R.check("the folded 02:30 selects only the later (CET) instant", ids == [2], + "" if ids == [2] else f"selected ids {ids}") + cur.close() + conn.close() + + +def main(): + conn_str = sys.argv[1] if len(sys.argv) > 1 else Stack.load().conn_str() + print(f"=== pbi slicer semantics ===\nview: {VIEW}") + check_zone(conn_str, "server default") + check_zone(conn_str + ";TimeZone=Europe/Berlin", "Europe/Berlin") + check_dst_overlap(conn_str) + return R.summary() + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/integration-tests/suites/test_sql_surface.py b/integration-tests/suites/test_sql_surface.py index cde5b31..277037e 100755 --- a/integration-tests/suites/test_sql_surface.py +++ b/integration-tests/suites/test_sql_surface.py @@ -289,6 +289,20 @@ def prepared_handle_is_reusable(): cur.getTypeInfo().fetchall() or (_ for _ in ()).throw(AssertionError("no type info")))) + def type_info_leads_wvarchar_with_varchar(): + """Power Query takes the first SQLGetTypeInfo row for a DATA_TYPE as its + CAST target. For SQL_WVARCHAR that must be VARCHAR, not one of the + Trino types the driver also renders as text (INTERVAL DAY TO SECOND + sorted first before core ranked a preferred row). + """ + SQL_WVARCHAR = -9 # as in test_describe_param.py + first = cur.getTypeInfo(SQL_WVARCHAR).fetchone() + assert first is not None, "no SQL_WVARCHAR rows" + assert first[0] == "VARCHAR", f"first SQL_WVARCHAR row is {first[0]!r}" + + R.run("SQLGetTypeInfo leads SQL_WVARCHAR with VARCHAR", + type_info_leads_wvarchar_with_varchar) + def datetime_columns_report_the_verbose_type(): """SQLColumns and SQLGetTypeInfo must not disagree about a datetime. diff --git a/integration-tests/suites/test_type_matrix.py b/integration-tests/suites/test_type_matrix.py index e4c3883..c949860 100755 --- a/integration-tests/suites/test_type_matrix.py +++ b/integration-tests/suites/test_type_matrix.py @@ -134,8 +134,9 @@ ("timestamp tz", "CAST('2020-02-03 04:05:06 UTC' AS TIMESTAMP WITH TIME ZONE)", None), ("uuid", "CAST('12151fd2-7586-11e9-8f9e-2a86e4085a59' AS UUID)", None), ("json", "CAST('{\"a\":1}' AS JSON)", None), - ("interval day", "INTERVAL '2' DAY", None), - ("interval year", "INTERVAL '2' YEAR", None), + # Trino's own text, which is what Power BI compares a slicer value against. + ("interval day", "INTERVAL '2' DAY", "2 00:00:00.000"), + ("interval year", "INTERVAL '2' YEAR", "2-0"), ("array", "ARRAY[1,2,3]", None), ("row", "CAST(ROW(1,'a') AS ROW(x INTEGER, y VARCHAR))", None), ] diff --git a/integration-tests/windows/WINDOWS.md b/integration-tests/windows/WINDOWS.md index 6848f63..bb140e1 100644 --- a/integration-tests/windows/WINDOWS.md +++ b/integration-tests/windows/WINDOWS.md @@ -119,6 +119,17 @@ which can predate the feature under test by days. | `--gateway ` | `$ODBC_TEST_HOST_GATEWAY`, else `192.168.197.1` | The host-only gateway the VM reaches the host on. `scripts/gen-certs.sh` reads the same environment variable, so the coordinator's certificate covers the address the VM connects to | | `--trino-host
` | the same as `--gateway` | Where the VM reaches Trino. The name `trino` is mapped to it in the VM's hosts file | | `--suite ` | unset | Run only the suites whose name contains the substring. `run-tests.sh --suite` forwards to this | +| `--allow-dm-trace` | off | Run even when ODBC Driver Manager tracing is on in the VM. Without it the run stops; see below | + +**Driver Manager tracing must be off.** The harness checks the three places +Windows keeps the switch (the current user's, the machine's and the 32-bit +Driver Manager's `ODBC.INI\ODBC` key) and stops if any has `Trace=1`. Tracing +writes every ODBC call to a file, which slows reads down so much that Trino can +abandon a long one (`ABANDONED_QUERY`). It is easy to leave on after a +diagnosis, and the result looks like a driver problem. Turn it off in the ODBC +Data Source Administrator (Tracing tab, **Stop Tracing Now**) or set `Trace` to +`0` under the key the error names. `--allow-dm-trace` is for a run where +tracing is the point. The verified configurations connect to `trino` rather than to an address. TLS sends no SNI for an IP literal, and Jetty then serves Trino's internal diff --git a/integration-tests/windows/windows_test.py b/integration-tests/windows/windows_test.py index b2ef9a6..bcd2585 100644 --- a/integration-tests/windows/windows_test.py +++ b/integration-tests/windows/windows_test.py @@ -127,6 +127,8 @@ def main(): # Wait for VM setup to complete (Python, pyodbc installed). wait_for_setup(session) + check_dm_tracing(session, args.allow_dm_trace) + deploy(session, args.gateway, dll_path) # Before `register_driver`, because the uninstaller it runs deregisters the @@ -419,6 +421,12 @@ def parse_args(): default="", help="only run suites whose name contains this string", ) + p.add_argument( + "--allow-dm-trace", + action="store_true", + help="run even when ODBC Driver Manager tracing is on in the VM " + "(it slows every ODBC call; for deliberate diagnosis only)", + ) return p.parse_args() @@ -563,9 +571,15 @@ def run_remote(session, script: str, argv) -> int: """Run one suite on the VM and return its exit code.""" args = " ".join(ps_quote(a) for a in argv) # Enable driver-side debug logging and DM tracing. + # + # PYTHONIOENCODING: over WinRM the suite's stdout is not a console, so + # Python would encode it as cp1252, which cannot represent the non-Latin + # test values some suites print (a UnicodeEncodeError ends the suite). The + # output is decoded as UTF-8 below, so it is encoded as UTF-8 here. r = session.run_ps( f'$env:ODBC_LOG_LEVEL = "debug"; ' f'$env:ODBC_LOG_FILE = "{REMOTE_LOG}"; ' + f'$env:PYTHONIOENCODING = "utf-8"; ' f'& {REMOTE_PYTHON} "{REMOTE_SUITES}\\{script}" {args}' ) stdout = r.std_out.decode("utf-8", errors="replace") @@ -649,6 +663,50 @@ def _port_available(port: int) -> bool: return False +# Where the Driver Manager reads its tracing switch: the current user (the +# ODBC Data Source Administrator's Tracing tab), the machine, and the 32-bit +# Driver Manager's own copy. +DM_TRACE_KEYS = ( + r"HKCU:\Software\ODBC\ODBC.INI\ODBC", + r"HKLM:\SOFTWARE\ODBC\ODBC.INI\ODBC", + r"HKLM:\SOFTWARE\WOW6432Node\ODBC\ODBC.INI\ODBC", +) + + +def check_dm_tracing(session, allowed: bool): + """Refuse to run while ODBC Driver Manager tracing is on in the VM. + + Tracing writes every ODBC call to a file, which slows reads down so much + that Trino can abandon a long one (ABANDONED_QUERY): timing-sensitive + suites then fail in a way that looks like a driver problem. Tracing is + easy to leave on after a diagnosis, and nothing else here would say so. + """ + keys = ", ".join(f"'{k}'" for k in DM_TRACE_KEYS) + r = session.run_ps( + f"foreach ($k in {keys}) {{ " + f"$p = Get-ItemProperty $k -ErrorAction SilentlyContinue; " + f"if ($p -and \"$($p.Trace)\" -eq '1') {{ \"$k -> $($p.TraceFile)\" }} }}" + ) + on = [line for line in r.std_out.decode("utf-8", errors="replace").splitlines() if line.strip()] + if not on: + return + message = "ODBC Driver Manager tracing is ON in the VM (Trace=1):\n" + "\n".join( + f" {line}" for line in on + ) + if allowed: + print(f"WARNING: {message}\n continuing because of --allow-dm-trace; " + "expect every suite to run far slower", file=sys.stderr) + return + print( + f"ERROR: {message}\n" + "Tracing slows every ODBC call enough for Trino to abandon long reads.\n" + "Turn it off (ODBC Data Source Administrator > Tracing > Stop Tracing Now, " + "or set Trace to 0 under the key above), or pass --allow-dm-trace.", + file=sys.stderr, + ) + sys.exit(1) + + def check_installers(session): """Run the shipped install.bat and uninstall.bat, and check what they left. diff --git a/src/backend.rs b/src/backend.rs index db2c2b0..dc154ed 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -12,6 +12,8 @@ use std::time::Duration; use snafu::Snafu; use stackable_odbc_core::types::QueryTimeout; + +use crate::type_conversion::SessionZone; use stackable_odbc_core::{ backend::Backend, errors::OdbcError, @@ -127,6 +129,36 @@ fn session_user_name( .unwrap_or_default() .to_string() } + +/// Trino's session property for a `SET TIME ZONE`, which the coordinator +/// returns as `X-Trino-Set-Session` and clears again on `SET TIME ZONE LOCAL` +/// (`SetTimeZoneTask.java`, `SystemSessionProperties.TIME_ZONE_ID`). +const TIME_ZONE_ID_PROPERTY: &str = "time_zone_id"; + +/// The session time zone a statement's timestamp-with-time-zone values are +/// delivered in. +/// +/// In the order Trino itself applies them: a `SET TIME ZONE` in force +/// (`time_zone_id`), else `TimeZone=` (sent as `X-Trino-Time-Zone`), else the +/// coordinator's default, read at connect. A `time_zone_id` this driver cannot +/// read falls through rather than failing the fetch. +fn effective_session_zone( + time_zone_id: Option<&str>, + configured: Option, + server_default: SessionZone, +) -> SessionZone { + let set = time_zone_id.and_then(|id| { + let zone = SessionZone::parse(id); + if zone.is_none() { + tracing::warn!( + time_zone_id = id, + "unrecognised session time zone; ignoring it" + ); + } + zone + }); + set.or(configured).unwrap_or(server_default) +} // `pub(crate)` only under `cfg(test)`: the FFI integration tests // (`ffi_integration_tests.rs`, a sibling of this module under `lib.rs`) need to // reach the `TRINO_*` capability bitmap constants declared in `info`. Non-test @@ -608,7 +640,10 @@ fn connection_failed(e: TrinoError) -> TrinoError { /// two are read into the result independently rather than through a shared /// early return. fn probe_session(conn: &TrinoConnection) -> SessionProbe { - let rows = match query_all_rows(conn, "SELECT version(), current_user".to_string()) { + let rows = match query_all_rows( + conn, + "SELECT version(), current_user, current_timezone()".to_string(), + ) { Ok(rows) => rows, Err(e) => { tracing::warn!(error = %e, "could not read the Trino server version and session user"); @@ -621,11 +656,21 @@ fn probe_session(conn: &TrinoConnection) -> SessionProbe { let mut probe = SessionProbe { user: column(1).map(str::to_string), + time_zone: column(2).and_then(SessionZone::parse), ..SessionProbe::default() }; if probe.user.is_none() { tracing::warn!("SELECT current_user returned no usable value"); } + if probe.time_zone.is_none() { + // Falls back to UTC, which shifts every timestamp-with-time-zone value + // by the server's offset, so it must not pass silently. + tracing::warn!( + current_timezone = column(2), + "SELECT current_timezone() returned no zone this driver can read; \ + delivering time-zone values in UTC unless TimeZone= is set" + ); + } let Some(raw) = column(0) else { tracing::warn!("SELECT version() returned no usable value"); @@ -666,6 +711,10 @@ struct SessionProbe { server_major: u32, /// Trino's `current_user`, for `SQL_USER_NAME`. user: Option, + /// Trino's `current_timezone()`: the session zone before any + /// `SET TIME ZONE`, which is `TimeZone=` when given and the coordinator's + /// default otherwise. + time_zone: Option, } /// The Trino [`stackable_odbc_core::backend::Backend`] implementation. @@ -711,6 +760,11 @@ pub struct TrinoConnection { /// off. Understating capability is the safe direction: a BI tool folds /// less than it could, rather than emitting SQL the server rejects. pub server_major: u32, + /// `TimeZone=` from the connection string, sent as `X-Trino-Time-Zone`. + pub configured_time_zone: Option, + /// The session zone `current_timezone()` reported at connect, or UTC when + /// the probe failed. Without `TimeZone=` this is the coordinator's default. + pub server_time_zone: SessionZone, /// The `DSN` the application connected with, for `SQL_DATA_SOURCE_NAME`. /// /// Empty when the connection string named no DSN, which is the one case the @@ -892,6 +946,23 @@ impl TrinoConnection { .is_active() } + /// The session time zone for a statement about to run. + /// + /// Read per statement, because `SET TIME ZONE` changes it after connect and + /// the client only records that as a session property. See + /// [`effective_session_zone`] for the order of the sources. + pub(crate) fn session_zone(&self) -> SessionZone { + let snapshot = self.runtime.block_on(self.client.session_snapshot()); + effective_session_zone( + snapshot + .properties + .get(TIME_ZONE_ID_PROPERTY) + .map(String::as_str), + self.configured_time_zone, + self.server_time_zone, + ) + } + /// Open a transaction if manual-commit mode wants one and none is open. /// /// Called from the statement paths rather than from @@ -1054,6 +1125,8 @@ pub(crate) fn disconnected_trino_conn_with_catalog(catalog: Option<&str>) -> Tri // leave behind if `fetch_server_version` could not reach a coordinator. dbms_version: String::new(), server_major: 0, + configured_time_zone: None, + server_time_zone: SessionZone::UTC, // The three identity strings, matching the client built above rather // than left empty: `TrinoBackend::connect` derives the first two // without reaching the coordinator, so a fabricated connection that @@ -1103,6 +1176,9 @@ pub struct TrinoStatement { pub(crate) columns: Vec, /// Column types from Trino (needed for converting subsequent pages). trino_types: Vec<(String, trino_rust_client::TrinoTy)>, + /// The session time zone at execution, which every page of this result + /// set is converted in, so one result set never mixes two zones. + session_zone: SessionZone, /// Trino's own column metadata for the result set, kept because a spooled /// segment is decoded against it and Trino sends it on one page only. /// @@ -1723,6 +1799,8 @@ impl Backend for TrinoBackend { client: Arc::new(client), dbms_version: String::new(), server_major: 0, + configured_time_zone: p.time_zone().map(SessionZone::Named), + server_time_zone: SessionZone::UTC, // Core supplies the DSN on both connection entry points, so this is // the whole of `SQL_DATA_SOURCE_NAME`: absent means the application // connected by driver rather than by DSN, which is the case the @@ -1742,6 +1820,9 @@ impl Backend for TrinoBackend { let probe = probe_session(&conn); conn.dbms_version = probe.dbms_version; conn.server_major = probe.server_major; + if let Some(zone) = probe.time_zone { + conn.server_time_zone = zone; + } conn.user_name = session_user_name(probe.user.as_deref(), p.session_user(), p.user()); Ok(conn) } @@ -2742,13 +2823,21 @@ mod tests { /// when the client's own timeout fires. A refused port would produce a /// connect error instead, and no coordinator is needed to make a request /// take longer than it is allowed to. + /// + /// The port is fixed rather than OS-assigned (`:0`): a sandbox that allows + /// loopback ports one by one, as Landlock does, can list a fixed port, but + /// not the random one this test would then connect to. It sits below + /// Linux's ephemeral range (32768 and up), so an outgoing connection does + /// not take it by chance. Only this helper binds it. fn timeout_failure() -> trino_rust_client::error::Error { - let listener = - std::net::TcpListener::bind("127.0.0.1:0").expect("the loopback interface is bindable"); - let port = listener - .local_addr() - .expect("a bound listener has an address") - .port(); + const PORT: u16 = 29871; + let listener = std::net::TcpListener::bind(("127.0.0.1", PORT)).unwrap_or_else(|e| { + panic!( + "this test listens on 127.0.0.1:{PORT}, a fixed port so a sandbox can allow it; \ + binding it failed (in use, or not allowed by the sandbox): {e}" + ) + }); + let port = PORT; // Holds the accepted connection open, which is what makes the request // time out rather than fail. Detached, and outlived by the test. std::thread::spawn(move || { @@ -3057,6 +3146,45 @@ mod tests { assert_eq!(session_user_name(None, None, None), ""); } + fn zone(name: &str) -> SessionZone { + SessionZone::parse(name).expect("a zone Trino reports") + } + + /// `SET TIME ZONE` changes the session after connect, and Trino reports it + /// as the `time_zone_id` session property, so it wins over both + /// connect-time sources. + #[test] + fn a_set_time_zone_wins_over_the_connection_string_and_the_server() { + assert_eq!( + effective_session_zone(Some("+05:30"), Some(zone("Europe/Berlin")), zone("UTC")), + zone("+05:30") + ); + } + + /// `TimeZone=` is sent as `X-Trino-Time-Zone`, so it is the session's zone + /// until a `SET TIME ZONE`; the server default applies only without it. + #[test] + fn the_connection_string_zone_wins_over_the_server_default() { + assert_eq!( + effective_session_zone(None, Some(zone("Europe/Berlin")), zone("UTC")), + zone("Europe/Berlin") + ); + assert_eq!( + effective_session_zone(None, None, zone("America/New_York")), + zone("America/New_York") + ); + } + + /// A `time_zone_id` this driver cannot read is not a reason to fail a + /// fetch: the next source is used, and the mismatch is logged. + #[test] + fn an_unreadable_time_zone_id_falls_back() { + assert_eq!( + effective_session_zone(Some("Mars/Olympus"), None, zone("UTC")), + zone("UTC") + ); + } + #[test] fn get_type_info_returns_all_trino_types() { let conn = disconnected_trino_conn(); diff --git a/src/backend/execute.rs b/src/backend/execute.rs index 70c6a8e..428155c 100644 --- a/src/backend/execute.rs +++ b/src/backend/execute.rs @@ -20,8 +20,8 @@ use super::{ map_trino_error_on, }; use crate::type_conversion::{ - TrinoTypeName, discards_fractional_seconds, json_to_column_value, trino_ty_precision, - trino_ty_scale, trino_ty_to_sql_type, type_name_precision, type_name_scale, + SessionZone, TrinoTypeName, discards_fractional_seconds, json_to_column_value, + trino_ty_precision, trino_ty_scale, trino_ty_to_sql_type, type_name_precision, type_name_scale, }; /// Convert decoded Trino rows into `Vec>`, alongside the cells @@ -37,6 +37,7 @@ use crate::type_conversion::{ fn convert_rows( rows: Vec, types: &[(String, TrinoTy)], + session_zone: SessionZone, ) -> (Vec>, HashSet<(usize, usize)>) { let mut truncated = HashSet::new(); let batch = rows @@ -51,7 +52,7 @@ fn convert_rows( if discards_fractional_seconds(&val, ty) { truncated.insert((row_idx, col_idx)); } - json_to_column_value(val, ty) + json_to_column_value(val, ty, session_zone) }) .collect() }) @@ -145,6 +146,10 @@ pub(super) fn exec_direct( // interpolating its parameters, so this is the only site that needs it. conn.ensure_transaction()?; + // Read before the submit: a `SET TIME ZONE` takes effect for the + // statements after it, not for itself. + let session_zone = conn.session_zone(); + let submit_start = Instant::now(); let mut page = { let _span = tracing::info_span!("trino.submit").entered(); @@ -272,7 +277,7 @@ pub(super) fn exec_direct( let _span = tracing::info_span!("trino.convert_batch", page = page_count).entered(); let rows = decode_page_rows(&conn.runtime, &conn.client, page.data, &kept_columns) .map_err(|e| conn.statement_error(e))?; - convert_rows(rows, &trino_types) + convert_rows(rows, &trino_types, session_zone) }; let total_convert_time = convert_start.elapsed(); let total_rows_fetched = batch.len() as u64; @@ -288,6 +293,7 @@ pub(super) fn exec_direct( pending_sql: None, columns, trino_types, + session_zone, raw_columns: kept_columns, batch, truncated_cells, @@ -325,6 +331,8 @@ pub(super) fn prepare( pending_sql: Some(sql.to_string()), columns: Vec::new(), trino_types: Vec::new(), + // Nothing is converted until `execute` replaces this statement. + session_zone: SessionZone::UTC, raw_columns: Vec::new(), batch: Vec::new(), truncated_cells: HashSet::new(), @@ -671,7 +679,7 @@ impl StatementBackend for TrinoStatement { decode_page_rows(runtime, client, page.data, &self.raw_columns) }; (self.batch, self.truncated_cells) = match decoded { - Ok(rows) => convert_rows(rows, &self.trino_types), + Ok(rows) => convert_rows(rows, &self.trino_types, self.session_zone), Err(e) => { let mapped = self.map_client_error(e); return Err(self.end_page_fetch(mapped)); @@ -859,6 +867,7 @@ mod tests { pending_sql: None, columns: Vec::new(), trino_types: Vec::new(), + session_zone: SessionZone::UTC, raw_columns: Vec::new(), batch: Vec::new(), truncated_cells: HashSet::new(), diff --git a/src/backend/info.rs b/src/backend/info.rs index d6e823c..430dc49 100644 --- a/src/backend/info.rs +++ b/src/backend/info.rs @@ -85,12 +85,12 @@ const TRINO_TZ_TIMESTAMP_SPACE_LEN: i32 = 1; /// COLUMN_SIZE must never be a literal copied from the per-column path (or /// vice versa). /// -/// Rows are sorted by DATA_TYPE ascending (as signed i16, so ODBC extension -/// types with negative codes sort first), then by TYPE_NAME ascending within -/// an equal DATA_TYPE, per the SQLGetTypeInfo spec's "ordered by DATA_TYPE and -/// then ... TYPE_NAME" requirement. This invariant is asserted directly by -/// `type_info_rows_sorted_by_data_type_then_type_name` below; keep new rows -/// in the correct sorted position rather than appending them. +/// Core orders the result set itself (DATA_TYPE, then the row marked +/// `with_preferred`, then TYPE_NAME; see +/// `stackable_odbc_core::ffi::info::sql_get_type_info`), so rows are grouped +/// here for reading, not for the spec. Every DATA_TYPE shared by several rows +/// marks its closest match, as `every_shared_data_type_has_one_preferred_row` +/// asserts. fn trino_type_info() -> &'static [TypeInfoRow] { static ROWS: OnceLock> = OnceLock::new(); ROWS.get_or_init(|| { @@ -148,6 +148,8 @@ fn trino_type_info() -> &'static [TypeInfoRow] { MaxScale(0), )) .with_literal_affixes(Some("'"), Some("'")), + // The closest match for SQL_WVARCHAR, so core orders it first + // among the types this driver renders as text. TypeInfoRow::new(TrinoTypeName::Varchar.name(), SqlDataType::EXT_W_VARCHAR) .with_column_size(catalog_column_size( SqlDataType::EXT_W_VARCHAR, @@ -156,7 +158,8 @@ fn trino_type_info() -> &'static [TypeInfoRow] { )) .with_literal_affixes(Some("'"), Some("'")) .with_create_params(Some("max length")) - .with_case_sensitive(true), + .with_case_sensitive(true) + .with_preferred(true), TypeInfoRow::new(TrinoTypeName::Char.name(), SqlDataType::EXT_W_CHAR) .with_column_size(catalog_column_size( SqlDataType::EXT_W_CHAR, @@ -289,7 +292,8 @@ fn trino_type_info() -> &'static [TypeInfoRow] { .with_literal_affixes(Some("TIME '"), Some("'")) .with_create_params(Some("precision")) .with_scale_range(Some(0), Some(MAX_FRACTIONAL_SECONDS_PRECISION)) - .with_verbose_type(SqlDataType::DATETIME.0, Some(SQL_CODE_TIME)), + .with_verbose_type(SqlDataType::DATETIME.0, Some(SQL_CODE_TIME)) + .with_preferred(true), // TIME WITH TIME ZONE shares plain TIME's DATA_TYPE (see // TrinoTypeName::sql_type) and still needs its own row: an // application looking SQLGetTypeInfo up by TYPE_NAME, to build a @@ -317,7 +321,8 @@ fn trino_type_info() -> &'static [TypeInfoRow] { .with_literal_affixes(Some("TIMESTAMP '"), Some("'")) .with_create_params(Some("precision")) .with_scale_range(Some(0), Some(MAX_FRACTIONAL_SECONDS_PRECISION)) - .with_verbose_type(SqlDataType::DATETIME.0, Some(SQL_CODE_TIMESTAMP)), + .with_verbose_type(SqlDataType::DATETIME.0, Some(SQL_CODE_TIMESTAMP)) + .with_preferred(true), // TIMESTAMP WITH TIME ZONE shares plain TIMESTAMP's DATA_TYPE, // for the reason TIME WITH TIME ZONE above gives. TypeInfoRow::new( @@ -1147,8 +1152,9 @@ pub(super) fn get_type_info() -> &'static [TypeInfoRow] { /// /// A failed parse falls back to a canonical name chosen here for `sql_type`, /// never to whichever [`trino_type_info`] row sorts first under that -/// `DATA_TYPE`. That row is `INTERVAL DAY TO SECOND`, an accident of the -/// table's required sort order, and it would misname every compound type: +/// `DATA_TYPE`. That is a positional pick, not a deliberate one (before core +/// ranked preferred rows it was `INTERVAL DAY TO SECOND`), and it would +/// misname every compound type: /// ARRAY, MAP, ROW, TUPLE, `ipaddress` and anything else with no dedicated /// `TrinoTypeName` variant. `trino_ty_to_sql_type` renders all of them as /// `EXT_W_VARCHAR` text, so `VARCHAR` is the honest name for that `DATA_TYPE`. @@ -1747,35 +1753,39 @@ mod tests { .unwrap_or_else(|| panic!("no SQLGetTypeInfo row for {type_name:?}")) } + /// Core orders the result set (DATA_TYPE, preferred row, TYPE_NAME), so the + /// declaration order here no longer matters. What does matter is that every + /// DATA_TYPE shared by several rows names its closest match: unmarked, the + /// alphabetically first row leads, and for SQL_WVARCHAR that was + /// `INTERVAL DAY TO SECOND`, which Power Query then used as the CAST + /// target for every varchar column. #[test] - fn type_info_rows_sorted_by_data_type_then_type_name() { - // Spec (SQLGetTypeInfo): "ordered by DATA_TYPE and then ... TYPE_NAME, - // both ascending." DATA_TYPE is a signed i16 (negative for ODBC - // extension types), so the comparison must not treat it as unsigned. - // This walks adjacent pairs rather than asserting a fixed sequence, - // so it keeps holding as rows are added or reordered. - for pair in trino_type_info().windows(2) { - let (prev, next) = (&pair[0], &pair[1]); - assert!( - prev.data_type().0 <= next.data_type().0, - "trino_type_info() not sorted by DATA_TYPE: {:?} (DATA_TYPE={}) \ - appears before {:?} (DATA_TYPE={})", - prev.type_name(), - prev.data_type().0, - next.type_name(), - next.data_type().0 - ); - if prev.data_type() == next.data_type() { - assert!( - prev.type_name() <= next.type_name(), - "rows sharing DATA_TYPE={} not sorted by TYPE_NAME: {:?} appears \ - before {:?}", - prev.data_type().0, - prev.type_name(), - next.type_name() - ); - } - } + fn every_shared_data_type_has_one_preferred_row() { + let issues = + stackable_odbc_core::conformance::type_info_preference_issues(trino_type_info()); + assert!( + issues.is_empty(), + "SQLGetTypeInfo preference markers: {issues:#?}" + ); + } + + /// The specific choices, not just their count: the plain type is the + /// closest match for each ODBC type it shares with Trino variants. + #[test] + fn preferred_rows_are_the_plain_types() { + let preferred: Vec<(i16, &str)> = trino_type_info() + .iter() + .filter(|r| r.preferred()) + .map(|r| (r.data_type().0, r.type_name())) + .collect(); + assert_eq!( + preferred, + vec![ + (SqlDataType::EXT_W_VARCHAR.0, TrinoTypeName::Varchar.name()), + (SqlDataType::TIME.0, TrinoTypeName::Time.name()), + (SqlDataType::TIMESTAMP.0, TrinoTypeName::Timestamp.name()), + ] + ); } /// `driver_version!()` must resolve `SQL_DRIVER_VER` from *this* crate's diff --git a/src/ffi_integration_tests.rs b/src/ffi_integration_tests.rs index 2b13484..e170ea7 100644 --- a/src/ffi_integration_tests.rs +++ b/src/ffi_integration_tests.rs @@ -33,8 +33,8 @@ use stackable_odbc_core::odbc_sys; use stackable_odbc_core::test_support::{attach_connection, detach_connection}; use stackable_odbc_core::types::{ AttrOdbcVersion, CDataType, Desc, EnvironmentAttribute, HandleType, HeaderDiagnosticIdentifier, - InfoType, ParamType, SQL_AUTOCOMMIT_OFF, SQL_AUTOCOMMIT_ON, SQL_FETCH_BOOKMARK, SQL_IC_LOWER, - SQL_INDEX_UNIQUE, SQL_LOCK_NO_CHANGE, SQL_NTS, SQL_NULL_DATA, SQL_PARAM_ERROR, + InfoType, Interval, ParamType, SQL_AUTOCOMMIT_OFF, SQL_AUTOCOMMIT_ON, SQL_FETCH_BOOKMARK, + SQL_IC_LOWER, SQL_INDEX_UNIQUE, SQL_LOCK_NO_CHANGE, SQL_NTS, SQL_NULL_DATA, SQL_PARAM_ERROR, SQL_PARAM_SUCCESS, SQL_POSITION, SQL_QUICK, SqlDataType, SqlReturn, StatementAttribute, expected_kind, }; @@ -197,6 +197,12 @@ unsafe fn alloc_handles() -> (*mut c_void, *mut c_void, *mut c_void) { /// Helper: connect to Trino at localhost:8443. unsafe fn connect_trino(conn: *mut c_void) -> SqlReturn { + unsafe { connect_trino_with(conn, "") } +} + +/// [`connect_trino`] with `extra` appended to the connection string, e.g. +/// `";TimeZone=Europe/Berlin"`. +unsafe fn connect_trino_with(conn: *mut c_void, extra: &str) -> SqlReturn { // The terminator is part of the buffer, because the length argument below // is SQL_NTS: that tells the driver the string is null-terminated and to // find the end itself. Without it the driver reads past this Vec until it @@ -207,7 +213,10 @@ unsafe fn connect_trino(conn: *mut c_void) -> SqlReturn { // // Every other wide buffer in this file passes an explicit length instead, // and needs no terminator. - let wide: Vec = CONN_STR.encode_utf16().chain(std::iter::once(0)).collect(); + let wide: Vec = format!("{CONN_STR}{extra}") + .encode_utf16() + .chain(std::iter::once(0)) + .collect(); unsafe { ffi::connect::sql_driver_connect_w::( conn, @@ -611,6 +620,59 @@ fn get_info_groups_that_constrain_each_other_agree() { } } +/// What Power Query sees: `SQLGetTypeInfo(DATA_TYPE)` through core's real +/// entry point, first row's TYPE_NAME. A shared DATA_TYPE must lead with the +/// plain type, never a variant such as `INTERVAL DAY TO SECOND` that merely +/// sorts first. Needs no server: `get_type_info` reads a static table. +#[test] +#[serial] +fn get_type_info_leads_each_shared_data_type_with_the_plain_type() { + unsafe { + let (env, conn) = alloc_conn_with_injected_trino_connection(); + for (data_type, expected) in [ + (SqlDataType::EXT_W_VARCHAR, "VARCHAR"), + (SqlDataType::TIME, "TIME"), + (SqlDataType::TIMESTAMP, "TIMESTAMP"), + ] { + let mut stmt: *mut c_void = std::ptr::null_mut(); + assert_eq!( + ffi::handle::sql_alloc_handle::( + HandleType::Stmt as i16, + conn, + &mut stmt + ), + SqlReturn::SUCCESS + ); + assert_eq!( + ffi::info::sql_get_type_info::(stmt, data_type.0), + SqlReturn::SUCCESS, + "SQLGetTypeInfo({data_type:?})" + ); + assert_eq!( + ffi::fetch::sql_fetch::(stmt), + SqlReturn::SUCCESS + ); + let mut buf = [0u16; 64]; + let mut ind: isize = 0; + assert_eq!( + ffi::fetch::sql_get_data::( + stmt, + 1, + CDataType::WChar as i16, + buf.as_mut_ptr().cast(), + std::mem::size_of_val(&buf) as isize, + &mut ind, + ), + SqlReturn::SUCCESS + ); + let name = String::from_utf16(&buf[..(ind as usize / 2)]).expect("UTF-16 TYPE_NAME"); + assert_eq!(name, expected, "first SQLGetTypeInfo row for {data_type:?}"); + let _ = ffi::handle::sql_free_handle::(HandleType::Stmt as i16, stmt); + } + cleanup_injected_conn(env, conn); + } +} + /// Property 2: no genuine `SQL_CONVERT_*` code ever returns 0 through /// `TrinoBackend`: per `AGENTS.md`, a `0` conversion bitmap is what makes /// the Windows Driver Manager block `SQLGetData` with `HYC00`. @@ -1605,9 +1667,10 @@ fn interval_year_month_returns_wchar() { ffi::fetch::sql_fetch::(stmt), SqlReturn::SUCCESS ); - let s = fetch_wchar(stmt); - assert!(s.contains('3'), "expected '3' in {s:?}"); - assert!(s.contains('7'), "expected '7' in {s:?}"); + // Trino's own rendering, unchanged: Power BI folds a slicer on this + // column to `cast(col as VARCHAR) = ''`, which only matches if + // the shown text is what that CAST produces. + assert_eq!(fetch_wchar(stmt), "3-7"); cleanup_stmt(stmt); } } @@ -1626,8 +1689,113 @@ fn interval_day_time_returns_wchar() { ffi::fetch::sql_fetch::(stmt), SqlReturn::SUCCESS ); - let s = fetch_wchar(stmt); - assert!(s.contains('2'), "expected '2' in {s:?}"); + assert_eq!(fetch_wchar(stmt), "2 03:04:05.678"); + cleanup_stmt(stmt); + } +} + +/// The values the Power BI slicer view (`seed-hive.sh`) holds, whose re-rendered +/// text (`-1-00`, `0 00:00:00.5`) differed from Trino's and so selected no rows. +#[test] +#[serial] +#[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] +fn intervals_are_delivered_as_trinos_cast_to_varchar_text() { + for expr in [ + "INTERVAL '-1' YEAR", + "INTERVAL '3' MONTH", + "INTERVAL '-1' DAY", + "INTERVAL '0.5' SECOND", + "INTERVAL '1' DAY + INTERVAL '30' MINUTE", + ] { + unsafe { + let (_env, _conn, stmt) = alloc_stmt(); + assert_eq!( + exec_direct(stmt, &format!("SELECT {expr}, CAST({expr} AS VARCHAR)")), + SqlReturn::SUCCESS + ); + assert_eq!( + ffi::fetch::sql_fetch::(stmt), + SqlReturn::SUCCESS + ); + let cast = get_wchar_col(stmt, 2); + assert_eq!(get_wchar_col(stmt, 1), cast, "{expr}"); + cleanup_stmt(stmt); + } + } +} + +/// Read column 1 into a `SQL_INTERVAL_STRUCT` as `target`. +unsafe fn fetch_interval(stmt: *mut c_void, target: CDataType) -> odbc_sys::IntervalStruct { + let mut out = odbc_sys::IntervalStruct { + interval_type: 0, + interval_sign: 0, + interval_value: odbc_sys::IntervalUnion { + day_second: odbc_sys::DaySecond::default(), + }, + }; + let mut ind: isize = 0; + let ret = unsafe { + ffi::fetch::sql_get_data::( + stmt, + 1, + target as i16, + (&raw mut out).cast(), + size_of::() as isize, + &mut ind, + ) + }; + assert_eq!(ret, SqlReturn::SUCCESS, "sql_get_data as {target:?} failed"); + out +} + +/// Delivering interval columns as text must not cost an application the +/// `SQL_C_INTERVAL_*` C types: core converts the text, per the spec's +/// "SQL to C: Character" table. +#[test] +#[serial] +#[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] +fn interval_year_month_reads_as_sql_c_interval_year_to_month() { + unsafe { + let (_env, _conn, stmt) = alloc_stmt(); + assert_eq!( + exec_direct(stmt, "SELECT INTERVAL '-3-7' YEAR TO MONTH"), + SqlReturn::SUCCESS + ); + assert_eq!( + ffi::fetch::sql_fetch::(stmt), + SqlReturn::SUCCESS + ); + let got = fetch_interval(stmt, CDataType::IntervalYearToMonth); + assert_eq!(got.interval_type, Interval::YearToMonth as i32); + assert_eq!(got.interval_sign, 1, "SQL_TRUE: negative"); + let ym = got.interval_value.year_month; + assert_eq!((ym.year, ym.month), (3, 7)); + cleanup_stmt(stmt); + } +} + +#[test] +#[serial] +#[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] +fn interval_day_time_reads_as_sql_c_interval_day_to_second() { + unsafe { + let (_env, _conn, stmt) = alloc_stmt(); + assert_eq!( + exec_direct(stmt, "SELECT INTERVAL '2 03:04:05' DAY TO SECOND"), + SqlReturn::SUCCESS + ); + assert_eq!( + ffi::fetch::sql_fetch::(stmt), + SqlReturn::SUCCESS + ); + let got = fetch_interval(stmt, CDataType::IntervalDayToSecond); + assert_eq!(got.interval_type, Interval::DayToSecond as i32); + assert_eq!(got.interval_sign, 0); + let ds = got.interval_value.day_second; + assert_eq!( + (ds.day, ds.hour, ds.minute, ds.second, ds.fraction), + (2, 3, 4, 5, 0) + ); cleanup_stmt(stmt); } } @@ -1655,7 +1823,10 @@ fn timestamp_with_tz_returns_wchar() { #[test] #[serial] #[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] -fn timestamp_with_named_tz_returns_utc_via_get_data() { +fn timestamp_with_named_tz_returns_utc_in_a_utc_session() { + // UTC because the shared connection sets no `TimeZone=` and the test + // stack's coordinator defaults to UTC: values are delivered in the session + // zone (see `timestamptz_is_delivered_in_the_session_time_zone`). unsafe { let (_env, _conn, stmt) = alloc_stmt(); // America/New_York in March 2025 is EDT (UTC-4). @@ -1701,6 +1872,230 @@ fn timestamp_with_named_tz_returns_utc_via_get_data() { } } +/// Column `col` read as a `SQL_TIMESTAMP_STRUCT`. +unsafe fn get_timestamp_col(stmt: *mut c_void, col: u16) -> odbc_sys::Timestamp { + let mut ts = odbc_sys::Timestamp::default(); + let mut ind: isize = 0; + let ret = unsafe { + ffi::fetch::sql_get_data::( + stmt, + col, + CDataType::TypeTimestamp as i16, + (&raw mut ts).cast(), + std::mem::size_of::() as isize, + &mut ind, + ) + }; + assert_eq!( + ret, + SqlReturn::SUCCESS, + "sql_get_data(TypeTimestamp) on column {col}" + ); + ts +} + +/// Run `sql` to completion in its own statement on `conn`. +unsafe fn run_on(conn: *mut c_void, sql: &str) { + unsafe { + let mut stmt: *mut c_void = std::ptr::null_mut(); + assert_eq!( + ffi::handle::sql_alloc_handle::(HandleType::Stmt as i16, conn, &mut stmt), + SqlReturn::SUCCESS + ); + assert_eq!( + exec_direct(stmt, sql), + SqlReturn::SUCCESS, + "{sql}: {}", + diag_message(stmt) + ); + cleanup_stmt(stmt); + } +} + +/// Assert that a timestamp-with-time-zone value arrives as the wall time Trino +/// itself gives it in the session zone, and return that zone's name. +/// +/// The oracle is Trino's own `at_timezone(x, current_timezone())` cast to a +/// plain `TIMESTAMP`: the wall time a Power BI literal folded back from the +/// delivered value is compared against. +unsafe fn assert_delivered_in_session_zone(conn: *mut c_void, label: &str) -> String { + unsafe { + let mut stmt: *mut c_void = std::ptr::null_mut(); + assert_eq!( + ffi::handle::sql_alloc_handle::(HandleType::Stmt as i16, conn, &mut stmt), + SqlReturn::SUCCESS + ); + assert_eq!( + exec_direct( + stmt, + "SELECT x, CAST(at_timezone(x, current_timezone()) AS TIMESTAMP(3)), \ + current_timezone() \ + FROM (VALUES TIMESTAMP '2025-03-10 20:21:22.123 America/New_York') t(x)" + ), + SqlReturn::SUCCESS, + "{label}: {}", + diag_message(stmt) + ); + assert_eq!( + ffi::fetch::sql_fetch::(stmt), + SqlReturn::SUCCESS + ); + let delivered = get_timestamp_col(stmt, 1); + let trinos_wall_time = get_timestamp_col(stmt, 2); + let zone = get_wchar_col(stmt, 3); + assert_eq!(delivered, trinos_wall_time, "{label}: session zone {zone}"); + cleanup_stmt(stmt); + zone + } +} + +/// The session zone comes from `TimeZone=` when it is given, and from the +/// coordinator otherwise. +#[test] +#[serial] +#[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] +fn timestamptz_is_delivered_in_the_session_time_zone() { + for (extra, want) in [ + ("", None), + (";TimeZone=Europe/Berlin", Some("Europe/Berlin")), + (";TimeZone=America/New_York", Some("America/New_York")), + ] { + unsafe { + let (env, conn, stmt) = alloc_handles(); + assert_eq!( + connect_trino_with(conn, extra), + SqlReturn::SUCCESS, + "{extra}: {}", + conn_diag_message(conn) + ); + let zone = assert_delivered_in_session_zone(conn, extra); + if let Some(want) = want { + assert_eq!(zone, want, "{extra}"); + } + cleanup(env, conn, stmt); + } + } +} + +/// `SET TIME ZONE` changes the zone of the statements after it, and +/// `SET TIME ZONE LOCAL` changes it back, without reconnecting. +#[test] +#[serial] +#[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] +fn set_time_zone_moves_later_statements_into_the_new_zone() { + unsafe { + let (env, conn, stmt) = alloc_handles(); + assert_eq!( + connect_trino_with(conn, ";TimeZone=Europe/Berlin"), + SqlReturn::SUCCESS, + "{}", + conn_diag_message(conn) + ); + assert_eq!( + assert_delivered_in_session_zone(conn, "connected"), + "Europe/Berlin" + ); + + run_on(conn, "SET TIME ZONE '+05:30'"); + assert_eq!( + assert_delivered_in_session_zone(conn, "after +05:30"), + "+05:30" + ); + + run_on(conn, "SET TIME ZONE 'America/New_York'"); + assert_eq!( + assert_delivered_in_session_zone(conn, "after America/New_York"), + "America/New_York" + ); + + run_on(conn, "SET TIME ZONE LOCAL"); + assert_delivered_in_session_zone(conn, "after LOCAL"); + cleanup(env, conn, stmt); + } +} + +/// Assert that a `TIME WITH TIME ZONE` arrives as the time of day Trino itself +/// gives it in the session: cast back to `TIME WITH TIME ZONE` in that +/// session, the delivered time is the same instant. +unsafe fn assert_time_tz_delivered_in_session(conn: *mut c_void, label: &str) { + let value = "TIME '13:14:15+05:00'"; + unsafe { + let mut stmt: *mut c_void = std::ptr::null_mut(); + assert_eq!( + ffi::handle::sql_alloc_handle::(HandleType::Stmt as i16, conn, &mut stmt), + SqlReturn::SUCCESS + ); + assert_eq!( + exec_direct(stmt, &format!("SELECT {value}")), + SqlReturn::SUCCESS + ); + assert_eq!( + ffi::fetch::sql_fetch::(stmt), + SqlReturn::SUCCESS + ); + let mut t = odbc_sys::Time::default(); + let mut ind: isize = 0; + assert_eq!( + ffi::fetch::sql_get_data::( + stmt, + 1, + CDataType::TypeTime as i16, + (&raw mut t).cast(), + std::mem::size_of::() as isize, + &mut ind, + ), + SqlReturn::SUCCESS + ); + cleanup_stmt(stmt); + + let shown = format!("{:02}:{:02}:{:02}", t.hour, t.minute, t.second); + let mut stmt: *mut c_void = std::ptr::null_mut(); + assert_eq!( + ffi::handle::sql_alloc_handle::(HandleType::Stmt as i16, conn, &mut stmt), + SqlReturn::SUCCESS + ); + let sql = format!( + "SELECT CAST({value} = CAST(TIME '{shown}' AS TIME WITH TIME ZONE) AS VARCHAR), \ + current_timezone()" + ); + assert_eq!(exec_direct(stmt, &sql), SqlReturn::SUCCESS, "{sql}"); + assert_eq!( + ffi::fetch::sql_fetch::(stmt), + SqlReturn::SUCCESS + ); + let same = get_wchar_col(stmt, 1); + let zone = get_wchar_col(stmt, 2); + assert_eq!( + same, "true", + "{label}: delivered {shown} in session zone {zone}" + ); + cleanup_stmt(stmt); + } +} + +/// `TIME WITH TIME ZONE` follows the session zone as `TIMESTAMP WITH TIME +/// ZONE` does, including after `SET TIME ZONE`. +#[test] +#[serial] +#[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] +fn time_with_tz_is_delivered_in_the_session_time_zone() { + for extra in ["", ";TimeZone=Europe/Berlin"] { + unsafe { + let (env, conn, stmt) = alloc_handles(); + assert_eq!( + connect_trino_with(conn, extra), + SqlReturn::SUCCESS, + "{extra}: {}", + conn_diag_message(conn) + ); + assert_time_tz_delivered_in_session(conn, extra); + run_on(conn, "SET TIME ZONE '+05:30'"); + assert_time_tz_delivered_in_session(conn, "after +05:30"); + cleanup(env, conn, stmt); + } + } +} + #[test] #[serial] #[ignore = "requires Trino at localhost:8443; run ./integration-tests/setup.sh first"] diff --git a/src/fuzz_api.rs b/src/fuzz_api.rs index 3b31357..ce142cc 100644 --- a/src/fuzz_api.rs +++ b/src/fuzz_api.rs @@ -33,15 +33,18 @@ use stackable_odbc_core::types::ColumnValue; use trino_rust_client::TrinoTy; pub use crate::type_conversion::{ - json_to_column_value, trino_type_name_to_sql_type, type_name_precision, type_name_scale, + SessionZone, json_to_column_value, trino_type_name_to_sql_type, type_name_precision, + type_name_scale, }; /// Convert one coordinator JSON value under its declared Trino type. /// /// A thin alias for [`json_to_column_value`], kept so a target can name the -/// whole read-path conversion without importing the type-name helpers. +/// whole read-path conversion without importing the type-name helpers. The +/// session zone is one with daylight saving time, so fuzzed timestamps also +/// reach the overlap and gap hours. pub fn json_value(val: Value, ty: &TrinoTy) -> ColumnValue { - json_to_column_value(val, ty) + json_to_column_value(val, ty, SessionZone::Named(chrono_tz::Tz::Europe__Berlin)) } /// Run core's ODBC escape translator with this driver's dialect. diff --git a/src/type_conversion.rs b/src/type_conversion.rs index 758491f..74f1775 100644 --- a/src/type_conversion.rs +++ b/src/type_conversion.rs @@ -5,10 +5,7 @@ use chrono::Datelike as _; use chrono::Timelike as _; use serde_json::Value; -use stackable_odbc_core::types::{ - ColumnValue, Interval, NANOS_PER_DAY, NANOS_PER_HOUR, NANOS_PER_MINUTE, NANOS_PER_SECOND, - PRECISION_UNDETERMINABLE, SqlDataType, column_size, -}; +use stackable_odbc_core::types::{ColumnValue, PRECISION_UNDETERMINABLE, SqlDataType, column_size}; use trino_rust_client::{TrinoFloat, TrinoInt, TrinoTy}; /// This driver's declared maximum fractional-seconds precision for TIME, @@ -186,8 +183,8 @@ impl TrinoTypeName { // DEFAULT_TEMPORAL_SCALE_WITHOUT_TYPE_NAME, Trino's actual // default declared precision). TimeWithTimeZone is also 12, not // HH:MM:SS.mmm+HH:MM (18): `parse_trino_time_with_tz` applies the - // offset and normalises to UTC (matching TIMESTAMP WITH TIME - // ZONE), so the offset never survives into SQL_TIME_STRUCT; + // offset and converts to the session time zone (matching TIMESTAMP + // WITH TIME ZONE), so the offset never survives into SQL_TIME_STRUCT; // only the fractional seconds do (preserved via `ColumnValue:: // Time`'s `fraction` field, delivered through SQL_C_CHAR/WCHAR // text conversions). Keep the two as distinct arms, matching @@ -207,8 +204,8 @@ impl TrinoTypeName { // YYYY-MM-DD HH:MM:SS.mmm = 23 chars (20 + scale 3, see // DEFAULT_TEMPORAL_SCALE_WITHOUT_TYPE_NAME). TimestampWithTimeZone // is also 23, not YYYY-MM-DD HH:MM:SS.mmm+HH:MM (29): - // `parse_trino_timestamp_tz` applies the offset and normalises to - // UTC, so it doesn't survive into SQL_TIMESTAMP_STRUCT (which has + // `parse_trino_timestamp_tz` applies the offset and converts to + // the session time zone, so it doesn't survive into SQL_TIMESTAMP_STRUCT (which has // no zone field either); only YYYY-MM-DD HH:MM:SS.mmm is // delivered. Kept as a distinct match arm (not merged with // `Timestamp`) to match `trino_ty_precision`, for the same reason @@ -431,7 +428,8 @@ pub fn trino_ty_precision(ty: &TrinoTy) -> u32 { DEFAULT_TEMPORAL_SCALE_WITHOUT_TYPE_NAME, )), // Also HH:MM:SS.mmm, not HH:MM:SS.mmm+HH:MM: the offset is applied - // and the value normalised to UTC (see `parse_trino_time_with_tz`), + // and the value converted to the session zone (see + // `parse_trino_time_with_tz`), // so only the fractional seconds reach the application, through the // text conversions. SQL_TIME_STRUCT has no fraction field of its own. TrinoTy::TimeWithTimeZone => precision_as_u32(column_size( @@ -447,7 +445,8 @@ pub fn trino_ty_precision(ty: &TrinoTy) -> u32 { DEFAULT_TEMPORAL_SCALE_WITHOUT_TYPE_NAME, )), // Also YYYY-MM-DD HH:MM:SS.mmm, not ...+HH:MM: the offset is applied - // and the value normalised to UTC (see `parse_trino_timestamp_tz`), + // and the value converted to the session zone (see + // `parse_trino_timestamp_tz`), // so only YYYY-MM-DD HH:MM:SS.mmm reaches SQL_TIMESTAMP_STRUCT. TrinoTy::TimestampWithTimeZone => precision_as_u32(column_size( SqlDataType::TIMESTAMP, @@ -784,12 +783,14 @@ fn parse_trino_time(s: &str) -> Option { }) } -/// Parse `TIME WITH TIME ZONE` and normalise to UTC. +/// Parse `TIME WITH TIME ZONE` and deliver it as the time of day at +/// `session_offset_minutes`, the session zone's offset (see +/// [`SessionZone::offset_minutes_at`]). /// /// `SQL_TIME_STRUCT` has no timezone field, so the offset cannot be carried. -/// Applying it and returning UTC matches what `TIMESTAMP WITH TIME ZONE` -/// already does; do not discard the offset silently, or the two "with time -/// zone" types behave inconsistently. +/// The value's own offset fixes the instant and the session's decides how it +/// is shown, as for `TIMESTAMP WITH TIME ZONE`; do not discard the offset +/// silently, or the two "with time zone" types behave inconsistently. /// /// Trino renders the zone two ways: a space-separated name (`"13:14:15.000 /// UTC"`) or a glued numeric offset (`"13:14:15+02:00"`, `"13:14:15-05:30"`). @@ -799,7 +800,7 @@ fn parse_trino_time(s: &str) -> Option { /// only affects the rare case of a non-UTC named zone, since Trino's own /// numeric-offset rendering is the common form. /// Returns `None` if the string is malformed. -fn parse_trino_time_with_tz(s: &str) -> Option { +fn parse_trino_time_with_tz(s: &str, session_offset_minutes: i32) -> Option { let t = s.trim(); // A space-separated named zone, e.g. "13:14:15.000 UTC". @@ -816,7 +817,7 @@ fn parse_trino_time_with_tz(s: &str) -> Option { ); 0 }; - return shift_time(time_part, offset_minutes); + return shift_time(time_part, offset_minutes - session_offset_minutes); } // A glued numeric offset, e.g. "13:14:15+02:00" or "13:14:15-05:30". @@ -835,10 +836,13 @@ fn parse_trino_time_with_tz(s: &str) -> Option { // `None` keeps the value as text, which is what every other unparseable // temporal string here already does. let offset_minutes = oh.checked_mul(60)?.checked_add(om)?.checked_mul(sign)?; - shift_time(time_part, offset_minutes) + shift_time( + time_part, + offset_minutes.checked_sub(session_offset_minutes)?, + ) } -/// Shift `HH:MM:SS[.f]` by `offset_minutes`, wrapping within the day. +/// Shift `HH:MM:SS[.f]` back by `offset_minutes`, wrapping within the day. /// /// A `TIME` has no date to carry an overflow into, so the result wraps /// within `[0, 24h)`. `rem_euclid` (not `%`) is used because a negative @@ -925,98 +929,100 @@ fn parse_trino_timestamp(s: &str) -> Option { }) } -/// Parse a Trino INTERVAL YEAR TO MONTH string "Y-M" into years and months. +/// The session time zone, in which timestamp-with-time-zone values are +/// delivered. /// -/// The sign prefixes the whole interval, not just the year component: Trino -/// serialises a negative interval as `"-Y-M"`. Parsing the leading `-` once -/// (rather than relying on `i32::parse` to see it on the first token) and -/// applying it to both fields keeps the two in agreement: a split -/// representation must not let one field be negative while the other is -/// positive. -fn parse_interval_year_month(s: &str) -> Option { - let t = s.trim(); - let (negative, body) = match t.strip_prefix('-') { - Some(rest) => (true, rest), - None => (false, t), - }; +/// Trino reports it from `current_timezone()` either as an IANA name (`UTC`, +/// `Europe/Berlin`) or, after `SET TIME ZONE '+05:30'`, as a fixed offset, so +/// both forms are kept. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum SessionZone { + Named(chrono_tz::Tz), + Fixed(chrono::FixedOffset), +} - let mut parts = body.splitn(2, '-'); - let years: i32 = parts.next()?.trim().parse().ok()?; - let months: i32 = parts.next()?.trim().parse().ok()?; +impl SessionZone { + pub const UTC: SessionZone = SessionZone::Named(chrono_tz::Tz::UTC); - let (years, months) = if negative { - (years.checked_neg()?, months.checked_neg()?) - } else { - (years, months) - }; + /// Read a zone the way Trino names one: an IANA name, or `±hh:mm`. + pub fn parse(s: &str) -> Option { + let s = s.trim(); + let sign = match s.as_bytes().first()? { + b'+' => 1, + b'-' => -1, + _ => return s.parse().ok().map(SessionZone::Named), + }; + let (h, m) = s[1..].split_once(':')?; + if h.len() != 2 || m.len() != 2 { + return None; + } + let (h, m): (i32, i32) = (h.parse().ok()?, m.parse().ok()?); + if h > 18 || m > 59 { + return None; + } + chrono::FixedOffset::east_opt(sign * (h * 3600 + m * 60)).map(SessionZone::Fixed) + } - Some(ColumnValue::IntervalYearMonth { - years, - months, - // Trino has one year-month interval type and it carries both fields, so - // the precision is always the two-field form. The narrower - // `Interval::Year` and `Interval::Month` have no Trino column type to - // come from. - precision: Interval::YearToMonth, - }) + /// This zone's offset from UTC, in minutes, at the instant `utc`. + /// + /// A `TIME WITH TIME ZONE` has no date, so a named zone is resolved at the + /// current instant, as Trino resolves one when it casts `TIME` to + /// `TIME WITH TIME ZONE` in the session zone. + pub fn offset_minutes_at(self, utc: chrono::NaiveDateTime) -> i32 { + use chrono::Offset as _; + use chrono::TimeZone as _; + let seconds = match self { + SessionZone::Named(tz) => tz.offset_from_utc_datetime(&utc).fix().local_minus_utc(), + SessionZone::Fixed(offset) => offset.local_minus_utc(), + }; + seconds / 60 + } + + /// The wall time in this zone at the instant `utc`. + /// + /// Unambiguous: every instant has exactly one wall time, including the two + /// instants of an autumn overlap hour, which share one. + fn wall_time(self, utc: chrono::NaiveDateTime) -> chrono::NaiveDateTime { + use chrono::TimeZone as _; + match self { + SessionZone::Named(tz) => tz.from_utc_datetime(&utc).naive_local(), + SessionZone::Fixed(offset) => offset.from_utc_datetime(&utc).naive_local(), + } + } } -/// Parse Trino's `INTERVAL DAY TO SECOND` text, e.g. `"-2 03:04:05.678"`. +/// The current instant, in UTC. /// -/// The sign prefixes the whole interval, not just the day component. -fn parse_interval_day_time(s: &str) -> Option { - let t = s.trim(); - let (negative, body) = match t.strip_prefix('-') { - Some(rest) => (true, rest), - None => (false, t), - }; - - let mut outer = body.splitn(2, ' '); - let days: i64 = outer.next()?.trim().parse().ok()?; - let time_part = outer.next()?.trim(); - - let mut tp = time_part.splitn(3, ':'); - let h: i64 = tp.next()?.parse().ok()?; - let m: i64 = tp.next()?.parse().ok()?; - let sec_part = tp.next()?; - let (sec_text, frac_text) = match sec_part.split_once('.') { - Some((sec, frac)) => (sec, frac), - None => (sec_part, ""), - }; - let sec: i64 = sec_text.parse().ok()?; - // `ColumnValue::IntervalDayTime` counts nanoseconds, so the fraction is kept - // whole. Trino renders this type with three fractional digits, its own - // storage being a millisecond count, but `parse_fraction_nanos` reads - // whatever arrives on the same "pad right, then take nine" rule the temporal - // parsers use, so a shorter fragment like "5" is read as 500ms rather than - // 5ns and a longer one does not have to be special-cased here. - let frac_nanos = i128::from(parse_fraction_nanos(frac_text)); - - let magnitude = i128::from(days) - .checked_mul(NANOS_PER_DAY)? - .checked_add(i128::from(h) * NANOS_PER_HOUR)? - .checked_add(i128::from(m) * NANOS_PER_MINUTE)? - .checked_add(i128::from(sec) * NANOS_PER_SECOND)? - .checked_add(frac_nanos)?; - - Some(ColumnValue::IntervalDayTime { - total_nanoseconds: if negative { -magnitude } else { magnitude }, - // Trino has one day-time interval type and it spans all four fields, so - // the precision is always the widest form. - precision: Interval::DayToSecond, - }) +/// From `SystemTime` because this crate builds chrono without its `clock` +/// feature. A clock before 1970 reads as the epoch, which only affects which +/// side of a DST change a named zone's offset is taken from. +fn utc_now() -> chrono::NaiveDateTime { + let since_epoch = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default(); + chrono::DateTime::from_timestamp( + i64::try_from(since_epoch.as_secs()).unwrap_or(0), + since_epoch.subsec_nanos(), + ) + .unwrap_or_default() + .naive_utc() } -/// Parse a Trino TIMESTAMP WITH TIME ZONE string and convert to UTC. +/// Parse a Trino TIMESTAMP WITH TIME ZONE string and deliver it as wall time +/// in the session time zone. /// /// Trino REST API format: `"YYYY-MM-DD HH:MM:SS.fraction TIMEZONE"` where /// TIMEZONE is either a numeric offset (`+05:30`, `-08:00`) or a named IANA -/// zone (`UTC`, `America/New_York`, `Europe/Berlin`). +/// zone (`UTC`, `America/New_York`, `Europe/Berlin`). That zone fixes the +/// instant; `session` decides how it is shown. /// -/// Returns `ColumnValue::Timestamp` with UTC-converted fields, matching the -/// official Trino ODBC driver's behaviour. The timezone information is consumed -/// during conversion: `SQL_TIMESTAMP_STRUCT` has no timezone field. -fn parse_trino_timestamp_tz(s: &str) -> Option { +/// `SQL_TIMESTAMP_STRUCT` has no time zone field, so one zone has to be chosen, +/// and the session's is the one Trino itself uses for a plain `TIMESTAMP`. A +/// Power BI slicer folds the value it showed back as exactly such a literal +/// (`"col" = CAST('2025-06-15 14:30:00' as TIMESTAMP)`), and Trino compares it +/// with the column after reading it in the session zone. Delivering UTC made +/// that filter match only in a UTC session. +fn parse_trino_timestamp_tz(s: &str, session: SessionZone) -> Option { let last_space = s.rfind(' ')?; let datetime_part = &s[..last_space]; let tz_part = s[last_space + 1..].trim(); @@ -1075,15 +1081,17 @@ fn parse_trino_timestamp_tz(s: &str) -> Option { .naive_utc() }; - // Fraction (sub-second nanoseconds) is preserved as-is: UTC conversion - // only shifts hours/minutes/seconds, never sub-second precision. + let local = session.wall_time(utc_ndt); + + // Fraction (sub-second nanoseconds) is preserved as-is: a zone change only + // shifts hours/minutes/seconds, never sub-second precision. Some(ColumnValue::Timestamp { - year: i16::try_from(utc_ndt.date().year()).ok()?, - month: utc_ndt.date().month() as u16, - day: utc_ndt.date().day() as u16, - hour: utc_ndt.time().hour() as u16, - minute: utc_ndt.time().minute() as u16, - second: utc_ndt.time().second() as u16, + year: i16::try_from(local.date().year()).ok()?, + month: local.date().month() as u16, + day: local.date().day() as u16, + hour: local.time().hour() as u16, + minute: local.time().minute() as u16, + second: local.time().second() as u16, fraction, }) } @@ -1132,7 +1140,10 @@ fn json_as_text(val: &Value) -> String { } /// Convert a JSON value from Trino to an ODBC ColumnValue, guided by the column type. -pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { +/// +/// `session` is the session time zone, in which a timestamp-with-time-zone value +/// is delivered (see [`parse_trino_timestamp_tz`]). +pub fn json_to_column_value(val: Value, ty: &TrinoTy, session: SessionZone) -> ColumnValue { if val.is_null() { return ColumnValue::Null; } @@ -1222,16 +1233,18 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { ColumnValue::String(val.to_string()) } } - // TIME WITH TIME ZONE: parse and normalise to UTC, mirroring - // TIMESTAMP WITH TIME ZONE. See `parse_trino_time_with_tz` for why + // TIME WITH TIME ZONE: parse and convert to the session zone's + // current offset, mirroring TIMESTAMP WITH TIME ZONE. See `parse_trino_time_with_tz` for why // this must not share `parse_trino_time`, which silently discards // the offset instead of applying it. TrinoTy::TimeWithTimeZone => { if let Value::String(ref s) = val { - parse_trino_time_with_tz(s).unwrap_or_else(|| { - tracing::warn!(raw = s, "failed to parse Trino TIME WITH TIME ZONE string"); - ColumnValue::String(s.clone()) - }) + parse_trino_time_with_tz(s, session.offset_minutes_at(utc_now())).unwrap_or_else( + || { + tracing::warn!(raw = s, "failed to parse Trino TIME WITH TIME ZONE string"); + ColumnValue::String(s.clone()) + }, + ) } else { ColumnValue::String(val.to_string()) } @@ -1247,13 +1260,14 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { ColumnValue::String(val.to_string()) } } - // TIMESTAMP WITH TIME ZONE: parse and convert to UTC via chrono-tz. - // Trino sends named zones (UTC, America/New_York, CET) or numeric - // offsets (+05:30, -08:00); the column type in the REST API metadata - // determines which parser is called, not the string content. + // TIMESTAMP WITH TIME ZONE: parse, then deliver in the session time + // zone via chrono-tz. Trino sends named zones (UTC, America/New_York, + // CET) or numeric offsets (+05:30, -08:00); the column type in the REST + // API metadata determines which parser is called, not the string + // content. TrinoTy::TimestampWithTimeZone => { if let Value::String(ref s) = val { - parse_trino_timestamp_tz(s).unwrap_or_else(|| { + parse_trino_timestamp_tz(s, session).unwrap_or_else(|| { tracing::warn!( raw = s, "failed to parse Trino TIMESTAMP WITH TIME ZONE string" @@ -1272,26 +1286,17 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { Value::String(s) => ColumnValue::Json(s), other => ColumnValue::Json(other.to_string()), }, - TrinoTy::IntervalYearToMonth => { - if let Value::String(ref s) = val { - parse_interval_year_month(s).unwrap_or_else(|| { - tracing::warn!(raw = s, "failed to parse Trino INTERVAL YEAR TO MONTH"); - ColumnValue::String(s.clone()) - }) - } else { - ColumnValue::String(val.to_string()) - } - } - TrinoTy::IntervalDayToSecond => { - if let Value::String(ref s) = val { - parse_interval_day_time(s).unwrap_or_else(|| { - tracing::warn!(raw = s, "failed to parse Trino INTERVAL DAY TO SECOND"); - ColumnValue::String(s.clone()) - }) - } else { - ColumnValue::String(val.to_string()) - } - } + // Intervals are delivered as Trino's own text, not parsed into fields. + // Power BI folds a slicer on an interval column to + // `cast(col as VARCHAR) = ''`, so the shown text must be + // exactly what Trino's CAST renders (`-1-0`, `0 00:00:00.500`); core's + // rendering of structured fields (`-1-00`, `0 00:00:00.5`) matched no + // row. Core still converts this text to the SQL_C_INTERVAL_* types, per + // the spec's "SQL to C: Character" table. + TrinoTy::IntervalYearToMonth | TrinoTy::IntervalDayToSecond => match val { + Value::String(s) => ColumnValue::String(s), + other => ColumnValue::String(other.to_string()), + }, // VARBINARY arrives as a base64-encoded string in the REST API payload. TrinoTy::VarBinary => { if let Value::String(ref s) = val { @@ -1307,7 +1312,7 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { if let Value::Array(items) = val { let vals = items .into_iter() - .map(|v| json_to_column_value(v, inner_ty)) + .map(|v| json_to_column_value(v, inner_ty, session)) .collect(); ColumnValue::Array(vals) } else { @@ -1319,8 +1324,8 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { let pairs = map .into_iter() .map(|(k, v)| { - let key_col = json_to_column_value(Value::String(k), key_ty); - let val_col = json_to_column_value(v, val_ty); + let key_col = json_to_column_value(Value::String(k), key_ty, session); + let val_col = json_to_column_value(v, val_ty, session); (key_col, val_col) }) .collect(); @@ -1334,7 +1339,7 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { let vals = items .into_iter() .zip(fields.iter()) - .map(|(v, (_name, ty))| json_to_column_value(v, ty)) + .map(|(v, (_name, ty))| json_to_column_value(v, ty, session)) .collect(); ColumnValue::Row(vals) } else { @@ -1346,7 +1351,7 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { let vals = items .into_iter() .zip(fields.iter()) - .map(|(v, ty)| json_to_column_value(v, ty)) + .map(|(v, ty)| json_to_column_value(v, ty, session)) .collect(); ColumnValue::Row(vals) } else { @@ -1354,7 +1359,7 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { } } // Nullable wrapper: delegate to the inner type (null already handled above) - TrinoTy::Option(inner) => json_to_column_value(val, inner), + TrinoTy::Option(inner) => json_to_column_value(val, inner, session), _ => match val { Value::String(s) => ColumnValue::String(s), other => ColumnValue::String(other.to_string()), @@ -1366,6 +1371,180 @@ pub fn json_to_column_value(val: Value, ty: &TrinoTy) -> ColumnValue { mod tests { use super::*; + /// Convert in a UTC session, which is what every test below that does not + /// name a session zone is about. + fn convert(val: Value, ty: &TrinoTy) -> ColumnValue { + json_to_column_value(val, ty, SessionZone::UTC) + } + + fn timestamp( + (year, month, day): (i16, u16, u16), + (hour, minute, second): (u16, u16, u16), + fraction: u32, + ) -> ColumnValue { + ColumnValue::Timestamp { + year, + month, + day, + hour, + minute, + second, + fraction, + } + } + + fn convert_in(val: Value, ty: &TrinoTy, zone: &str) -> ColumnValue { + json_to_column_value( + val, + ty, + SessionZone::parse(zone).expect("a zone Trino reports"), + ) + } + + fn tz_in(raw: &str, zone: &str) -> ColumnValue { + json_to_column_value( + Value::String(raw.into()), + &TrinoTy::TimestampWithTimeZone, + SessionZone::parse(zone).expect("a zone Trino reports"), + ) + } + + /// A timestamp-with-time-zone value is delivered as wall time in the + /// session time zone. Power BI folds the value it was shown back as a + /// plain `TIMESTAMP` literal, which Trino reads in the session zone; the + /// two only agree if the shown value is already in that zone. + #[test] + fn timestamp_tz_is_delivered_in_the_session_zone() { + // Summer: CEST, UTC+2. + assert_eq!( + tz_in("2025-06-15 12:30:00.000000 UTC", "Europe/Berlin"), + timestamp((2025, 6, 15), (14, 30, 0), 0) + ); + // Winter: CET, UTC+1. + assert_eq!( + tz_in("2025-01-15 12:30:00.000000 UTC", "Europe/Berlin"), + timestamp((2025, 1, 15), (13, 30, 0), 0) + ); + // A UTC session keeps UTC. + assert_eq!( + tz_in("2025-06-15 12:30:00.000000 UTC", "UTC"), + timestamp((2025, 6, 15), (12, 30, 0), 0) + ); + } + + /// The value's own zone and the session's are independent: the instant is + /// fixed by the first and rendered in the second, across a date boundary. + #[test] + fn timestamp_tz_moves_from_its_own_zone_to_the_session_zone() { + // 20:21:22.123 EDT (UTC-4) = 00:21:22.123 UTC = 01:21:22.123 CET. + assert_eq!( + tz_in("2025-03-10 20:21:22.123 America/New_York", "Europe/Berlin"), + timestamp((2025, 3, 11), (1, 21, 22), 123_000_000) + ); + } + + /// Trino reports a fixed-offset session zone as `+05:30` from + /// `current_timezone()` after `SET TIME ZONE '+05:30'`. + #[test] + fn timestamp_tz_is_delivered_in_a_fixed_offset_session_zone() { + assert_eq!( + tz_in("2025-06-15 12:30:00.000000 UTC", "+05:30"), + timestamp((2025, 6, 15), (18, 0, 0), 0) + ); + assert_eq!( + tz_in("2025-06-15 12:30:00.000000 UTC", "-08:00"), + timestamp((2025, 6, 15), (4, 30, 0), 0) + ); + } + + /// Both instants of the autumn overlap hour render as the same wall time: + /// an instant has exactly one wall time, so this direction is unambiguous. + /// The ambiguity is Trino's, reading the folded literal back, and it picks + /// the later instant (measured on Trino 483); a value in the first 02:30 + /// hour therefore cannot be selected by a Power BI slicer. + #[test] + fn both_instants_of_the_dst_overlap_render_as_the_same_wall_time() { + for utc in [ + "2025-10-26 00:30:00.000000 UTC", + "2025-10-26 01:30:00.000000 UTC", + ] { + assert_eq!( + tz_in(utc, "Europe/Berlin"), + timestamp((2025, 10, 26), (2, 30, 0), 0), + "{utc}" + ); + } + } + + /// A `TIME WITH TIME ZONE` is delivered as the time of day in the + /// session's offset, as a `TIMESTAMP WITH TIME ZONE` is; the value's own + /// offset only fixes the instant. + #[test] + fn time_with_tz_is_delivered_in_the_session_offset() { + let time = |hour, minute, second| ColumnValue::Time { + hour, + minute, + second, + fraction: 0, + }; + assert_eq!( + parse_trino_time_with_tz("13:14:15+02:00", 120), + Some(time(13, 14, 15)) + ); + assert_eq!( + parse_trino_time_with_tz("13:14:15+00:00", -300), + Some(time(8, 14, 15)) + ); + // Wraps within the day, as the UTC normalisation already did. + assert_eq!( + parse_trino_time_with_tz("23:30:00+00:00", 120), + Some(time(1, 30, 0)) + ); + assert_eq!( + convert_in( + Value::String("13:14:15+00:00".into()), + &TrinoTy::TimeWithTimeZone, + "+05:30" + ), + time(18, 44, 15) + ); + } + + /// A time has no date, so a named session zone is resolved at the current + /// instant, which is what Trino does when it casts `TIME` to + /// `TIME WITH TIME ZONE` (`+02:00` in Europe/Berlin on 2026-10-07). + #[test] + fn a_session_zones_offset_depends_on_the_day_it_is_read() { + let at = |y, m, d| { + chrono::NaiveDate::from_ymd_opt(y, m, d) + .and_then(|d| d.and_hms_opt(12, 0, 0)) + .expect("a valid date") + }; + let berlin = SessionZone::parse("Europe/Berlin").expect("a zone"); + assert_eq!(berlin.offset_minutes_at(at(2026, 1, 15)), 60); + assert_eq!(berlin.offset_minutes_at(at(2026, 7, 15)), 120); + let fixed = SessionZone::parse("+05:30").expect("a zone"); + assert_eq!(fixed.offset_minutes_at(at(2026, 1, 15)), 330); + assert_eq!(SessionZone::UTC.offset_minutes_at(at(2026, 7, 15)), 0); + } + + #[test] + fn session_zone_parses_what_current_timezone_reports() { + for name in [ + "UTC", + "Europe/Berlin", + "America/New_York", + "+05:30", + "-08:00", + "+00:00", + ] { + assert!(SessionZone::parse(name).is_some(), "{name}"); + } + for bad in ["", "Europe/Berlim", "+5", "+25:00", "05:30"] { + assert_eq!(SessionZone::parse(bad), None, "{bad:?}"); + } + } + #[test] fn bigint_maps_to_ext_big_int() { assert_eq!( @@ -1403,7 +1582,7 @@ mod tests { // base64("\xDE\xAD\xBE\xEF") == "3q2+7w==" let val = Value::String("3q2+7w==".to_string()); assert_eq!( - json_to_column_value(val, &TrinoTy::VarBinary), + convert(val, &TrinoTy::VarBinary), ColumnValue::Bytes(vec![0xDE, 0xAD, 0xBE, 0xEF]) ); } @@ -1412,7 +1591,7 @@ mod tests { fn varbinary_empty_decodes_to_empty_bytes() { let val = Value::String(String::new()); assert_eq!( - json_to_column_value(val, &TrinoTy::VarBinary), + convert(val, &TrinoTy::VarBinary), ColumnValue::Bytes(Vec::new()) ); } @@ -1421,17 +1600,14 @@ mod tests { fn varbinary_invalid_base64_falls_back_to_string() { let val = Value::String("not!valid!base64".to_string()); assert_eq!( - json_to_column_value(val, &TrinoTy::VarBinary), + convert(val, &TrinoTy::VarBinary), ColumnValue::String("not!valid!base64".to_string()) ); } #[test] fn varbinary_null_maps_to_null() { - assert_eq!( - json_to_column_value(Value::Null, &TrinoTy::VarBinary), - ColumnValue::Null - ); + assert_eq!(convert(Value::Null, &TrinoTy::VarBinary), ColumnValue::Null); } #[test] @@ -1477,16 +1653,13 @@ mod tests { #[test] fn json_null_returns_column_null() { - assert_eq!( - json_to_column_value(Value::Null, &TrinoTy::Varchar), - ColumnValue::Null - ); + assert_eq!(convert(Value::Null, &TrinoTy::Varchar), ColumnValue::Null); } #[test] fn json_string_returns_column_string() { assert_eq!( - json_to_column_value(Value::String("hello".into()), &TrinoTy::Varchar), + convert(Value::String("hello".into()), &TrinoTy::Varchar), ColumnValue::String("hello".into()) ); } @@ -1494,7 +1667,7 @@ mod tests { #[test] fn json_number_bigint_returns_i64() { assert_eq!( - json_to_column_value(serde_json::json!(42), &TrinoTy::TrinoInt(TrinoInt::I64)), + convert(serde_json::json!(42), &TrinoTy::TrinoInt(TrinoInt::I64)), ColumnValue::I64(42) ); } @@ -1504,7 +1677,7 @@ mod tests { #[test] fn out_of_range_integer_for_declared_type_is_an_error_not_a_wrap() { // Server declared INTEGER but sent a value that does not fit i32. - let val = json_to_column_value( + let val = convert( serde_json::json!(4_294_967_296i64), &TrinoTy::TrinoInt(TrinoInt::I32), ); @@ -1515,13 +1688,13 @@ mod tests { #[test] fn in_range_integer_still_converts() { - let val = json_to_column_value(serde_json::json!(42i64), &TrinoTy::TrinoInt(TrinoInt::I32)); + let val = convert(serde_json::json!(42i64), &TrinoTy::TrinoInt(TrinoInt::I32)); assert_eq!(val, ColumnValue::I32(42)); } #[test] fn out_of_range_i16_falls_back_to_text() { - let val = json_to_column_value( + let val = convert( serde_json::json!(70_000i64), &TrinoTy::TrinoInt(TrinoInt::I16), ); @@ -1530,13 +1703,13 @@ mod tests { #[test] fn out_of_range_i8_falls_back_to_text() { - let val = json_to_column_value(serde_json::json!(200i64), &TrinoTy::TrinoInt(TrinoInt::I8)); + let val = convert(serde_json::json!(200i64), &TrinoTy::TrinoInt(TrinoInt::I8)); assert_eq!(val, ColumnValue::String("200".to_string())); } #[test] fn negative_out_of_range_integer_falls_back_to_text() { - let val = json_to_column_value( + let val = convert( serde_json::json!(-2_147_483_649i64), &TrinoTy::TrinoInt(TrinoInt::I32), ); @@ -1545,7 +1718,7 @@ mod tests { #[test] fn out_of_range_float_for_real_is_not_infinity() { - let val = json_to_column_value( + let val = convert( serde_json::json!(1e300f64), &TrinoTy::TrinoFloat(TrinoFloat::F32), ); @@ -1567,7 +1740,7 @@ mod tests { ("Infinity", f64::INFINITY), ("-Infinity", f64::NEG_INFINITY), ] { - let val = json_to_column_value( + let val = convert( serde_json::json!(raw), &TrinoTy::TrinoFloat(TrinoFloat::F64), ); @@ -1591,7 +1764,7 @@ mod tests { ("Infinity", f32::INFINITY), ("-Infinity", f32::NEG_INFINITY), ] { - let val = json_to_column_value( + let val = convert( serde_json::json!(raw), &TrinoTy::TrinoFloat(TrinoFloat::F32), ); @@ -1615,7 +1788,7 @@ mod tests { /// text two literal quote marks it never sent. #[test] fn unparseable_float_string_falls_back_without_json_quotes() { - let val = json_to_column_value( + let val = convert( serde_json::json!("abc"), &TrinoTy::TrinoFloat(TrinoFloat::F64), ); @@ -1624,7 +1797,7 @@ mod tests { #[test] fn in_range_float_for_real_still_converts() { - let val = json_to_column_value( + let val = convert( serde_json::json!(3.5f64), &TrinoTy::TrinoFloat(TrinoFloat::F32), ); @@ -1645,7 +1818,7 @@ mod tests { #[test] fn json_bool_returns_column_bool() { assert_eq!( - json_to_column_value(Value::Bool(true), &TrinoTy::Boolean), + convert(Value::Bool(true), &TrinoTy::Boolean), ColumnValue::Bool(true) ); } @@ -2125,7 +2298,7 @@ mod tests { #[test] fn option_i64_non_null_converts_as_i64() { assert_eq!( - json_to_column_value( + convert( serde_json::json!(99), &TrinoTy::Option(Box::new(TrinoTy::TrinoInt(TrinoInt::I64))) ), @@ -2136,7 +2309,7 @@ mod tests { #[test] fn option_i64_null_returns_null() { assert_eq!( - json_to_column_value( + convert( Value::Null, &TrinoTy::Option(Box::new(TrinoTy::TrinoInt(TrinoInt::I64))) ), @@ -2149,7 +2322,7 @@ mod tests { #[test] fn date_string_parses_to_column_date() { assert_eq!( - json_to_column_value(Value::String("1998-01-14".into()), &TrinoTy::Date), + convert(Value::String("1998-01-14".into()), &TrinoTy::Date), ColumnValue::Date { year: 1998, month: 1, @@ -2169,7 +2342,7 @@ mod tests { #[test] fn a_date_before_1_ce_parses_to_column_date() { assert_eq!( - json_to_column_value(Value::String("-0001-01-01".into()), &TrinoTy::Date), + convert(Value::String("-0001-01-01".into()), &TrinoTy::Date), ColumnValue::Date { year: -1, month: 1, @@ -2185,7 +2358,7 @@ mod tests { #[test] fn a_timestamp_before_1_ce_parses_to_column_timestamp() { assert_eq!( - json_to_column_value( + convert( Value::String("-0001-01-01 12:34:56.789".into()), &TrinoTy::Timestamp ), @@ -2207,8 +2380,8 @@ mod tests { #[test] fn dates_and_timestamps_read_the_same_years() { for year in ["-4713", "-0001", "0000", "0001", "1970", "9999"] { - let date = json_to_column_value(Value::String(format!("{year}-06-15")), &TrinoTy::Date); - let timestamp = json_to_column_value( + let date = convert(Value::String(format!("{year}-06-15")), &TrinoTy::Date); + let timestamp = convert( Value::String(format!("{year}-06-15 00:00:00")), &TrinoTy::Timestamp, ); @@ -2225,7 +2398,7 @@ mod tests { #[test] fn year_zero_parses_to_column_date() { assert_eq!( - json_to_column_value(Value::String("0000-01-01".into()), &TrinoTy::Date), + convert(Value::String("0000-01-01".into()), &TrinoTy::Date), ColumnValue::Date { year: 0, month: 1, @@ -2240,7 +2413,7 @@ mod tests { #[test] fn a_year_beyond_the_date_struct_falls_back_to_text() { assert!(matches!( - json_to_column_value(Value::String("+99999-01-01".into()), &TrinoTy::Date), + convert(Value::String("+99999-01-01".into()), &TrinoTy::Date), ColumnValue::String(_) )); } @@ -2248,7 +2421,7 @@ mod tests { #[test] fn time_string_parses_to_column_time() { assert_eq!( - json_to_column_value(Value::String("13:14:15".into()), &TrinoTy::Time), + convert(Value::String("13:14:15".into()), &TrinoTy::Time), ColumnValue::Time { hour: 13, minute: 14, @@ -2263,7 +2436,7 @@ mod tests { // The fraction is kept, not discarded: SQL_TIME_STRUCT cannot carry // it, but the SQL_C_CHAR/SQL_C_WCHAR string rendering can. assert_eq!( - json_to_column_value(Value::String("09:05:03.336".into()), &TrinoTy::Time), + convert(Value::String("09:05:03.336".into()), &TrinoTy::Time), ColumnValue::Time { hour: 9, minute: 5, @@ -2276,7 +2449,7 @@ mod tests { #[test] fn time_with_timezone_parses_correctly() { assert_eq!( - json_to_column_value( + convert( Value::String("13:14:15.000 UTC".into()), &TrinoTy::TimeWithTimeZone ), @@ -2294,7 +2467,7 @@ mod tests { // The two "with time zone" types must agree: TIMESTAMP WITH TIME // ZONE converts to UTC, so discarding the offset here rather than // applying it would make TIME WITH TIME ZONE contradict it. - let val = parse_trino_time_with_tz("13:14:15+02:00").expect("parses"); + let val = parse_trino_time_with_tz("13:14:15+02:00", 0).expect("parses"); assert_eq!( val, ColumnValue::Time { @@ -2308,7 +2481,7 @@ mod tests { #[test] fn time_with_negative_offset_normalises_to_utc() { - let val = parse_trino_time_with_tz("13:14:15-05:30").expect("parses"); + let val = parse_trino_time_with_tz("13:14:15-05:30", 0).expect("parses"); assert_eq!( val, ColumnValue::Time { @@ -2322,7 +2495,7 @@ mod tests { #[test] fn time_with_offset_wraps_across_midnight() { - let val = parse_trino_time_with_tz("01:00:00+02:00").expect("parses"); + let val = parse_trino_time_with_tz("01:00:00+02:00", 0).expect("parses"); assert_eq!( val, ColumnValue::Time { @@ -2336,7 +2509,7 @@ mod tests { #[test] fn time_with_utc_offset_is_unchanged() { - let val = parse_trino_time_with_tz("13:14:15.000 UTC").expect("parses"); + let val = parse_trino_time_with_tz("13:14:15.000 UTC", 0).expect("parses"); assert_eq!( val, ColumnValue::Time { @@ -2352,7 +2525,7 @@ mod tests { fn time_with_timezone_keeps_fraction_through_offset_shift() { // The offset shift only touches whole minutes, so a fractional-seconds // part must survive `shift_time` unchanged. - let val = parse_trino_time_with_tz("13:14:15.123456+02:00").expect("parses"); + let val = parse_trino_time_with_tz("13:14:15.123456+02:00", 0).expect("parses"); assert_eq!( val, ColumnValue::Time { @@ -2371,9 +2544,9 @@ mod tests { // `i32` and only overflows when converted to minutes. Release builds // carry no overflow checks, so an unchecked multiply here reports a // *different* time instead of declining the value. - assert_eq!(parse_trino_time_with_tz("00:00:00+949378864"), None); + assert_eq!(parse_trino_time_with_tz("00:00:00+949378864", 0), None); assert_eq!( - json_to_column_value( + convert( Value::String("+999\0\0\0\0+00949378864".into()), &TrinoTy::Option(Box::new(TrinoTy::TimeWithTimeZone)), ), @@ -2386,7 +2559,7 @@ mod tests { // The same class one frame down, in `shift_time`: the time-of-day hour // is text too, so `hour * 60` overflows before the offset is ever // applied. - assert_eq!(parse_trino_time_with_tz("2147483647:00:00+01:00"), None); + assert_eq!(parse_trino_time_with_tz("2147483647:00:00+01:00", 0), None); } #[test] @@ -2394,7 +2567,7 @@ mod tests { // The checked arithmetic must not narrow what a valid offset can be: // the widest zones in use are +14:00 and -12:00. assert_eq!( - parse_trino_time_with_tz("13:14:15+14:00"), + parse_trino_time_with_tz("13:14:15+14:00", 0), Some(ColumnValue::Time { hour: 23, minute: 14, @@ -2403,7 +2576,7 @@ mod tests { }) ); assert_eq!( - parse_trino_time_with_tz("13:14:15-12:00"), + parse_trino_time_with_tz("13:14:15-12:00", 0), Some(ColumnValue::Time { hour: 1, minute: 14, @@ -2416,7 +2589,7 @@ mod tests { #[test] fn timestamp_string_parses_to_column_timestamp() { assert_eq!( - json_to_column_value( + convert( Value::String("1998-01-14 13:14:15".into()), &TrinoTy::Timestamp ), @@ -2435,7 +2608,7 @@ mod tests { #[test] fn timestamp_with_millis_converts_fraction_to_nanoseconds() { assert_eq!( - json_to_column_value( + convert( Value::String("1998-01-14 13:14:15.123".into()), &TrinoTy::Timestamp ), @@ -2454,7 +2627,7 @@ mod tests { /// UTC is a no-op conversion: fields should pass through unchanged. #[test] fn timestamp_with_named_timezone_converts_to_utc() { - let val = json_to_column_value( + let val = convert( Value::String("1998-01-14 13:14:15.000 UTC".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2474,115 +2647,60 @@ mod tests { #[test] fn date_null_returns_null() { - assert_eq!( - json_to_column_value(Value::Null, &TrinoTy::Date), - ColumnValue::Null - ); + assert_eq!(convert(Value::Null, &TrinoTy::Date), ColumnValue::Null); } #[test] fn decimal_maps_to_decimal_variant() { use serde_json::json; - let val = json_to_column_value(json!("123.456"), &TrinoTy::Decimal(6, 3)); + let val = convert(json!("123.456"), &TrinoTy::Decimal(6, 3)); assert_eq!(val, ColumnValue::Decimal("123.456".to_string())); } #[test] fn json_ty_maps_to_json_variant() { use serde_json::json; - let val = json_to_column_value(json!(r#"{"a":1}"#), &TrinoTy::Json); + let val = convert(json!(r#"{"a":1}"#), &TrinoTy::Json); assert_eq!(val, ColumnValue::Json(r#"{"a":1}"#.to_string())); } - #[test] - fn interval_year_month_parses_correctly() { + /// An interval column is delivered as Trino's own text, unchanged. + /// + /// Power BI folds a slicer on an interval column to + /// `cast(col as VARCHAR) = ''`, so the text an application is + /// shown must be exactly what Trino's `CAST(... AS VARCHAR)` produces, or + /// the filter silently selects nothing. Trino renders `INTERVAL '-1' YEAR` + /// as `-1-0`; a parse into structured fields re-rendered by core gave + /// `-1-00`. + #[test] + fn interval_year_month_is_delivered_as_trinos_text() { use serde_json::json; - let val = json_to_column_value(json!("3-7"), &TrinoTy::IntervalYearToMonth); - assert_eq!( - val, - ColumnValue::IntervalYearMonth { - years: 3, - months: 7, - precision: Interval::YearToMonth, - } - ); + for raw in ["3-7", "-1-0", "0-3"] { + let val = convert(json!(raw), &TrinoTy::IntervalYearToMonth); + assert_eq!(val, ColumnValue::String(raw.to_string()), "{raw}"); + } } + /// The day-time counterpart: `INTERVAL '0.5' SECOND` is `0 00:00:00.500` + /// in Trino, which core's re-rendering turned into `0 00:00:00.5`. #[test] - fn interval_day_time_parses_correctly() { + fn interval_day_time_is_delivered_as_trinos_text() { use serde_json::json; - let val = json_to_column_value(json!("2 03:04:05.678"), &TrinoTy::IntervalDayToSecond); - assert_eq!( - val, - ColumnValue::IntervalDayTime { - total_nanoseconds: 2 * NANOS_PER_DAY - + 3 * NANOS_PER_HOUR - + 4 * NANOS_PER_MINUTE - + 5 * NANOS_PER_SECOND - + 678_000_000, - precision: Interval::DayToSecond, - } - ); - } - - #[test] - fn negative_interval_day_time_is_fully_negative() { - let val = parse_interval_day_time("-2 03:04:05.678").expect("parses"); - // -(2 days + 3h4m5.678s) = -183_845_678 ms in nanoseconds. - assert_eq!( - val, - ColumnValue::IntervalDayTime { - total_nanoseconds: -183_845_678_000_000, - precision: Interval::DayToSecond, - } - ); - } - - #[test] - fn negative_zero_day_interval_keeps_its_sign() { - // "-0 03:04:05" must keep its sign: parsing the sign only off `days` - // loses it entirely, because "-0".parse::() is 0. - let val = parse_interval_day_time("-0 03:04:05").expect("parses"); - assert_eq!( - val, - ColumnValue::IntervalDayTime { - total_nanoseconds: -11_045_000_000_000, - precision: Interval::DayToSecond, - } - ); - } - - #[test] - fn positive_interval_day_time_is_unchanged() { - let val = parse_interval_day_time("2 03:04:05.678").expect("parses"); - assert_eq!( - val, - ColumnValue::IntervalDayTime { - total_nanoseconds: 183_845_678_000_000, - precision: Interval::DayToSecond, - } - ); - } - - /// A fraction finer than Trino's own millisecond rendering survives now that - /// the variant counts nanoseconds: the parser no longer truncates at three - /// digits. - #[test] - fn interval_day_time_keeps_sub_millisecond_digits() { - let val = parse_interval_day_time("0 00:00:01.234567").expect("parses"); - assert_eq!( - val, - ColumnValue::IntervalDayTime { - total_nanoseconds: NANOS_PER_SECOND + 234_567_000, - precision: Interval::DayToSecond, - } - ); + for raw in [ + "2 03:04:05.678", + "-1 00:00:00.000", + "-0 03:04:05.000", + "0 00:00:00.500", + ] { + let val = convert(json!(raw), &TrinoTy::IntervalDayToSecond); + assert_eq!(val, ColumnValue::String(raw.to_string()), "{raw}"); + } } /// Numeric offset +05:30: 10:30 local = 05:00 UTC (subtract 5h30m). #[test] fn timestamp_with_tz_numeric_offset_converts_to_utc() { - let val = json_to_column_value( + let val = convert( Value::String("2024-03-15 10:30:00.000 +05:30".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2602,7 +2720,7 @@ mod tests { #[test] fn timestamp_tz_named_utc_converts_to_utc_timestamp() { - let val = json_to_column_value( + let val = convert( Value::String("2020-05-05 22:00:00.000 UTC".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2624,7 +2742,7 @@ mod tests { fn timestamp_tz_named_zone_converts_to_utc() { // America/New_York in March 2025 is EDT (UTC-4). // 20:21:22 EDT = 2025-03-11 00:21:22 UTC (date rolls forward). - let val = json_to_column_value( + let val = convert( Value::String("2025-03-10 20:21:22.123 America/New_York".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2646,7 +2764,7 @@ mod tests { fn timestamp_tz_numeric_offset_converts_to_utc() { // +05:30 means wall clock is 5h30m ahead of UTC. // 10:30:00 +05:30 = 05:00:00 UTC (same day). - let val = json_to_column_value( + let val = convert( Value::String("2024-03-15 10:30:00.000 +05:30".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2667,7 +2785,7 @@ mod tests { #[test] fn timestamp_tz_negative_offset_converts_to_utc() { // -08:00: 16:00:00 PST = 2024-12-16 00:00:00 UTC (date rolls forward). - let val = json_to_column_value( + let val = convert( Value::String("2024-12-15 16:00:00.000 -08:00".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2689,7 +2807,7 @@ mod tests { fn timestamp_tz_dst_winter_converts_correctly() { // America/New_York in December is EST (UTC-5). // 23:00:00 EST = 2025-01-02 04:00:00 UTC. - let val = json_to_column_value( + let val = convert( Value::String("2025-01-01 23:00:00.000 America/New_York".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2713,7 +2831,7 @@ mod tests { fn timestamp_tz_posix_abbreviation_cet_converts_to_utc() { // CET (Central European Time) = UTC+1. // 15:00:00 CET = 14:00:00 UTC. - let val = json_to_column_value( + let val = convert( Value::String("2025-01-15 15:00:00.000 CET".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2736,7 +2854,7 @@ mod tests { /// a winter date so the expected result matches CET. #[test] fn timestamp_tz_europe_berlin_winter_converts_to_utc() { - let val = json_to_column_value( + let val = convert( Value::String("2025-01-15 15:00:00.000 Europe/Berlin".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2759,7 +2877,7 @@ mod tests { fn timestamp_tz_europe_berlin_summer_converts_to_utc() { // 2025-07-15 is in CEST (UTC+2). // 15:00:00 CEST = 13:00:00 UTC. - let val = json_to_column_value( + let val = convert( Value::String("2025-07-15 15:00:00.000 Europe/Berlin".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2782,7 +2900,7 @@ mod tests { #[test] fn timestamp_tz_hour_only_numeric_offset() { // +05 = +05:00. 10:00:00 +05:00 = 05:00:00 UTC. - let val = json_to_column_value( + let val = convert( Value::String("2024-06-01 10:00:00.000 +05".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2803,7 +2921,7 @@ mod tests { /// Zero offset (+00:00) is equivalent to UTC. #[test] fn timestamp_tz_zero_numeric_offset() { - let val = json_to_column_value( + let val = convert( Value::String("2024-06-01 10:00:00.000 +00:00".into()), &TrinoTy::TimestampWithTimeZone, ); @@ -2825,7 +2943,7 @@ mod tests { /// `TrinoTy::Timestamp`, and no UTC conversion applies to it. #[test] fn timestamp_no_tz_is_unaffected_by_tz_changes() { - let val = json_to_column_value( + let val = convert( Value::String("2025-06-15 09:30:45.678".into()), &TrinoTy::Timestamp, ); @@ -2847,7 +2965,7 @@ mod tests { #[test] fn timestamp_tz_preserves_fraction_through_conversion() { // 10:30:00.999 +05:30 = 05:00:00.999 UTC; fraction stays 999ms. - let val = json_to_column_value( + let val = convert( Value::String("2024-03-15 10:30:00.999 +05:30".into()), &TrinoTy::TimestampWithTimeZone, );