php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74613 PDO Exceptions not thrown when OUTPUT clause used in a failed query
Submitted: 2017-05-18 15:00 UTC Modified: 2017-06-30 11:36 UTC
From: dan dot marra at gmail dot com Assigned:
Status: Not a bug Package: PDO DBlib
PHP Version: Irrelevant OS: Linux
Private report: No CVE-ID: None
 [2017-05-18 15:00 UTC] dan dot marra at gmail dot com
Description:
------------
---
From manual page: http://www.php.net/language.control-structures
---

When you have an error in a t-sql query, but that query ALSO has an OUTPUT clause in it, PDO does not throw an exception when expected (likely because there is a return value).

Test script:
---------------
/* in MSSQL */
CREATE TABLE foobar (
foo int not null
);

ALTER TABLE foobar
ADD CONSTRAINT unique_foo UNIQUE (foo);   
GO 

insert into foobar (foo) values (1);
go


/* IN PHP */
<?php

$conn = new PDO('dblib:host='.$hostname.';dbname='.$database, $username, $password);
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$stmt = $conn->prepare("insert into foobar (foo) output INSERTED.foo values (1)");
try {
    $stmt->execute();
    $foo = $stmt->fetchColumn();
} catch (PDOException $ex) {
    // THIS DOES NOT GET HIT, But it *should* because a unique constraint is violated. 
    die("you dun goofed son");
}

echo "all is good";

?>

Expected result:
----------------
what SHOULD happen is that an exception should be thrown due to the unique constraint violation. Instead, it is ignored.

If you remove the output clause from the query, this works as expected.

Actual result:
--------------
Error from mssql is ignored

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-06-29 10:26 UTC] adambaratz@php.net
-Status: Open +Status: Not a bug -Package: PDO related +Package: PDO DBlib
 [2017-06-29 10:26 UTC] adambaratz@php.net
I wouldn't describe this as an issue with pdo_dblib. The OUTPUT clause does change how the query operates. If you're using FreeTDS, you can enable logging and compare the server responses:
http://www.freetds.org/userguide/logging.htm

When you use the OUTPUT clause, the constraint issue is returned, but as a message instead of an error. pdo_dblib is only doing what it's told. My preference is that it doesn't apply special logic on top of what the DB is returning. What works for you and your particular setup may not be appropriate for everyone.

You can catch this state by checking for a return value of false from $stmt->fetchColumn. You can then call $stmt->errorInfo() if you want to do something with the returned message.
 [2017-06-29 14:58 UTC] dan dot marra at gmail dot com
I have to disagree. PDO is an abstraction library, and as such there shouldn't be unexpected behavior for specific edge cases If PDO gives me the option to define exceptions be thrown on errors, then PDO should respect that choice consistently. Code did not execute as expected, so why would it not be an exception? 

I think you would be hard pressed to find anyone who wants to have code that happily executes, and silently fails because 99% of the time they can rely on the exception, only to be surprised when an edge case does not do this. 

I would agree with you if the insert statement itself executed, but only a value was not returned. Your solution would make sense. This is not the case however. The statement fails entirely. If this is not a situation where an exception should be thrown, I don't know what is. 

The message being sent at the moment is "PDO exceptions cannot be trusted". I can't see how it would not be appropriate for everyone to get an exception when a statement does not execute as expected. 

AT THE VERY LEAST; the documentation should, in big red letters, notify users that this issue exists
 [2017-06-30 11:36 UTC] adambaratz@php.net
To automatically translate some messages into errors, you'd have to introduce a lot of complexity to pdo_dblib. The simplest approach would be to mirror all possible codes in the source... for all versions of SQL Server and Sybase, and keep them up to date. But then there'd be edge cases, like how you can use SET ARITHABORT/ARITHIGNORE/ANSI_WARNINGS to control how overflow and divide-by-zero errors should be handled. There might be similar hooks for the issue you're dealing with. Exceptions are triggered consistently, for anything reported by the database as an error.

I think our disagreement is around what pdo_dblib should abstract. The usual pattern for PHP extensions is to be a minimal layer on top of C APIs. Basically, don't hide info to maximize flexibility in userland. You can extend the PDO and PDOStatement classes -- see the PDO::ATTR_STATEMENT_CLASS option -- if you want to package up changes like this.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Tue Jan 22 17:01:25 2019 UTC