Empty unit test fixtures (rows: []) emit invalid LIMIT 0 syntax#699
Empty unit test fixtures (rows: []) emit invalid LIMIT 0 syntax#699Benjamin-Knight wants to merge 5 commits into
Conversation
Chore/version v1.10.0rc1
…698) dbt-core's get_fixture_sql/get_expected_sql hardcode `limit 0` for empty fixture rows, which is not valid T-SQL. These macros are not dispatched, so shadow them in the adapter package and emit `select top 0` instead. get_expected_sql's empty branch builds typed nulls rather than selecting from dbt_internal_unit_test_actual, which is out of scope inside the view created by sqlserver__get_unit_test_sql. Also fix sqlserver__get_columns_in_query for queries starting with a CTE (hit by unit tests with an empty `expect` block): such queries cannot be wrapped in a subquery, so describe their result set via sp_describe_first_result_set instead of executing them. Non-CTE queries keep the existing TOP 0 wrapping. The CTE detection is factored into a shared sqlserver__select_starts_with_cte helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Please base your changes from master if aiming v1.10.1 to avoid the temporal rc1 change I made only into the release branch
There was a problem hiding this comment.
Think I had already done this?
There was a problem hiding this comment.
I don't see any other solution than shadowing, that said, inside the shadowed macro we could introduce a call to macro sqlserver__get_fixture_sql(...) instead. This would keep the SQL Server-specific logic isolated and make the implementation closer to what we would want upstream if they use adapter calls later.
This is not that important but ideally with a link to an upstream dbt-core/dbt-adapters issue requesting dispatch support. If upstream adds that dispatch point later, our future patch should mostly be removing the shadow and keeping only the SQL Server-specific sqlserver__get_fixture_sql implementation.
2 participants
Fixes #698
dbt-core hardcodes
limit 0for empty fixture rows, which is not valid T-SQL. These macros are not dispatched, so shadow them in the adapter package and emitselect top 0instead.Also fix sqlserver__get_columns_in_query for queries starting with a CTE (hit by unit tests with an empty
expectblock): such queries cannot be wrapped in a subquery, so describe their result set via sp_describe_first_result_set instead of executing them. Non-CTE queries keep the existing TOP 0 wrapping. The CTE detection is factored into a shared sqlserver__select_starts_with_cte helper.