Skip to content

Comments

Fix physicalType accessibility in sql_type_converter#1052

Open
jochenchrist wants to merge 2 commits intomainfrom
claude/analyze-issue-1039-EV18N
Open

Fix physicalType accessibility in sql_type_converter#1052
jochenchrist wants to merge 2 commits intomainfrom
claude/analyze-issue-1039-EV18N

Conversation

@jochenchrist
Copy link
Contributor

Description

Fixes issue #1039 by properly handling SchemaProperty.physicalType in the SQL type conversion logic.

Changes

datacontract/export/sql_type_converter.py:

  • Updated _get_type() to document the preference for physicalType when available, as it carries more precision information
  • Enhanced convert_to_sql_type() to correctly handle physicalType from SchemaProperty:
    • When only physicalType is set (no logicalType), it's treated as a direct SQL type passthrough (e.g., user-specified native types like "TIMESTAMP(6)", "VARCHAR(255)")
    • When both are set, physicalType drives server-specific type mapping (preserving precision like "int" vs "long")
    • Maintains backward compatibility with customProperties["physicalType"] overrides
  • Updated convert_type_to_oracle() to:
    • Accept both SchemaProperty and FieldLike for consistency
    • Use physicalType directly for native Oracle types when set
    • Support customProperties["oracleType"] overrides
    • Fall back to logicalType mapping when physicalType is not available

tests/test_sql_type_converter_physicaltype.py (new):

  • Added comprehensive test suite with 216 lines covering:
    • Direct passthrough of physicalType when logicalType is absent
    • Server-specific type mapping when both physicalType and logicalType are set
    • Fallback to logicalType when physicalType is not set
    • Oracle-specific type handling
    • Custom property overrides for both generic and server-specific types
    • Edge cases like complex native SQL types (INTERVAL, TIMESTAMP WITH TIME ZONE)

Test Coverage

All new functionality is covered by the added test suite:

  • TestPhysicalTypeDirectPassthrough: 4 tests

  • TestPhysicalTypeWithLogicalType: 5 tests

  • TestLogicalTypeFallback: 4 tests

  • TestConvertTypeToOracle: 6 tests

  • TestServerSpecificOverrides: 3 tests

  • Tests pass

  • ruff format

  • README.md updated (if relevant)

  • CHANGELOG.md entry added

https://claude.ai/code/session_01LzYDYYGTpu2vfaGjApwR9i

claude and others added 2 commits February 13, 2026 14:49
…#1039)

The convert_to_sql_type() function was not properly accessing the
physicalType attribute on ODCS SchemaProperty objects. SchemaProperty
stores physicalType as a first-class attribute, not in customProperties,
so the _get_config_value() lookup always returned None.

Changes:
- convert_to_sql_type(): Now checks SchemaProperty.physicalType directly
  as a passthrough when logicalType is not set (indicating a user-specified
  native SQL type like "TIMESTAMP(6)" or "VARCHAR(255)")
- convert_type_to_oracle(): Updated to accept Union[SchemaProperty,
  FieldLike], added oracleType customProperty override support, and
  properly handles both ODCS and DCS field types
- Added comprehensive tests covering physicalType passthrough, logical
  type fallback, Oracle type handling, and server-specific overrides

https://claude.ai/code/session_01LzYDYYGTpu2vfaGjApwR9i
Keep generalized Union[SchemaProperty, FieldLike] signature consistent
with all other converters in the file.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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