Conversation
odata_describe() mapped the EDM type NAME alone, which is blind to everything the reader is not. An Edm.Decimal was described as "DECIMAL" whatever its Precision/Scale facets said, so a column the reader binds as DECIMAL(38,18) -- or as DOUBLE, when no Scale is declared -- was described as neither. A Collection(Edm.String) and a complex type were both described as "VARCHAR", where the reader binds VARCHAR[] and a STRUCT. The one function whose entire job is to answer "what will this column be?" was therefore free to disagree with the answer. It now goes through the same property-aware mapping the reader uses. The reported facets had the matching defect: an absent Precision/Scale/MaxLength was reported as 0, indistinguishable from a declared zero -- and Scale="0" against an absent Scale is exactly what decides DECIMAL(p,0) against DOUBLE. An undeclared facet is now NULL. Verified against the live Northwind V4 service, whose UnitPrice declares no facets: describe said DECIMAL while odata_read bound DOUBLE; both now say DOUBLE. Follow-up to #254.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #254, stacked on #255 (this PR targets that branch, so the diff shows only the describe change; retarget to
mainonce #255 merges).The defect
odata_describe()mapped the EDM type name alone, through the facet-blindConvertEdmTypeStringToDuckDbTypeString. That is blind to everything the reader is not:Edm.Decimal Scale="18"DECIMAL(38,18)DECIMALEdm.Decimal Precision="19"(no Scale)DOUBLEDECIMALCollection(Edm.String)VARCHAR[]VARCHARSTRUCT(...)VARCHARSo the one function whose entire job is to answer "what will this column be?" was free to disagree with the answer. It now goes through
BuildLogicalTypeForProperty— the same mapping the reader uses.Facets
The reported
precision/scale/max_lengthhad the matching defect: an absent facet was reported as0, indistinguishable from a declared zero.Scale="0"against an absentScaleis exactly what decidesDECIMAL(p,0)againstDOUBLE, so the field a caller would consult to understand the type could not express the distinction. An undeclared facet is nowNULL.This is a visible change for anyone reading those three fields — worth noting in the release summary.
Verification
Live, against the public Northwind V4 service (its
UnitPricedeclares no facets):New
test/cpp/test_odata_describe_types.cppcovers all four shapes plus the absent-vs-zero facet distinction against a local server — there was no C++ coverage ofodata_describeat all before. The livetest/sql/odata_describe.teststill passes unchanged, as does the full suite (3189 assertions / 583 cases).🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.