dbt doesn't properly return error message on wrong syntax

[dataders]

re-phrase of: #135

using dbt-sqlserver==0.19.1, if you make a syntax error in a model, the dbt CLI does not return the error message like it used to in dbt-sqlserver<0.19.1.

This has me stumped so I'm tagging all the folks who know more about TSQL than me: @NandanHegde15 @mikaelene @semcha @panasenco

steps to reproduce

1. configure a target of type=sqlserver
2. install dbt-sqlserver==0.19.1
3. create a model (repro.sql) with intentionally wrong sql

   -- models/repro.sql
   -- notice the 'x' after "SELECT"
   SELECTx 1 as col_name

4. run the new model dbt run -m repro

   (dbt) anders.swanson@AMAC02FG0TMMD6R SalesOps % dbt run -m repro  
   Running with dbt=0.19.1
   Found 21 models, 46 tests, 0 snapshots, 0 analyses, 712 macros, 0 operations, 1 seed file, 24 sources, 1 exposure
   
   16:07:15 | Concurrency: 1 threads (target='dev')
   16:07:15 | 
   16:07:15 | 1 of 1 START view model ajs.repro.................................... [RUN]
   16:07:16 | 1 of 1 OK created view model ajs.repro............................... [OK in 1.27s]
   16:07:16 | 
   16:07:16 | Finished running 1 view model in 8.44s.
   
   Completed successfully
   
   Done. PASS=1 WARN=0 ERROR=0 SKIP=0 TOTAL=1
   

more info

in the full extract of the dbt log you can see this query batch get executed

USE [dbt-msft-serverless-db];
execute(
    'create view "test"."repro__dbt_tmp" as
    SELECTx 1 as col_name'
);

Executing the above query batch directly against the database provides the following error message, that isn't returned by dbt.

Msg 102, Level 15, State 1, Procedure demo__dbt_tmp, Line 2
Incorrect syntax near 'SELECTx'.

Interestingly enough, looking at the db's logs, it does show the error/failure.

<batch_information><failure_reason>Err 102, Level 15, Server dbt-msft-server
Incorrect syntax near 'SELECTx'.</failure_reason></batch_information>

original motivation for change

The whole reason for this change is that if you try the below query, the error message, is 'CREATE VIEW' must be the first statement in a query batch. This is why the CREATE VIEW was wrapped in an EXEC.

USE [dbt-msft-serverless-db]

CREATE VIEW dbo.view_test AS (
    SELECT 1 as col_name
)

version info

dbt --version
installed version: 0.19.1
   latest version: 0.19.1

Up to date!

Plugins:
  - sqlserver: 0.19.1
  - synapse: 0.19.1

[mikaelene]

I think it used to return the error. At least that it was an error. Don't know when this bug was introduced. Does anyone know?

There has been a lot of changes that I have not looked into really. This looks suspicious:

dbt-sqlserver/dbt/adapters/sqlserver/connections.py

Lines 367 to 382 in 679b351

	def get_response(cls, cursor) -> AdapterResponse:
	#message = str(cursor.statusmessage)
	message = 'OK'
	rows = cursor.rowcount
	#status_message_parts = message.split() if message is not None else []
	#status_messsage_strings = [
	# part
	# for part in status_message_parts
	# if not part.isdigit()
	#]
	#code = ' '.join(status_messsage_strings)
	return AdapterResponse(
	_message=message,
	#code=code,
	rows_affected=rows
	)

I don't know how this is used, but reading from the code it would always return 'OK'.

[dataders]

Don't know when this bug was introduced. Does anyone know?

It was introduced in #126 which was part of the latest release which a @SportsTribution confirms in #135 that it happens in the v0.19.1 release but not before. I was able to repro the same.

[dataders]

This looks suspicious:

dbt-sqlserver/dbt/adapters/sqlserver/connections.py

Lines 367 to 382 in 679b351

	def get_response(cls, cursor) -> AdapterResponse:
	#message = str(cursor.statusmessage)
	message = 'OK'
	rows = cursor.rowcount
	#status_message_parts = message.split() if message is not None else []
	#status_messsage_strings = [
	# part
	# for part in status_message_parts
	# if not part.isdigit()
	#]
	#code = ' '.join(status_messsage_strings)
	return AdapterResponse(
	_message=message,
	#code=code,
	rows_affected=rows
	)

@mikaelene that really does look sus. Older than I thought #81. I'll play w reverting that change.

[mikaelene]

Looks like I am to blame here :-). Guess I just wanted it to work and never finished the last mile. Sorry. I'll see if I can have a look as well.

[jeroen-mostert]

I'll just leave this commit here because I was unsure if it was clean enough for a PR (or if it belongs on this level, or further upstream in dbt itself), but I encountered this particular problem when we homebrewed support for multiple DBs -- the root of the issue (at least with our change) was that you must process all result sets explicitly if a batch has more than one statement, otherwise you won't get error notifications for anything but the first statement. I have not verified if this is the exact same issue or if my change fixes it, but it does look awfully familiar.

[larsjohanssonblank]

Hi folks,
upgraded to dbt 0.19.1 and dbt-sqlserver 0.19.1 and I got these kind of errors.
SQL Server 2017 on-prem.
dbt execution tells me everything is ok:
09:33:04 | 1 of 1 START table model staging.revenue_monthly_1062................ [RUN]
09:33:05 | 1 of 1 OK created table model staging.revenue_monthly_1062........... [OK in 0.25s]
But no table is created, and when I look into the dbt generated model code it has an error (the generated SQL is not correct, due to variables and macros and a pair of forgotten '' around a variable value )
date_key >= CAST(2019-04-01 AS DATE) is not correct, it should be
date_key >= CAST('2019-04-01' AS DATE)
But thats not the real issue here, that error is easily fixed.
The main thing is that the model runs "ok" (but silently not runs ok) causing a lot of downstream errors.

[mikaelene]

Hej @larsjohanssonblank! This is a known issue. I think it is solved in branch cursor_conundrum. Can you try that branch?

@swanderz do yo know what is failing in the integration tests in that branch for azure-sql?

[larsjohanssonblank]

I'll try later today (I'll hope), thanks @mikaelene

[dataders]

@mikaelene it was just an inane error about the Azure CLI not being able to installed. It completed now, so I'm going to merge it.