Skip to content

Fix Power BI DirectQuery slicers on text, interval, time and timestamptz columns - #11

Merged
adwk67 merged 20 commits into
mainfrom
fix/review-feedback
Oct 9, 2026
Merged

adwk67 merged 20 commits into
mainfrom
fix/review-feedback

Conversation

@adwk67

@adwk67 adwk67 commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

What this changes

Issue: a Power BI DirectQuery slicer on a text column failed with Cannot cast varchar(…) to interval day to second. Fixing it exposed three more column types whose slicers silently selected no rows. Driver and connector
must be upgraded together.

Column type Before After
varchar Folded CAST(… AS INTERVAL DAY TO SECOND) → error; unquoted constants VARCHAR leads SQL_WVARCHAR in SQLGetTypeInfo (core preferred row); connector quotes and escapes text constants
interval … Driver re-rendered -1-00, 0 00:00:00.5; Power BI's cast(col as VARCHAR) = '<shown>' matched nothing Delivered as Trino's own text (-1-0, 0 00:00:00.500); SQL_C_INTERVAL_* still works via core
time Power BI folds against 1899-12-30, Trino casts onto today → 0 rows Connector reports SQL_CONVERT_TIME without SQL_CVT_TIMESTAMP → visible "couldn't fold" error; README workarounds (view with CAST(col AS VARCHAR), Import mode)
timestamp with time zone Delivered in UTC; Trino reads the folded literal in the session zone → matched only in UTC sessions Delivered as wall time in the session zone (SET TIME ZONE > TimeZone= > coordinator default via current_timezone() in the connect probe); time with time zone likewise, at the zone's current offset

Also: rustls 0.23.45 (RUSTSEC-2026-0285); Windows suites now run with UTF-8 output (two suites crashed on cp1252).
Core is pinned to 2f9f725 by a TEMPORARY rev — switch both Cargo.toml entries to the core release tag before
merging.

Spec basis

  • SQLGetTypeInfo ordering ("how closely the data type maps"): see the core PR.
  • SQL to C: Character → all C interval types: see the core PR. Interval columns are described as SQL_WVARCHAR
    (unchanged), so the character table is the one that governs.
  • SQL to C: Time, footnote [c]: "The date fields of the timestamp structure are set to the current date" —
    https://learn.microsoft.com/en-us/sql/odbc/reference/appendixes/sql-to-c-time — Trino's CAST(time AS timestamp)
    follows ODBC; Power BI's 1899-12-30 anchor does not. The SQL_CONVERT_TIME override is therefore a deliberate
    misreport, documented in the connector and pinned by test_folding_contract.py.

Trino behaviour (measured, Trino 483)

  • CAST(interval AS VARCHAR): -1-0, 0 00:00:00.500, 1 00:00:00.000.
  • CAST(CAST(TIME '14:30:00' AS TIMESTAMP) AS DATE) = current_date → true.
  • Berlin session: from_iso8601_timestamp('2025-10-26T00:30:00Z') = CAST('2025-10-26 02:30:00' AS TIMESTAMP) →
    false, …01:30:00Z… → true (Trino picks the later instant of the DST overlap; pinned in the slicer suite, README
    Troubleshooting).
  • SET TIME ZONE sets the session property time_zone_id; current_timezone() may be a fixed offset (+05:30).
  • CAST(TIME '12:00:00' AS TIME WITH TIME ZONE) uses the session zone's current offset (+02:00 in Europe/Berlin on
    2026-10-07).

Checklist

  • pre-commit run --all-files passes (2026-10-08; run outside the nono sandbox, whose loopback restriction
    fails two backend::tests that need a local socket).
  • CHANGELOG.md has entries under ## [Unreleased] (Changed + Fixed, including backfilled entries for the
    type-info and connector-quoting commits).
  • No client error added or moved outside map_trino_error.
  • New tests were checked failing first or by mutation: acceptance suite red before the fixes; interval unit/FFI
    tests red (3-07, -1-00); SQL_CONVERT_TIME contract red without the override; session-zone FFI tests fail
    when the driver delivers UTC or ignores SET TIME ZONE; timetz FFI test fails with UTC delivery.

If it applies

  • Integration suite against a live Trino: all suites green (pbi slicer semantics 64/0, folding contract 41/0,
    type matrix 690/0, ffi 78/0).
  • Windows suites in the VM (full run 2026-10-08, fe0e43f): integration ×4 32/0, sql surface 71/0, escape
    sequences 123/0, session keys 10/0, folding contract 41/0, pbi slicer semantics 64/0, tls 19/0, raw C ABI
    154/0, describe param 26/0, type matrix 690/0, transactions 10/0; spooling fails (pre-existing, see notes);
    oauth and harness unit tests skipped by design.
  • Core defects fixed in core (preferred row, char→interval conversion, overflow); needs core 2f9f725 or the
    release that contains it.
  • Connector rebuilt and loaded in Power BI Desktop (2026-10-08): text, interval, timestamptz slicers select
    their rows (UTC and TimeZone=Europe/Berlin); time slicer shows the fold error; Trino history: 0 failed
    queries.

Notes for the reviewer

  • Visible to every application: interval text format and timestamptz/timetz values outside a UTC session change
    (CHANGELOG "Changed").
  • Windows spooling fallback check fails with ABANDONED_QUERY — pre-existing, not caused by this branch: a
    2,000-row fetch takes 69–187 s on the Windows VM with both main and this branch (Linux 1.6 s), so 20,000 rows
    leave >5 min between page fetches. Not CPU-, disk- or memory-bound; under separate investigation.
    Explained: ODBC
    Driver Manager tracing was switched on in the test VM. With it off, the same 2,000 rows take 1.3–1.7 s on Windows
    (42–172 s with it on); not a driver defect.
  • Not verified: Power BI Service via the on-premises data gateway.
  • timestamp_with_utc_tz_returns_utc_via_get_data has the same "only in a UTC session" naming as the renamed test;
    left unchanged.

Update 2026-10-09: CI and test follow-ups

  • 6e83771 test: MinIO for the spooling CI leg now comes from docker.io/pgsty/minio and docker.io/pgsty/mc
    (as stackabletech/demos uses); pulls of quay.io/minio were refused with "unauthorized". Spooling suite locally:
    10 passed, 1 skipped by design.
  • 7c2722d test: a_timeout_names_the_cause_beneath_reqwest listens on the fixed loopback port 29871 instead of an
    OS-assigned one, so a per-port sandbox (Landlock) can allow it; pre-commit run --all-files now passes inside one.
  • chore: core pin 2f9f725 → 53e5a89, which adds exact numeric → SQL_C_INTERVAL_* conversion and
    01S07 for a non-zero interval-text digit past the ninth (see the core PR). No driver code change; with the new
    pin cargo test --locked -- --include-ignored 514 passed, every integration suite passes (pbi slicer semantics
    64/0, spooling 10/0), pre-commit run --all-files 16/16.
  • e1b3c91 chore: core pin → release tag v0.1.1 (4f5e802, the squash-merged core work plus the version bump; same code as
    53e5a89). Re-verified: 514 passed, all integration suites pass, pre-commit 16/16.
  • a466d0b test: the Windows runner stops when ODBC Driver Manager tracing is on in the VM (it made reads 25–130×
    slower and caused the spooling ABANDONED_QUERY noted above); --allow-dm-trace overrides. Documented in
    WINDOWS.md; the README's troubleshooting section covers ABANDONED_QUERY / Query not found on large reads.
    Full Windows run green, spooling included.
  • e26349e test: re-running setup.sh on a running stack no longer leaves Trino with an empty /etc/trino; a changed
    config fragment now recreates Trino (config hash as a compose label). Full Linux run green.
  • c7c0b81 docs: WINDOWS.md, the README and the runner's doc comment describe the tracing slowdown qualitatively
    instead of quoting test-VM timings.
  • 197cf66 docs: the README's Power BI section notes that incremental-refresh bounds on a timestamp with time zone
    column follow the session time zone (TimeZone). Background: in Power BI Desktop the RangeStart/RangeEnd filter
    folds into one Trino query for timestamp, timestamp with time zone and date (after Change Type → Date/Time)
    columns, with no folding warning in the policy dialog. The bounds arrive as plain TIMESTAMP literals, so for a
    timestamptz column Trino compares them in the session zone (checked in the Trino CLI: the same boundary comparison is
    false in UTC and true in Europe/Berlin). No code change.

🤖 Generated with Claude Code

adwk67 and others added 12 commits October 6, 2026 17:34
…isitor

Power Query hands text constants to the visitor unquoted (verified in Power BI
Desktop), so `Cast(_, "VARCHAR")` folded a slicer into `CAST(hello world as
VARCHAR)`. Quote the value and double single quotes. Adds a round-trip check to
test_folding_contract.py and ordinary text rows (incl. O'Brien) to types_test.

Together with the preferred VARCHAR type-info row this fixes DirectQuery slicers
on text columns; the two changes must ship together.
…BI tests

No table in the stack has an INTERVAL column: a Hive table cannot store the
type and the postgresql catalog does not expose Postgres intervals. seed-hive.sh
now also creates hive.tx.interval_test, a view over postgresql.public.types_test
with an INTERVAL DAY TO SECOND and an INTERVAL YEAR TO MONTH column, alongside
the table's text, integer, date, time(6), timestamp(6) and timestamp(6) with
time zone columns, so Power BI slicers on each type can be tested from one view.
…ling)

Power BI folds a slicer selection into a filter built from the value it showed.
For interval, time and timestamp-with-time-zone columns that filter silently
selects no rows. test_pbi_slicer_semantics.py replays the filters Power BI
Desktop sent on 2026-10-07 through the driver and checks the row counts, in the
default session time zone and in Europe/Berlin.

It fails on purpose: it is the acceptance test for the fixes that follow
(intervals, time, timestamptz). seed-hive.sh adds negative and fractional
interval values to hive.tx.interval_test for it.
A Power BI DirectQuery slicer on an interval column folds to
cast(col as VARCHAR) = '<shown value>', so the value shown has to be exactly
what Trino's CAST renders. The driver parsed interval text into fields and core
re-rendered them, which gave -1-00 for Trino's -1-0 and 0 00:00:00.5 for
0 00:00:00.500: the filter ran, matched nothing, and the report showed no rows
without an error.

Interval columns are now delivered as the text Trino sent. Reading them as
SQL_C_INTERVAL_* still works, because stackable-odbc-core now converts
character data to the interval C types as the SQL to C: Character table
requires; core is pinned to that commit (b37ca99) until a release is tagged.
The two interval parsers, which nothing else used, are removed.

Tests: unit tests expect the text verbatim; the FFI tests compare each slicer
view value against Trino's own CAST AS VARCHAR and read both interval families
back as SQL_INTERVAL_STRUCT; the type matrix checks the interval text. The
slicer-semantics suite's interval checks pass; its time and
timestamp-with-time-zone checks still fail, as intended until those fixes land.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A DirectQuery 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, like ODBC's own conversion
tables (SQL to C: Time, footnote [c]), puts a cast time on the current date,
so the filter ran and silently selected no rows.

The connector now reports SQL_CONVERT_TIME as the driver's answer minus
SQL_CVT_TIMESTAMP. Power BI then refuses the fold with "We couldn't fold the
expression to the data source" instead of showing an empty report, which was
measured in Power BI Desktop, as was "(Blank)" still folding. It is a
deliberate misreport, and the connector comment says so.

Two alternatives were tried in Power BI Desktop and rejected: presenting time
columns as WVARCHAR through SQLColumns stopped the table folding at all, and
Trino rejects time = varchar anyway; changing the column to Text in Power
Query is refused in DirectQuery.

test_folding_contract.py pins the override to the driver's answer minus that
one bit and checks that Trino still anchors on the current date. The slicer
suite no longer checks time values, which Power BI no longer sends. The README
documents the limitation and two workarounds, a view casting the column to
VARCHAR and Import mode, both checked in Power BI Desktop, along with Import
mode's blank-equals-midnight behaviour.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Power BI folds a slicer on a timestamp-with-time-zone column to
"col" = CAST('<shown value>' as TIMESTAMP), and Trino reads that plain
TIMESTAMP literal in the session time zone. The driver delivered these values
in UTC, so the filter only selected its rows in a UTC session; with
TimeZone=Europe/Berlin it silently matched nothing.

Values are now delivered as wall time in the session zone, the zone Trino
itself uses for a plain TIMESTAMP. The zone is worked out per statement, in
Trino's own order: a SET TIME ZONE in force (the time_zone_id session property
the client records), else TimeZone=, else the coordinator's default, which the
connect probe now reads with current_timezone() at no extra round trip. Trino
can report a fixed offset such as +05:30, so SessionZone keeps both named
zones and offsets. Every page of a result set uses the zone read at execution.

An instant has one wall time, so the driver's direction is unambiguous; the two
instants of an autumn overlap hour are both shown as the same time. Reading the
folded literal back, Trino picks the later instant, so a slicer cannot select a
value from the first repeated hour. The slicer suite pins that, and the README
says so under Troubleshooting.

Tests: unit tests for the conversion in named, fixed-offset and DST-overlap
cases and for the zone precedence; FFI tests comparing delivered values with
Trino's own at_timezone(x, current_timezone()) across TimeZone=, SET TIME ZONE
and SET TIME ZONE LOCAL, both of which fail if the driver delivers UTC
regardless; the slicer suite's Europe/Berlin checks now pass. The README's
TimeZone row, which ended mid-sentence, is completed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ixes

0e858e6 (the preferred SQLGetTypeInfo row) and dd31b43 (quoting text
constants in the connector's Constant visitor) are both observable by
applications but landed without changelog entries. Both are needed for
DirectQuery slicers on text columns, so the entry for the connector says that
the driver and the connector have to be upgraded together.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Over WinRM a suite's stdout is not a console, so Python on the VM encoded it
as cp1252. The folding contract and Power BI slicer suites print non-Latin
test values (日本語, emoji), so both died with UnicodeEncodeError part-way
through and were reported as failed. The runner already decodes the output as
UTF-8, so run_remote now sets PYTHONIOENCODING=utf-8 to match.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-ups from the whole-branch review of fix/review-feedback.

TIME WITH TIME ZONE was still normalised to UTC while TIMESTAMP WITH TIME
ZONE now follows the session zone, so with TimeZone=Europe/Berlin the two
columns of one row disagreed, and the comment justifying UTC ("matches what
TIMESTAMP WITH TIME ZONE already does") had become false. A time has no
date, so a named session zone is resolved at its current offset, which is
what Trino does when it casts TIME to TIME WITH TIME ZONE (+02:00 in
Europe/Berlin on 2026-10-07, measured). An FFI test checks the delivered time
against Trino's own cast in a UTC session, with TimeZone=Europe/Berlin and
after SET TIME ZONE; it fails if the time is delivered in UTC.

The connect probe now warns when current_timezone() returns a zone the driver
cannot read, instead of silently falling back to UTC, as the SET TIME ZONE
path already did.

timestamp_with_named_tz_returns_utc_via_get_data is renamed
timestamp_with_named_tz_returns_utc_in_a_utc_session, which is what it now
tests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Picks up the core fix that reports interval text with an oversized leading
field as 22015 instead of wrapping it into a small value reported as success,
and bounds the month field to 0-11. Still a temporary commit pin until a core
release containing the branch's changes is tagged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@adwk67 adwk67 self-assigned this Oct 8, 2026
@adwk67
adwk67 marked this pull request as ready for review October 9, 2026 07:51
@adwk67
adwk67 requested a review from maltesander October 9, 2026 07:51
adwk67 and others added 8 commits October 9, 2026 10:14
The spooling CI leg failed before any test ran: pulls of
quay.io/minio/minio and quay.io/minio/mc were refused with
"unauthorized: access to the requested resource is not authorized".

Switch both to pgsty's builds, as stackabletech/demos already does
(stacks/airflow/minio.yaml): docker.io/pgsty/minio:RELEASE.2026-08-04T00-00-00Z
and docker.io/pgsty/mc:RELEASE.2026-09-16T00-00-00Z. Both carry /bin/sh,
which minio-init's retry loop needs.

Verified locally with setup.sh --profile spooling: the bucket is created
and the spooling suite passes (10 passed, 1 skipped by design).

The workflow comment that named quay.io as the source of dropped
connections now describes the spooling leg without it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
a_timeout_names_the_cause_beneath_reqwest bound 127.0.0.1:0 and then
connected to whatever port the OS chose. A sandbox that allows loopback
ports one by one, as Landlock does, can list a fixed port but not a random
one, so the test could not pass in such a sandbox even with binding allowed.

It now listens on 127.0.0.1:29871: below Linux's ephemeral range, so an
outgoing connection does not take it by chance, and bound by this helper
only. A failed bind names the port and says it may be in use or not allowed
by the sandbox.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Picks up core's exact numeric -> SQL_C_INTERVAL_* conversion and the 01S07
for a non-zero interval-text digit past the ninth. No driver code changes.
Still a temporary commit pin until a core release is tagged; the comment
now names the branch head rather than one commit's subject, so it does not
go stale with the next bump.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Replaces the temporary commit pin with the released tag. v0.1.1 is the
squash-merged core work this branch was tested against (53e5a89 plus the
version bump and changelog), so the dependency's code is unchanged; the
lockfile records core 0.1.1 at tag v0.1.1 (4f5e802).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ODBC Driver Manager tracing writes every call to a file. On the test VM
2,000 rows of tpcds.sf1.customer took 1.3-1.7 s with it off and 42-172 s
with it on, slow enough for Trino to abandon a long read
(ABANDONED_QUERY). Left on after a diagnosis, it failed the spooling
suite and passed for a driver problem.

windows_test.py now checks the three places Windows keeps the switch
(current user, machine, 32-bit Driver Manager) right after connecting and
stops when any is on, naming the key and its trace file.
--allow-dm-trace continues with a warning, for runs where tracing is the
point. WINDOWS.md documents both; the README's troubleshooting section
explains ABANDONED_QUERY / "Query not found" on large reads and tracing
as a measured cause.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gen-trino-config.sh removed generated/trino and created it again. A
running coordinator bind-mounts that directory, so after a re-run the
container held the deleted one: /etc/trino was empty, Trino kept the
settings it had read at its last start, and a restart would have found no
config at all. setup.sh only recreated Trino when the profiles changed, so
a changed fragment did not reach a running stack either.

The generator now empties the directory in place, and setup.sh exports a
hash of the assembled config that compose.yaml sets as a label on the
trino service: a changed config is a changed service definition, which
compose recreates. An unchanged re-run leaves Trino running.

Checked against the live stack: after a re-run /etc/trino keeps its 6
entries (was empty); a fragment change recreates Trino (was not); an
unchanged re-run keeps the container; switching to the spooling profile
and the full integration run still pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
WINDOWS.md, the README's troubleshooting entry and check_dm_tracing's doc
comment quoted timings from one test VM, which say nothing about anyone
else's setup. All three now describe the effect qualitatively: tracing
writes every ODBC call to a file and slows reads down enough for Trino to
abandon a long one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
For a timestamp with time zone column, Power BI sends RangeStart and
RangeEnd as plain TIMESTAMP literals, which Trino interprets in the
session time zone. Partition boundaries therefore move if TimeZone
changes on a dataset that already has partitions.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@maltesander maltesander left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@adwk67
adwk67 merged commit 798f358 into main Oct 9, 2026
9 checks passed
@adwk67
adwk67 deleted the fix/review-feedback branch October 9, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants