Skip to content

Cursor.description collapses TIMESTAMP_NTZ to 'timestamp' on the SELECT path (#786) - #893

Open
peco-engineer-bot[bot] wants to merge 1 commit into
mainfrom
ai/issue-786
Open

Cursor.description collapses TIMESTAMP_NTZ to 'timestamp' on the SELECT path (#786)#893
peco-engineer-bot[bot] wants to merge 1 commit into
mainfrom
ai/issue-786

Conversation

@peco-engineer-bot

Copy link
Copy Markdown
Contributor

Summary

Automated fix for #786 — Cursor.description collapses TIMESTAMP_NTZ to 'timestamp' on the SELECT path.

Extended the existing Spark:DataType:SqlName Arrow-metadata override in _col_to_description (thrift_backend.py) to map b"TIMESTAMP_NTZ" to type_code "timestamp_ntz", recovering the distinction that Thrift collapses (both TIMESTAMP and TIMESTAMP_NTZ arrive as TTypeId.TIMESTAMP_TYPE); also taught convert_to_assigned_datatypes_in_column_table (utils.py) to parse the new "timestamp_ntz" type_code like "timestamp". Verified via a live-warehouse E2E test (description[1][1] == 'timestamp_ntz', description[0][1] == 'timestamp') plus two unit tests; full unit suite (82 tests) stays green.

Root cause & plan

Root cause: In src/databricks/sql/backend/thrift_backend.py, _col_to_description derives the DB-API type_code from the Thrift TTypeId enum name. Spark TIMESTAMP and TIMESTAMP_NTZ both arrive over the wire as TTypeId.TIMESTAMP_TYPE, so after stripping _TYPE and lowercasing both become 'timestamp'. The function already has a Spark:DataType:SqlName Arrow-metadata override hook (added by PR #560) that recovers VARIANT, but it does not handle TIMESTAMP_NTZ, so the _NTZ distinction is lost on the SELECT path.
Files: src/databricks/sql/backend/thrift_backend.py, tests/e2e/test_driver.py, tests/unit/test_util.py
Planned coverage:

  • E2E against the live warehouse: SELECT with CAST('...' AS TIMESTAMP) AS tz_aware and CAST('...' AS TIMESTAMP_NTZ) AS tz_naive; assert cursor.description[0][1] == 'timestamp' and cursor.description[1][1] == 'timestamp_ntz'. Fails today because both report 'timestamp'. Ground truth: issue's stated expectation and the DESCRIBE/SQLAlchemy reflection map where 'timestamp_ntz' is a valid key. (TIMESTAMP_NTZ collapsed to 'timestamp' in cursor.description on the SELECT path)

Files changed

  • tests/e2e/test_driver.py
  • src/databricks/sql/backend/thrift_backend.py
  • src/databricks/sql/utils.py
  • tests/unit/test_util.py
  • tests/unit/test_thrift_backend.py

Test plan

  • tests/e2e/test_driver.py::TestPySQLCoreSuite::test_timestamp_ntz_description_type_code — fails (red) against the original code, passes (green) after the fix
  • tests/unit/test_thrift_backend.py::ThriftBackendTestSuite::test_col_to_description — fails (red) against the original code, passes (green) after the fix
  • tests/unit/test_util.py::TestUtils::test_convert_to_assigned_datatypes_in_column_table — fails (red) against the original code, passes (green) after the fix

🤖 Generated by engineer-bot (bug-fix flow) — review before merge.

…CT path (#786)

Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com>
@peco-engineer-bot peco-engineer-bot Bot added the engineer-bot Maintainer-applied gate: triggers engineer-bot (bug-fix on issue / take-over on PR). label Aug 5, 2026

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: 1 Medium · 1 Low

Targeted, well-tested fix that correctly recovers the TIMESTAMP_NTZ distinction on the Thrift SELECT path via the existing Arrow-metadata override. One medium concern: the PEP-249 DATETIME type object still only matches "timestamp", so NTZ columns silently stop comparing equal to DATETIME — a contract regression worth addressing. Also flagged a low cross-backend consistency gap on the SEA conversion path.

Other findings

  • 🟡 Medium — The PEP-249 DATETIME type object is not updated to include the new "timestamp_ntz" type_code, so this change is a subtle behavioral regression for the DATETIME comparison.

DBAPITypeObject.__eq__ matches only the exact strings passed at construction, and DATETIME is constructed with just "timestamp". Before this PR a TIMESTAMP_NTZ column collapsed to type_code "timestamp", so cursor.description[i][1] == databricks.sql.DATETIME returned True. After this PR the type_code becomes "timestamp_ntz", so that comparison now returns False for NTZ columns — even though PEP-249 intends DATETIME to cover all datetime-family types. Downstream consumers (e.g. SQLAlchemy/pandas adapters) that test col_type == DATETIME will silently stop recognizing NTZ columns as datetimes.

Consider extending DATETIME to also match "timestamp_ntz", and adding an assertion to the E2E test that description[1][1] == DATETIME still holds.

  • 🔵 Low — The fix teaches the Thrift/column path (convert_to_assigned_datatypes_in_column_table, utils.py:774) to parse "timestamp_ntz" like "timestamp", but the parallel SEA inline conversion path is not updated. SqlTypeConverter.TYPE_MAPPING has a key for SqlType.TIMESTAMP ("timestamp") but none for "timestamp_ntz"; convert_value returns the raw string unchanged when the type_code is missing from the mapping. If the SEA backend ever surfaces a "timestamp_ntz" type_code, NTZ values would be returned as un-parsed strings while the Thrift path returns datetime objects — a cross-backend inconsistency. Worth confirming SEA does not (or, if it does, adding the mapping) to keep the two backends aligned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engineer-bot Maintainer-applied gate: triggers engineer-bot (bug-fix on issue / take-over on PR).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants