Skip to content

fix: widen Edm.Decimal facets that leave no integral digits - #255

Merged
jrosskopf merged 1 commit into
mainfrom
fix/254-decimal-facets
Sep 24, 2026
Merged

jrosskopf merged 1 commit into
mainfrom
fix/254-decimal-facets

Conversation

@jrosskopf

@jrosskopf jrosskopf commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #254.

The defect

A TOPdesk reporting service publishes every money column as Edm.Decimal with a declared Scale and no Precision. An absent Precision is unbounded in CSDL, but BuildDecimalLogicalType defaulted it to 18, so Scale="18" produced DECIMAL(18,18) — eighteen fractional digits with no room for a single integral one. Every value of 1 or more then failed to cast and came back NULL:

erpl_web warning: 2 values in 1 column could not be converted and were returned as NULL (odata_read).
  column 'serviceagreementcost': 2 failures; first offending value: 490.0; first error:
  Could not cast value 490.000000 to DECIMAL(18,18)

The same degenerate type also came out of a declared Precision <= Scale, because the scale was truncated down to the precision rather than the precision widened. Both shapes are fixed, so the fix does not depend on which one the service emits.

The rule

CSDL facets before after
Scale absent or "variable" DOUBLE DOUBLE (unchanged, #80)
Scale="18", no Precision DECIMAL(18,18) DECIMAL(38,18)
Precision="18" Scale="18" DECIMAL(18,18) DECIMAL(38,18)
Precision="5" Scale="10" DECIMAL(5,5) DECIMAL(38,10)
Precision="13" Scale="3" DECIMAL(13,3) unchanged
Precision="47" Scale="2" DECIMAL(38,2) unchanged
Precision="19" Scale="0" DECIMAL(19,0) unchanged
Scale >= 38 DECIMAL(38,38) DOUBLE

No mapping that works today is narrowed; only the degenerate ones widen, and only a scale too wide for any DuckDB DECIMAL falls back to DOUBLE.

Tests

  • test/cpp/test_odata_edm.cpp — the facet matrix above, including a tinyxml2 parse of the reported CSDL shape.
  • test/cpp/test_odata_server_e2e.cpp — an end-to-end case against ODataTestServer: real metadata parsing, real catalog, real JSON conversion, only the remote service stubbed. It reproduces the report through both ATTACH ... (TYPE odata) (the reporter's own SELECT DISTINCT) and odata_read(), and before the fix it failed with the exact warning quoted above.

erpl_web_tests: 3146 assertions in 582 cases, all green. SQLLogicTests: all green.

Docs are updated in a companion PR on the landing page (Type Mapping + a Troubleshooting entry).

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

A TOPdesk reporting service publishes every money column as Edm.Decimal with a
declared Scale and no Precision. An absent Precision is unbounded in CSDL, but
BuildDecimalLogicalType defaulted it to 18, so Scale="18" produced
DECIMAL(18,18) -- eighteen fractional digits with no room for a single integral
one. Every value of 1 or more then failed to cast and was returned as NULL with
a conversion warning.

The same degenerate type came out of a declared Precision <= Scale, because the
scale was truncated down to the precision rather than the precision widened.

Both now widen to DuckDB's maximum of 38, keeping the scale the service declared
about its own data. A scale too wide for any DuckDB DECIMAL falls back to DOUBLE,
as an absent or "variable" scale already did. No mapping that works today is
narrowed.

Covered by a unit case over the facet matrix and by an end-to-end case that
reproduces the report through ATTACH and odata_read against a local server, with
only the remote service stubbed.

Fixes #254
@jrosskopf
jrosskopf merged commit 5a8b89c into main Sep 24, 2026
73 checks passed
@jrosskopf
jrosskopf deleted the fix/254-decimal-facets branch September 24, 2026 04:01
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.

Wrong type inference for decimal type in ODATA source

1 participant