Cursor.description collapses TIMESTAMP_NTZ to 'timestamp' on the SELECT path (#786) - #893
Cursor.description collapses TIMESTAMP_NTZ to 'timestamp' on the SELECT path (#786)#893peco-engineer-bot[bot] wants to merge 1 commit into
Conversation
Sorry, something went wrong.
…CT path (#786) Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
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:
Files changed
Test plan
🤖 Generated by engineer-bot (bug-fix flow) — review before merge.