Doctrine DBAL
  1. Doctrine DBAL
  2. DBAL-630

Incorrect PostgreSQL boolean handling

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 2.4
    • Fix Version/s: None
    • Component/s: Platforms
    • Security Level: All
    • Labels:
      None

      Description

      This is a follow-up to issue #457 ("Use int values instead of strings for PostgreSQL booleans"), which is still not fixed. Int values are no solution at all. In fact the root cause lies deeper, outside the PostgreSQL platform class.

      1. The patch to fix #457 does not change the default behaviour of the PostgreSQL platform class (method convertBooleans() returns strings 'true'/'false'). When the PostgreSQL PDO driver is configured to emulate prepared statements, it still results in unexpected failures, storing boolean false entity values as true in the database.

      2. The new alternative boolean conversion mode activated by PostgreSqlPlatform::setUseBooleanTrueFalseStrings(false) is of no use as it prevents execution of DQL queries with boolean conditions, because integers 0 and 1 are not valid boolean literals in PostgreSQL.

      The root cause is the notion of a PHP value being convertible to a database value. Because in fact there are two different types of "database values":

      • Literals used directly in SQL statements
      • Values passed as parameters to prepared statements

      To make this absolutely clear:

      Prerequisites
      $pdo = new PDO(...);
      $pdo->exec('CREATE TABLE my_table(bool_col BOOLEAN NOT NULL)');
      $stmt = $pdo->prepare('INSERT INTO my_table(bool_col) VALUES(?)');
      
      Using string 'false'
      $value = 'false';
      
      // This works, using the SQL literal false
      $pdo->exec('INSERT INTO my_table(bool_col) VALUES(' . $value . ')');
      
      // This works, too. But it's remarkable that Postgres accepts the string 'false'
      // as a boolean value. Compare this to the string 'NULL' in an SQL statement vs.
      // 'NULL' as a prepared statement param (instead of PHP null).
      $stmt->bindValue(1, $value, PDO::PARAM_BOOL);
      $stmt->execute();
      
      // Sets bool_col to true! The PostgreSQL PDO driver correctly expects a boolean
      // and (string)'false' yields true.
      $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
      $stmt->bindValue(1, $value, PDO::PARAM_BOOL);
      $stmt->execute();
      
      Using boolean false
      $value = false;
      
      // This will obviously fail, because false is cast to an empty string, resulting
      // in "... VALUES()".
      $pdo->exec('INSERT INTO my_table(bool_col) VALUES(' . $value . ')');
      
      // Works
      $stmt->bindValue(1, $value, PDO::PARAM_BOOL);
      $stmt->execute();
      
      // Works, too
      $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
      $stmt->bindValue(1, $value, PDO::PARAM_BOOL);
      $stmt->execute();
      
      Using integer 0
      $value = 0;
      
      // Causes 'ERROR: column "bool_col" is of type boolean but expression is of type integer'
      $pdo->exec('INSERT INTO my_table(bool_col) VALUES(' . $value . ')');
      
      // Works
      $stmt->bindValue(1, $value, PDO::PARAM_BOOL);
      $stmt->execute();
      
      // Works, because of implicit PHP type cast 0 -> false
      $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
      $stmt->bindValue(1, $value, PDO::PARAM_BOOL);
      $stmt->execute();
      

      There are two locations in DBAL and ORM where AbstractPlatform::convertBooleans() is called to build SQL literals:

      DoctrineDBALPlatformsAbstractPlatform::getDefaultValueDeclarationSQL()
      $default = " DEFAULT '" . $this->convertBooleans($field['default']) . "'";
      

      Wow, this is even being enclosed in single quotes!? But then the whole method is buggy anyway, e.g. using an unescaped string value for a string literal (scenarios for SQL injection unlikely but possible).

      DoctrineORMQuerySqlWalker::walkLiteral()
      case AST\Literal::BOOLEAN:
      	$bool = strtolower($literal->value) == 'true' ? true : false;
      	$boolVal = $this->conn->getDatabasePlatform()->convertBooleans($bool);
      	return $boolVal;
      

      ...and the result is later used as a boolean literal in an SQL query.

      To solve this we need something like AbstractPlatform::convertBoolToSqlLiteral() (returning strings true and false for the Postgres platform) and AbstractPlatform::convertBoolToDbValue() (converting to integer 0 or 1 for platforms without a native bool type).

      Note 1: The docs currently suggest to call $conn->getDatabasePlatform()->setUseBooleanTrueFalseStrings($flag). This is bad OO design, because getDatabasePlatform() returns an AbstracPlattform instance which does not have a contract for the method.

      Note 2: What makes this problem so nasty is the fact that switching to emulated prepares makes an application fail in a non-obvious way. There will be no traceable errors but simply all boolean false values in ORM entities stored as boolean true. When integration tests use a different database (e.g. an SQLite in-memory DB to minimize test execution time) the problem will even escape the tests. And the distance between cause and effect also makes the problem very hard to find. Who would expect a database driver setting to cause booleans in the DB to be the opposite of what they're supposed to be? Especially as this only becomes apparent after later re-hydrating stored entities.

      Note 3: Why emulated prepared statements matter: When PostgreSQL processes a prepared statement, its query planner works out a query plan and uses it for all subsequent executions of this query. This way it has to make a rather crude guess at the number of affected rows from each table in a join. When a non-prepared query is executed, the query planner can take into account the given values (mostly the ones in the "WHERE" part of the query) and make a much more specific guess at which plan will perform best.
      In our case, we decided to switch to emulated prepares after we found out that a complex query in our application would run five times faster with emulated prepares.

      Note 4: Is there a reason for AbstractPlatform::convertBooleans() accepting either a single bool value or an array of bool values? I didn't find client code calling it with an array. This makes the method less obvious, is currently implemented with code duplication and at least for the PostgreSQL plattform class, the "array of bool" functionality is not even tested.

        Activity

        Hide
        Stan Imbt added a comment -

        Davi, I can reproduce the referenced PHP bug, both with and without emulated prepares, but it is not related to this Doctrine bug.

        Doctrine calls PDOStatement::bindValue(), specifying the data type PDO::PARAM_BOOL in case of bool fields. The PHP bug only occurs when the param data type is not provided, apparently converting the param value to string ((string)false === '') and passing that on to the DBMS.

        Show
        Stan Imbt added a comment - Davi, I can reproduce the referenced PHP bug, both with and without emulated prepares, but it is not related to this Doctrine bug. Doctrine calls PDOStatement::bindValue(), specifying the data type PDO::PARAM_BOOL in case of bool fields. The PHP bug only occurs when the param data type is not provided, apparently converting the param value to string ((string)false === '') and passing that on to the DBMS.
        Hide
        Davi Koscianski Vidal added a comment -

        Stan, almost the same 'test case' as PHP bug:

        <?php
        
        /*
        CREATE TABLE "booleantest" (
          "persistence_object_identifier" serial NOT NULL,
          "hidden" boolean NOT NULL
        );
         */
        
        $handle = new PDO('pgsql:host=127.0.0.1 dbname=postgres', 'postgres', 'postgres');
        $handle->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        $handle->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        $handle->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
        $handle->setAttribute(PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT, true);
        
        $statement = $handle->prepare('INSERT INTO booleantest (hidden) VALUES (?)');
        
        // works as expected
        $statement->execute(array(true));
        echo 'TRUE has been inserted' . PHP_EOL;
        
        $statement->bindValue(1, "true", PDO::PARAM_BOOL);
        echo 'TRUE has been inserted' . PHP_EOL;
        
        // dies with
        // PDOException: SQLSTATE[22P02]: Invalid text representation: 7 ERROR:  invalid input syntax for type boolean: ""
        // $statement->execute(array(FALSE));
        
        $statement->bindValue(1, "false", PDO::PARAM_BOOL);
        $statement->debugDumpParams();
        $statement->execute();
        echo 'FALSE has been inserted' . PHP_EOL;
        

        When PDO::ATTR_EMULATE_PREPARES is set to true, $stmt->debugDumpParams() outputs:

        SQL: [43] INSERT INTO booleantest (hidden) VALUES (?)
        Params:  1
        Key: Position #0:
        paramno=0
        name=[0] ""
        is_param=1
        param_type=2
        

        But when it is disabled, it outputs:

        SQL: [43] INSERT INTO booleantest (hidden) VALUES (?)
        Params:  1
        Key: Position #0:
        paramno=0
        name=[0] ""
        is_param=1
        param_type=5
        

        This is exactly the same output from tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php (after adding debugDumpParam() to DBAL\Driver\PDOStatement.php, obviously). param_type = 2 when type = PDO::PARAM_STR.
        For debugging purposes, I changed DBAL\Driver\PDOStatement.php line 67 from return parent::bindValue($param, $value, $type); to parent::bindValue(1, "false", \PDO::PARAM_BOOL); (so I will always store a boolean false on database), but debugDumpParam() keeps telling me that param_type is 2 if I'm using emulated prepares.

        Show
        Davi Koscianski Vidal added a comment - Stan, almost the same 'test case' as PHP bug: <?php /* CREATE TABLE "booleantest" ( "persistence_object_identifier" serial NOT NULL, "hidden" boolean NOT NULL ); */ $handle = new PDO('pgsql:host=127.0.0.1 dbname=postgres', 'postgres', 'postgres'); $handle->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $handle->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $handle->setAttribute(PDO::ATTR_EMULATE_PREPARES, true ); $handle->setAttribute(PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT, true ); $statement = $handle->prepare('INSERT INTO booleantest (hidden) VALUES (?)'); // works as expected $statement->execute(array( true )); echo 'TRUE has been inserted' . PHP_EOL; $statement->bindValue(1, " true " , PDO::PARAM_BOOL); echo 'TRUE has been inserted' . PHP_EOL; // dies with // PDOException: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for type boolean : "" // $statement->execute(array(FALSE)); $statement->bindValue(1, " false " , PDO::PARAM_BOOL); $statement->debugDumpParams(); $statement->execute(); echo 'FALSE has been inserted' . PHP_EOL; When PDO::ATTR_EMULATE_PREPARES is set to true, $stmt->debugDumpParams() outputs: SQL: [43] INSERT INTO booleantest (hidden) VALUES (?) Params: 1 Key: Position #0: paramno=0 name=[0] "" is_param=1 param_type=2 But when it is disabled, it outputs: SQL: [43] INSERT INTO booleantest (hidden) VALUES (?) Params: 1 Key: Position #0: paramno=0 name=[0] "" is_param=1 param_type=5 This is exactly the same output from tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php (after adding debugDumpParam() to DBAL\Driver\PDOStatement.php, obviously). param_type = 2 when type = PDO::PARAM_STR. For debugging purposes, I changed DBAL\Driver\PDOStatement.php line 67 from return parent::bindValue($param, $value, $type); to parent::bindValue(1, "false", \PDO::PARAM_BOOL); (so I will always store a boolean false on database), but debugDumpParam() keeps telling me that param_type is 2 if I'm using emulated prepares.
        Hide
        Davi Koscianski Vidal added a comment -

        Using $stmt->bindValue(1, 'false', \PDO::PARAM_STR); works as expected. Maybe a dirty workaround?

        Show
        Davi Koscianski Vidal added a comment - Using $stmt->bindValue(1, 'false', \PDO::PARAM_STR); works as expected. Maybe a dirty workaround?
        Hide
        Davi Koscianski Vidal added a comment -

        This same test case passes with PHP 5.3.18, but fails with 5.5.3 and 5.5.11.

        $ phpenv shell 5.5.11
        
        $ php --version
        PHP 5.5.11 (cli) (built: Apr  3 2014 09:52:27) 
        Copyright (c) 1997-2014 The PHP Group
        Zend Engine v2.5.0, Copyright (c) 1998-2014 Zend Technologies
        
        $ vendor/bin/phpunit -c postgres.phpunit.xml tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php 
        PHPUnit 3.7.34 by Sebastian Bergmann.
        
        Configuration read from.postgres.phpunit.xml
        
        ..F
        
        Time: 3.91 seconds, Memory: 4.25Mb
        
        There was 1 failure:
        
        1) Doctrine\Tests\DBAL\Functional\Ticket\DBAL630Test::testBooleanConversionBoolParamEmulatedPrepares
        Failed asserting that true is false.
        
        ./tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php:79
        
        FAILURES!
        Tests: 3, Assertions: 6, Failures: 1.
        
        $ phpenv shell system 
        
        $ php --version
        PHP 5.5.3-1ubuntu2.2 (cli) (built: Feb 28 2014 20:06:05) 
        Copyright (c) 1997-2013 The PHP Group
        Zend Engine v2.5.0, Copyright (c) 1998-2013 Zend Technologies
            with Zend OPcache v7.0.3-dev, Copyright (c) 1999-2013, by Zend Technologies
        
        $ vendor/bin/phpunit -c postgres.phpunit.xml tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php 
        PHPUnit 3.7.34 by Sebastian Bergmann.
        
        Configuration read from ./postgres.phpunit.xml
        
        ..F
        
        Time: 1.59 seconds, Memory: 4.25Mb
        
        There was 1 failure:
        
        1) Doctrine\Tests\DBAL\Functional\Ticket\DBAL630Test::testBooleanConversionBoolParamEmulatedPrepares
        Failed asserting that true is false.
        
        ./tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php:79
        
        FAILURES!
        Tests: 3, Assertions: 6, Failures: 1.
        
        $ phpenv shell 5.3.18 
        
        $ php --version
        PHP 5.3.18 (cli) (built: Apr  2 2014 16:47:04) 
        Copyright (c) 1997-2012 The PHP Group
        Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies
        
        $ vendor/bin/phpunit -c postgres.phpunit.xml tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php 
        PHPUnit 3.7.34 by Sebastian Bergmann.
        
        Configuration read from ./postgres.phpunit.xml
        
        ...
        
        Time: 1.06 seconds, Memory: 6.50Mb
        
        OK (3 tests, 6 assertions)
        
        Show
        Davi Koscianski Vidal added a comment - This same test case passes with PHP 5.3.18, but fails with 5.5.3 and 5.5.11. $ phpenv shell 5.5.11 $ php --version PHP 5.5.11 (cli) (built: Apr 3 2014 09:52:27) Copyright (c) 1997-2014 The PHP Group Zend Engine v2.5.0, Copyright (c) 1998-2014 Zend Technologies $ vendor/bin/phpunit -c postgres.phpunit.xml tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php PHPUnit 3.7.34 by Sebastian Bergmann. Configuration read from.postgres.phpunit.xml ..F Time: 3.91 seconds, Memory: 4.25Mb There was 1 failure: 1) Doctrine\Tests\DBAL\Functional\Ticket\DBAL630Test::testBooleanConversionBoolParamEmulatedPrepares Failed asserting that true is false. ./tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php:79 FAILURES! Tests: 3, Assertions: 6, Failures: 1. $ phpenv shell system $ php --version PHP 5.5.3-1ubuntu2.2 (cli) (built: Feb 28 2014 20:06:05) Copyright (c) 1997-2013 The PHP Group Zend Engine v2.5.0, Copyright (c) 1998-2013 Zend Technologies with Zend OPcache v7.0.3-dev, Copyright (c) 1999-2013, by Zend Technologies $ vendor/bin/phpunit -c postgres.phpunit.xml tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php PHPUnit 3.7.34 by Sebastian Bergmann. Configuration read from ./postgres.phpunit.xml ..F Time: 1.59 seconds, Memory: 4.25Mb There was 1 failure: 1) Doctrine\Tests\DBAL\Functional\Ticket\DBAL630Test::testBooleanConversionBoolParamEmulatedPrepares Failed asserting that true is false. ./tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php:79 FAILURES! Tests: 3, Assertions: 6, Failures: 1. $ phpenv shell 5.3.18 $ php --version PHP 5.3.18 (cli) (built: Apr 2 2014 16:47:04) Copyright (c) 1997-2012 The PHP Group Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies $ vendor/bin/phpunit -c postgres.phpunit.xml tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php PHPUnit 3.7.34 by Sebastian Bergmann. Configuration read from ./postgres.phpunit.xml ... Time: 1.06 seconds, Memory: 6.50Mb OK (3 tests, 6 assertions)
        Hide
        Davi Koscianski Vidal added a comment - - edited

        I think I messed something when creating the PR. The bot automatically created this ticket: http://www.doctrine-project.org/jira/browse/DBAL-863

        I'm sorry.

        Show
        Davi Koscianski Vidal added a comment - - edited I think I messed something when creating the PR. The bot automatically created this ticket: http://www.doctrine-project.org/jira/browse/DBAL-863 I'm sorry.

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Stan Imbt
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated: