Skip to content

fix(describe): keep the query's own error instead of retrying it blind - #845

Open
Benjamin-Knight wants to merge 1 commit into
dbt-msft:masterfrom
Benjamin-Knight:fix/describe-fallback-masks-query-error
Open

Benjamin-Knight wants to merge 1 commit into
dbt-msft:masterfrom
Benjamin-Knight:fix/describe-fallback-masks-query-error

Conversation

@Benjamin-Knight

Copy link
Copy Markdown
Collaborator

_describe_result_set treated every describe failure as the procedure declining
to describe a good query, and fell back to executing.

Classify instead: sp_describe_first_result_set reports what it cannot describe
with its own 11500-11599 family (11514 #temp, 11526 uncompilable), which every
backend carries through in the message text. Those keep the fallback. Anything
else is raised — executing raises it identically, and no fallback can produce
column metadata for a query that does not compile.

Fixes #844 .

Tests

Unit coverage for both directions in test_get_column_schema_from_query.py
(207/208 raise without a second add_select_query; 11514/11526 still execute).
test_describe_error_surfaces.py builds a contract-enforced CTE model naming a
column its upstream lacks and asserts the output carries the column name and not
closed connection — on master that test reports Cannot create cursor on closed connection.

A CTE-headed query has its column shape read by
sp_describe_first_result_set rather than by executing it, and that describe
compiles the query -- so a model selecting a column an upstream table does
not have fails there, with Msg 207.

Every describe failure was treated as the procedure declining to describe a
good query and fell back to executing it. Handling the error has already
closed the connection, so the fallback failed on that instead and the
transport-level message replaced the one naming the column. The real error
survived only at debug level, which is not where an on-call reader is
looking, and retrying could not help because a missing column is not
transient.

The describe's own "metadata could not be determined" errors -- the
11500-11599 family, a #temp table it cannot see through or a statement it
cannot compile -- still fall back to executing. Anything else is raised:
executing raises it identically, and no fallback can produce column
metadata for a query that does not compile.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@axellpadilla

Copy link
Copy Markdown
Collaborator

The 207/208 propagation looks correct, but I think the 115xx fallback still has a connection lifecycle issue: add_select_query() hits exception_handler(), which calls release() before re-raising. So after a real 11514/11526, _describe_result_set() returns None and the fallback immediately calls add_select_query(sql) again on the same now-closed thread connection.

The current unit test mocks add_select_query() directly, so it skips that lifecycle. Could we add an integration test with a real describe failure, for example a query where EXEC sys.sp_describe_first_result_set N'...' returns 11514/11526, and verify the fallback actually succeeds?

Also, sys.dm_exec_describe_first_result_set instead of calling sp_describe_first_result_set exposes structured error_number/error_type metadata and could avoid both the connection-release problem and parsing str(e) with a regex.

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.

Invalid column name surfaces as "Attempt to use a closed connection"

2 participants