work-around requested for Microsoft SQL Server MSODBCSQL handling of NULL

[gjcarrette]

PHP Driver version or file name

php-sqlsrv-5.8.0

SQL Server version

Microsoft SQL Server 2017 (RTM-CU21) (KB4557397) - 14.0.3335.7
Microsoft SQL Server 2016 (SP2-CU14) (KB4564903) - 13.0.5830.85

Client operating system

CentOS Linux release 7.6.1810 (Core)
Linux 3.10.0-957.12.1.el7.x86_64 #1 SMP Mon Apr 29 14:59:59 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

PHP version

PHP 7.4.3

Microsoft ODBC Driver version

Name : msodbcsql17
Version : 17.7.1.1
Build Date : Fri 15 Jan 2021 02:56:50 PM EST

Table schema

None required to run the test.

Problem description

The Microsoft SQL Server ISNULL function seems to badly handle a NULL value transmitted to it
using SQLBindParameter with a data type of SQL_CHAR and length of 1. Because the only
purpose of PDO_SQLSRV is to interact with the Microsoft flagship relational database product
I am asking for a work-around to be implemented so that a NULL is transmitted
using SQLBindParameter with data type of SQL_VARCHAR and length of 0. This work
around has been proven with the SQLRelay open source file src/connections/odbc.cpp starting at
about line 2407 where there is extensive commentary about this issue.

This work around would speed up the full retirement of the PDO_DBLIB freetds driver at the company where I work. In order to continue using Microsoft SQL Server I can supply a patch to PDO_SQLSRV if such a patch would be accepted in principal rather than rejected as not needed due to the possibility of fixing Microsoft SQL Server instead.

Expected behavior and actual behavior

select cast(ISNULL(:K, -1) as int) as K
is expected to return -1 when K is NULL. Instead it gives
[Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Conversion failed when converting the varchar value ='*' to data type int.

Repro code or steps to reproduce

Run the attached php file with environment variables set to the database server, user credentials and database used for testing. Also attached is a stdout and an odbc trace.
env DB_SERVER=localhost DB_NAME=master DB_USER=user1 DB_PASS=pass1 php infnext-1870-sqlsrv-null-bug.php

Using the php file from this zip. The zip also contains a stdout and an odbc trace log.

test-artifacts.zip

[yitam]

Thanks @gjcarrette for bringing this to our attention. We will investigate and get back to you on this.

[yitam]

Hi @gjcarrette, just so you know, VARCHAR with column size 0 is essentially a varchar(max).

Based on the documentation of ISNULL, the replacement_value is the expression to be returned if check_expression is NULL, and replacement_value must be of a type that is implicitly convertible to the type of check_expression.

In your example, when K is null, this is what happens (you see that it returns an *):
select cast (-1 as char(1))

The same happens if you change your query to select (ISNULL(:K, -1));, which returns a *. It is not convertible to an integer.

However, if you try one of these:

select cast (1 as char(1)) -- returns 1
select cast (0 as char(1)) -- returns 0

Hence, using 1 or 0 instead of -1 gives you what is expected. I'm not asking you to change your script(s), but the above explains that this is not a bug. It is as designed.

[gjcarrette]

Thanks for providing the clarification on the designed behavior of Microsoft SQL Server.
This implies that my patch is needed even more, because at the level of the PDO_SQLSRV it does not make sense to pass a NULL value for K as a char(1) because the PDO_SQLSRV driver level has no way to
tell if the ISNULL replacement value argument is going to fit into char(1).
You see what I mean? So if my T-SQL test script had ISNULL(:K, 1) then it would work ok.
But if it had ISNULL(:K, 999) then it would also fail, because 999 does not fit in char(1).

Is there any way with pdo_sqlsrv at the php level to bind :K to a null value that will work?
I have tried PARAM_NULL and that does not work with ISNULL(:K,-1) either,
because the T-SQL data type being passed to SQLBindParameter is CHAR(1).

What I am saying is that it is pdo_sqlsrv that is making the arbitrary decision that a null value is a null char(1). That is not general enough. Is there a way at the level of PHP to change that to be a null as a varchar(0) (max)?

Obviously other choices in pdo_sqlsrv could be made too, such as CHAR(1000) and that would work
out too.

If there were a way at the PHP level that the application programmer could specify that the NULL
should be a CHAR(1000) that would be fine too. Is there such a way?

[yitam]

I see your points, @gjcarrette , and varchar(max) might be one way to fix this. We will do some more investigation / testing and get back to you on this.

[gjcarrette]

Yes, clearly deserves some investigation.

You will see my pull request which is a crude fix. On the other hand, taking inspiration from your more typical ODBC programmer, who would be using C or FORTRAN back in the day, the call to SQLBindParameter is likely in the case of an integer (on a 64-bit machine) to have:

• ValueType being SQL_C_SBIGINT
• ParameterType SQL_BIGINT
• StrLen_or_IndPtr being a pointer to memory with SQL_NULL_DATA

But the decisions made in PDO, in particular the PDO::PARAM_NULL type, pretty much preclude this more general treatment where there are many different types of T-SQL NULL.

The millions of lines of legacy PHP at the company where I work were written against the PDO_DBLIB driver, which of course has PDO::ATTR_EMULATE_PREPARES, and so a T-SQL NULL, or any other value, appeared directly in the T-SQL, such as ISNULL(NULL, -1) in which case I can't say what T-SQL type that NULL would be.

The engineers made a performance and lifecycle management decision to retire the use of PDO_DBLIB in favor of PDO_SQLSRV. The performance issue is that DBLIB, being built on freetds, lacks proper network buffering at the application layer, so sends out too make small packets, and suffers a performance hit in cloud environments.

Trying to minimize the amount of T-SQL rewriting. Even though ISNULL(CAST( :X AS INT), -1) would work fine.
There is a wrapper that converts all parameterized calls to sp_executesql calls, and that tends to fix many problems. But it can't be used in all cases.

[yitam]

Hi @gjcarrette

Did you see my comment to your pull request?

I've done some testing and your fix seems to solve another issue #1102 as well.

[yitam]

Any update @gjcarrette ?

[gjcarrette]

Sorry about the lack of response I was on vacation looking now.

[yitam]

5.10.0-beta 2 is just released. Please give it a try, @gjcarrette . Thanks again for your contribution.

[yitam]

Closing this now please feel free to ask us to reopen.