JDBC - Clause Output and Exception

[DVD27]

Driver version or jar name

Version 6.4

SQL Server version

Microsoft SQL Server 2016 (SP1-CU6) (KB4037354) - 13.0.4457.0 (X64)
Nov 8 2017 17:32:23
Copyright (c) Microsoft Corporation
Developer Edition (64-bit) on Windows Server 2016 Standard 10.0 (Build 14393: ) (Hypervisor)

Client operating system

Windows 7

Java/JVM version

java version "1.8.0_144"
Java(TM) SE Runtime Environment (build 1.8.0_144-b01)
Java HotSpot(TM) 64-Bit Server VM (build 25.144-b01, mixed mode)

Table schema

DROP TABLE IF EXISTS TEST;

CREATE TABLE TEST (
  ID INT IDENTITY NOT NULL,
  FIELD1 VARCHAR(255) NOT NULL,
  FIELD2 VARCHAR(255) NOT NULL
);

CREATE OR ALTER PROCEDURE [dbo].proc_insert_masse_TEST @json NVARCHAR(MAX)
AS 
BEGIN TRANSACTION
BEGIN TRY
	SET NOCOUNT ON;

	MERGE INTO TEST AS target
	USING (
		SELECT *
		FROM OPENJSON(@json)
		WITH (
			FIELD1 VARCHAR(255) 'strict $.FIELD1'
		)
	) AS src 
	ON (1 = 0)
	WHEN NOT MATCHED THEN
		INSERT (FIELD1)
		VALUES (src.FIELD1)
	OUTPUT inserted.ID;

	COMMIT TRANSACTION;
END TRY
BEGIN CATCH
	DECLARE @ErrorMessage NVARCHAR(4000) = ERROR_MESSAGE();  
	ROLLBACK TRANSACTION;
	RAISERROR('Error occured during the insert : %s' , 16, 1, @ErrorMessage);
END CATCH;

Problem description

The JDBC driver return an empty result when an error occured.

Expected behavior and actual behavior

When we use this procedure, we should have an error because the column FIELD2 cannot be null.

Repro code

-- Example 1 : should say that the column FIELD2 cannot be null
EXECUTE [dbo].proc_insert_masse_TEST N'[{"FIELD1" : "TEST"}]';
-- Example 2 : Should say that one field is absent
EXECUTE [dbo].proc_insert_masse_TEST N'[{}]';

Note : When we use this example in MSSQL Management tools we have the excepted result, this is why we think that is an issue.

[rene-ye]

Hi @DVD27, we have been able to reproduce the issue and are currently working on a fix. It is worth noting that the Exception does not occur when the statement is executed, but when the ResultSet is accessed. If this doesn't sound right, please feel free to follow up on the issue and describe the desired behavior.

[rene-ye]

Hi @DVD27, PR #664 was recently created to address this issue. Feel free to test the changes and get back to us with any problems or concerns. I'll be attaching jars below this post, or you can build it from the branch in the PR.

[rene-ye]

mssql-jdbc-issue659-jars.zip
Attaching Jars for testing.

[DVD27]

Now I have the error when I retrieve the resultset of my request but... when I try to do an other request before on the same connection I have this error :

SQL Error [3971] [S0001]: The server failed to resume the transaction. Desc:4500000001.

For exemple I execute this request :

EXECUTE [dbo].proc_insert_masse_TEST N'[{"FIELD1" : "TEST"}]';

And after this :

EXECUTE [dbo].proc_insert_masse_TEST N'[{}]';

[rene-ye]

Hi @DVD27, I believe this is happening because you are using transactions in the procedure. Please have a look at this post which explains why the error is occurring. I removed the transaction related T-SQL code from the repro above and it does not cause this error.

[DVD27]

Ok, it's possible I have do some error in my code, I will see later.
However, I read your code and I see this :

if (rowCount == BEFORE_FIRST_ROW && columns.length == 1) {
    lookForErrors();
}

But this doesn't work at all when we use this procedure :

CREATE OR ALTER PROCEDURE [dbo].proc_insert_masse_TEST @json NVARCHAR(MAX)
AS
BEGIN TRANSACTION
BEGIN TRY
SET NOCOUNT ON;

MERGE INTO TEST AS target
USING (
	SELECT *
	FROM OPENJSON(@json)
	WITH (
		FIELD1 VARCHAR(255) 'strict $.FIELD1'
	)
) AS src 
ON (1 = 0)
WHEN NOT MATCHED THEN
	INSERT (FIELD1)
	VALUES (src.FIELD1)
OUTPUT inserted.ID, inserted.FIELD1;

COMMIT TRANSACTION;

END TRY
BEGIN CATCH
DECLARE @errorMessage NVARCHAR(4000) = ERROR_MESSAGE();
ROLLBACK TRANSACTION;
RAISERROR('Error occured during the insert : %s' , 16, 1, @errorMessage);
END CATCH;

Because now we have 2 columns, and we could imagine have n columns.

We have an other problem when we use also this proecdure :

CREATE OR ALTER PROCEDURE [dbo].proc_insert_masse_TEST @json NVARCHAR(MAX)
AS
BEGIN TRANSACTION
BEGIN TRY
SET NOCOUNT ON;

MERGE INTO TEST AS target
USING (
	SELECT *
	FROM OPENJSON(@json)
	WITH (
		FIELD1 VARCHAR(255) 'strict $.FIELD1',
		FIELD2 VARCHAR(255) 'lax $.FIELD2'
	)
) AS src 
ON (1 = 0)
WHEN NOT MATCHED THEN
	INSERT (FIELD1, FIELD2)
	VALUES (src.FIELD1, src.FIELD2)
OUTPUT inserted.ID;

COMMIT TRANSACTION;

END TRY
BEGIN CATCH
DECLARE @errorMessage NVARCHAR(4000) = ERROR_MESSAGE();
ROLLBACK TRANSACTION;
RAISERROR('Error occured during the insert : %s' , 16, 1, @errorMessage);
END CATCH;

If we execute this code :
EXECUTE [dbo].proc_insert_masse_TEST N'[{"FIELD1" : "TEST", "FIELD2" : "TEST"}, {"FIELD1" : "TEST"}]';
We should have an error but... we have one value returned and no data inserted (because we rollback and throw an error) and also no error.

Finally, we think that should be preferable to have the error when we execute the statement but if I understand correctly your code it's seem difficult to do this.

[rene-ye]

Hi @DVD27, the column/row check condition was a workaround to filter out bizarre table names which contained the error token. I see now that it's not the way to go and will get back to you in the near future with a more robust fix which will address these new cases.

[rene-ye]

Hi @DVD27, I've updated the PR with changes. I'll attach jars for testing below.
issue659-jars-for-testing.zip

[DVD27]

It's better.

First, I doesn't have anymore this error : "The server failed to resume the transaction. Desc:4500000001."
For all my tests, all passed, I have always the excepted result.

The only thing which can be strange it's to have the error during the retrieve of result.
However, if we use the clause OUTPUT it's for retrieve result so... could be good in many case.

Thank you very much for you work and your speed anwser.

[cheenamalhotra]

Closing issue since PR is merged and will be part of 6.5.1 preview release.

[ulvii]

Hi @DVD27,

Please be advised that we are about to revert #664, which might also require you to make changes in your application. We have included a test in #1003 that verifies the scenario mentioned by you. Please let us know if you have any questions or need help with updating your application.