Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversion failure for null values #1102

Closed
gjdanis opened this issue Feb 25, 2020 · 6 comments
Closed

Conversion failure for null values #1102

gjdanis opened this issue Feb 25, 2020 · 6 comments
Labels

Comments

@gjdanis
Copy link

gjdanis commented Feb 25, 2020

PHP Driver version or file name

5.8

SQL Server version

Microsoft SQL Server 2017 (RTM-CU8) (KB4338363) - 14.0.3029.16 (X64) Jun 13 2018 13:35:56 Copyright (C) 2017 Microsoft Corporation Developer Edition (64-bit) on Linux (Ubuntu 16.04.4 LTS)

Client operating system

Windows 10 enterprise

PHP version

PHP 7.2.9 (cli) (built: Aug 22 2018 18:31:06) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
with Zend OPcache v7.2.9, Copyright (c) 1999-2018, by Zend Technologies
with Xdebug v2.6.0, Copyright (c) 2002-2018, by Derick Rethans

Microsoft ODBC Driver version

Table schema

CREATE DATABASE test

Problem description

It looks like null values are bound as char(1). This can lead to problems if the value is used with a type that's longer than one byte. For example:

    $username =...
    $password =...

    $pdo = new \PDO('sqlsrv:Server=localhost; Database=test', $username, $password);
    $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
    try {
      $statement = $pdo->prepare('DECLARE @d DATETIME = ISNULL(:value, GETDATE()); SELECT @d');
      $statement->bindValue(':value', null);
      $statement->execute();
    } catch (\Throwable $th) {
      echo $th->getMessage();
      die();
    }

Internally this is what's sent to the server is this (I used SQL Server Profiler to grab the SQL):

declare @p1 int
set @p1=NULL
exec sp_prepexec @p1 output,N'@P1 char(1)',N'DECLARE @d DATETIME = ISNULL(@P1, GETDATE()); SELECT @d',NULL
select @p1

Which results in this error: Conversion failed when converting date and/or time from character string.

We can fix the problem by using a type like @P1 char(20) rather than @P1 char(1).

Expected behavior and actual behavior

No hard failure.

Repro code or steps to reproduce

See the description.

@gjdanis
Copy link
Author

gjdanis commented Feb 25, 2020

This might just be a problem with ISNULL. Changing the query to DECLARE @d DATETIME = COALESCE(@P1, GETDATE()); SELECT @d works fine, but I'm wondering if the driver can prevent us from having to change the SQL here by changing the parameter type we send to the server.

@david-puglielli
Copy link
Contributor

@gjdanis Thank you for reporting this. We will investigate and get back to you.

@yitam
Copy link
Contributor

yitam commented Mar 2, 2020

Hi @gjdanis , this might be an issue on the server side, because even if you want a parameter to be NULL, somehow the driver or the server needs to figure out what data type the NULL is.

The pdo_sqlsrv driver could make an extra trip to the server by getting the data type of this null value, but this might affect the query performance. Besides, in most cases setting it as char(1) works, except for binary fields, where we set it as a binary(1) instead.

Other than the alternative you mentioned above, you may want to consider modifying your query as follows:

DECLARE @d SQL_VARIANT = ISNULL(:value, GETDATE()); SELECT @d

This is what I got by using the sql_variant type,

$output = $statement->fetch();
var_dump($output);
...
array(2) {
  [""]=>
  string(23) "2020-03-02 13:41:29.500"
  [0]=>
  string(23) "2020-03-02 13:41:29.500"
}

@yitam
Copy link
Contributor

yitam commented Mar 2, 2020

I take it back, @gjdanis . Using sql_variant type does not generate any error but doesn't work as shown above either. I got the above output by actually making an extra trip to the server.

Regarding your suggestion to use @p1 char(20) rather than @p1 char(1), it might work in your particular case because you know the size, but this will not guarantee it works in the other scenarios.

@yitam
Copy link
Contributor

yitam commented Mar 9, 2020

Do you still have any question for us, @gjdanis ?

@yitam yitam added the wontfix label Mar 9, 2020
@yitam
Copy link
Contributor

yitam commented Mar 20, 2020

Closing this due to inactivity @gjdanis. Please feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants