Skip to content

A Trino decimal reads as a number, so SUM(amount) is 1234.5 instead of "1234.50" - #1414

Merged
WaylandYang merged 4 commits into
deeplethe:devfrom
rudycelekli:trino-decimal
Oct 27, 2026
Merged

WaylandYang merged 4 commits into
deeplethe:devfrom
rudycelekli:trino-decimal

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Why

Trino's client protocol writes most numbers as JSON numbers, but it writes decimal as a string. In JsonEncodingUtils, a SqlDecimal goes through generator.writeString(decimalValue.toString()) (source). The Trino engine passed data through untouched. A money column, or any SUM/AVG over one, reached the model as "1234.50", while COUNT(*) in the same row was the number 7.

The other HTTP engines already restore these values by column type: Databricks and Snowflake through coerce, ClickHouse through its cell. As the coerce comment notes, the model does arithmetic differently on "42" and 42. A Trino, Starburst, Iceberg or Hive catalog was the one place where amounts stayed quoted.

What changes

Column now also reads the type each column already carries. After the pages are collected, cells in a decimal(p,s) column go through coerce("DECIMAL", …), the same conversion Databricks and Snowflake decimals get. Nulls stay null, and other types are untouched.

How it was checked

  • A new wiremock test, a_decimal_reads_as_a_number, returns varchar(5), decimal(12,2) and bigint columns, including a null decimal. It expects {"region":"east","total":1234.5,"orders":7}.
  • cargo test -p utopia-server --bins query_engine: 73 passed.
  • With the fix reverted, the test fails with left: ["{\"region\":\"east\",\"total\":\"1234.50\",\"orders\":7}", …].
  • cargo fmt --all --check and cargo clippy -p utopia-server --all-targets -- -D warnings pass.

Before review

  • Every commit is signed off (git commit -s)
  • cargo fmt --all --check, cargo clippy and cargo test pass for the touched crate

Requested ADR follow-up

Updated ADR 0018 as requested in review. This appended revision changes documentation only; Rust and Cargo files are byte-for-byte unchanged from the prior public head. cargo fmt --all --check and git diff --check pass. No new Rust test run or live-cluster acceptance is claimed for this revision. Final-head CI remains required.

Merge after #1413, as requested; the ADR retains that exact-value dependency.

…f "1234.50"

Signed-off-by: Rudy Celekli <rudy@gradiahq.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Correct. Please add Trino decimals to ADR 0018's fourth-gate paragraph. We'll merge this after #1413 so long decimals keep their digits.

rudycelekli and others added 2 commits October 27, 2026 00:15
Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks. After #1412 landed this conflicted in ADR 0018, so I merged dev into the branch, kept both sentences, and reworded yours to the present tense now that the long-decimal guard from #1413 goes in first. Approving and merging.

@WaylandYang
WaylandYang merged commit 1142792 into deeplethe:dev Oct 27, 2026
6 checks passed
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