Skip to content

Fix ODBC conformance for numeric types (1.3)#183

Merged
staticlibs merged 1 commit intoduckdb:v1.3-ossivalisfrom
staticlibs:decimals_info_13
Jul 3, 2025
Merged

Fix ODBC conformance for numeric types (1.3)#183
staticlibs merged 1 commit intoduckdb:v1.3-ossivalisfrom
staticlibs:decimals_info_13

Conversation

@staticlibs
Copy link
Collaborator

This is a backport of the PR #177 to v1.3-ossivalis stable branch.

This change addresses multiple ODBC specification conformance issues discovered during a thorough audit of GetColumnSize() and GetDisplaySize() implementations.

Issues fixed:

  1. DECIMAL/NUMERIC precision and scale reporting

    • SQLColAttribute/SQLDescribeCol incorrectly reported column sizes
    • Fixed GetColumnSize() to return precision instead of precision+scale
  2. Signed/Unsigned integer display sizes

    • GetDisplaySize() now correctly distinguishes signed vs unsigned types
    • UTINYINT: 3 chars, USMALLINT: 5 chars, UINTEGER: 10 chars, UBIGINT: 20 chars
    • TINYINT: 4 chars, SMALLINT: 6 chars, INTEGER: 11 chars, BIGINT: 20 chars
  3. TIME/TIMESTAMP fractional seconds precision

    • Added GetTemporalPrecision() to detect precision from LogicalTypeId
    • Added missing type mappings for TIMESTAMP_MS, TIMESTAMP_SEC, TIMESTAMP_NS
    • Fixed GetColumnSize() to return full column size per ODBC spec:
    • TIME: 8 (base) or 9 + precision
    • TIMESTAMP: 19 (base) or 20 + precision
    • Fixed GetDisplaySize() to include fractional seconds in display size
    • Updated FillIRD() and SQLDescribeCol to set precision/scale for temporal types

Implementation details:

  • include/api_info.hpp: Added GetTemporalPrecision(), updated size calculations
  • src/api_info.cpp: Added missing timestamp variant type mappings
  • src/common/duckdb_odbc.cpp: Set temporal precision/scale in IRD
  • src/prepared.cpp: Handle temporal types in SQLDescribeCol
  • test/: Added comprehensive test coverage for all fixes

All changes verified against DuckDB behavior and ODBC specification. All 79 tests passing with new test coverage added.

This is a backport of the PR duckdb#177 to `v1.3-ossivalis` stable branch.

This change addresses multiple ODBC specification conformance issues discovered
during a thorough audit of GetColumnSize() and GetDisplaySize() implementations.

Issues fixed:
1. DECIMAL/NUMERIC precision and scale reporting
    - SQLColAttribute/SQLDescribeCol incorrectly reported column sizes
    - Fixed GetColumnSize() to return precision instead of precision+scale

2. Signed/Unsigned integer display sizes
    - GetDisplaySize() now correctly distinguishes signed vs unsigned types
    - UTINYINT: 3 chars, USMALLINT: 5 chars, UINTEGER: 10 chars, UBIGINT: 20 chars
    - TINYINT: 4 chars, SMALLINT: 6 chars, INTEGER: 11 chars, BIGINT: 20 chars

3. TIME/TIMESTAMP fractional seconds precision
    - Added GetTemporalPrecision() to detect precision from LogicalTypeId
    - Added missing type mappings for TIMESTAMP_MS, TIMESTAMP_SEC, TIMESTAMP_NS
    - Fixed GetColumnSize() to return full column size per ODBC spec:
    * TIME: 8 (base) or 9 + precision
    * TIMESTAMP: 19 (base) or 20 + precision
    - Fixed GetDisplaySize() to include fractional seconds in display size
    - Updated FillIRD() and SQLDescribeCol to set precision/scale for temporal types

Implementation details:
- include/api_info.hpp: Added GetTemporalPrecision(), updated size calculations
- src/api_info.cpp: Added missing timestamp variant type mappings
- src/common/duckdb_odbc.cpp: Set temporal precision/scale in IRD
- src/prepared.cpp: Handle temporal types in SQLDescribeCol
- test/: Added comprehensive test coverage for all fixes

All changes verified against DuckDB behavior and ODBC specification.
All 79 tests passing with new test coverage added.
@staticlibs staticlibs merged commit 34a5b40 into duckdb:v1.3-ossivalis Jul 3, 2025
@staticlibs staticlibs deleted the decimals_info_13 branch July 3, 2025 17: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.

2 participants