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

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.

This branch was previously deployed

1 inactive deployment
azure-prod 46c6ce0c Deployed Aug 5, 2026 by peco-engineer-bot[bot] via test-with-coverage #606
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