minus5/gofreetds

OUTPUT parameter handling is broken by PR#47

Closed this issue · 1 comments

I believe the change to OUTPUT parameter handling introduced in PR#47 is incorrect and should be reverted.

My understanding of the issue is based on the stored procedure used in the test case added as part of the patch. In pure T-SQL, the code to create this stored procedure is:

create proc test_sp_bit_type_null_output
(
	@shouldReturn bit,
	@bitValue bit=1 output
) as
	if 1 = @shouldReturn
	begin
    	set @bitValue = 1
		return 0
	end
	return -1

The patch seems to be based on the premise that the default value for the OUTPUT parameter @bitValue should be used if an input value for the parameter is not explicitly provided. The test case calls the procedure in this way, only providing a value for the first input parameter @shouldReturn:

rst, err := conn.ExecSp(spName, false)

(conn_sp_test.go line 428)

However, I believe this is a misapprehension about how OUTPUT parameters work in T-SQL. To call this procedure from T-SQL and collect the value of the OUTPUT parameter, it is always necessary to provide an input value. This can be seen when calling the procedure with @shouldReturn set to 0, which does not modify the values of @bitValue

declare @out bit --@out is created with the value NULL
exec test_sp_bit_type_null_output @shouldReturn = 0, @bitValue = @out OUTPUT
select @out AS out

The result is:

out
-----
NULL

(1 row(s) affected)

The default value for @bitValue will only be used when the parameter is not referenced in the call - in which case, the value of the OUTPUT parameter cannot be collected:

exec test_sp_bit_type_null_output @shouldReturn = 0

On this basis, I believe the patch is misguided and should be rolled back.

ianic commented

Reverted to the state before patch.
Thanks.