Add test to verify null cast in boolean type #632

tlfbrito · 2014-07-18T20:29:38Z

This test verify how null values are casted to boolean values in PostgresPlatform.

NULL values are wrongly casted when flag useBooleanTrueFalseStrings is true, This issue can be related to #625

Fix pg boolean conversion

stof · 2014-07-18T22:09:55Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

@@ -786,6 +786,9 @@ public function convertBooleans($item)
        return $this->doConvertBooleans(
            $item,
            function ($boolean) {
+                if (is_null($boolean)) {


please use null === $bpolean to check for null values, for consistency with the Doctrine coding standards

@stof Thanks for the feedback. It's done ;)

tlfbrito · 2014-07-22T20:06:18Z

@Ocramius Can you give me feedback on this? Is this good to merge?

Ocramius · 2014-07-22T20:13:43Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

@@ -786,6 +786,9 @@ public function convertBooleans($item)
        return $this->doConvertBooleans(
            $item,
            function ($boolean) {
+                if (null === $boolean) {
+                    return 'NULL';
+                }
                return true === $boolean ? 'true' : 'false';


Empty newline before this line

Ocramius · 2014-07-22T20:13:51Z

Looks sane

Add new line after an IF statement to be consistent with the coding standard

deeky666 · 2014-07-23T08:07:49Z

Hmmm when I use PDO::ATTR_EMULATE_PREPARES (boolean true/false strings) and try to insert 'NULL' into a boolean column it evaluates to true. If I insert null it evaluates to false. Did I do something wrong or how would you insert a real NULL value into a boolean column when using PDO::ATTR_EMULATE_PREPARES? If my test was not wrong I assume your patch isn't solving any issue here and even making it "worse" as a PHP null value would now evaluate to true.

No matter what, I would really like to have a functional test for this in Doctrine\Tests\DBAL\Functional\Ticket\DBAL630Test to verify your patch is working as expected.

Update from master

tlfbrito · 2014-07-24T22:06:54Z

@deeky666 I think that it works because in your test you have pre-setted the bindValue type as PDO::PARAM_BOOL
The PDO::ATTR_EMULATE_PREPARES doesn't affect this issue.

$stmt->bindValue(1, $platform->convertBooleansToDatabaseValue(null), PDO::PARAM_BOOL);

But I think that doctrine call bindValue without the type setted and it is figured out later, depending on the value type.
Can you help me with this one?

Add functional test to check null conversion with boolean type. This test required NULL to be allowed in the table column.

Update from master repository

deeky666 · 2014-07-28T09:30:15Z

tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php

+
+    public function testBooleanConversionNullParamEmulatedPrepares()
+    {
+        $this->_conn->exec('CREATE TABLE dbal630_allow_nulls (id SERIAL, bool_col BOOLEAN);');


I'm not sure but I think you have to move the table creation statement to the setUp() method to avoid errors if the table already exists (like if you run the test suite a second time).

deeky666 · 2014-07-28T09:35:53Z

Okay I think I get what you mean. But the question I am asking myself is how do you need to convert the boolean value if you do not omit the binding type. In Doctrine\DBAL\Connection you can pass the binding types to several methods and then you will have that exact use-case I described here.

Like:

$conn->insert('mytable', array('bool_column' => null), array('bool_column' => \PDO::PARAM_BOOL));

Add unit test to check boolean conversion with and without PDO boolean type

Update from master repository

tlfbrito · 2014-07-29T00:46:45Z

@deeky666 I think that these new tests should be enough to show want happen when the bindValue is used with and without PDO type. However I am having problems with PHP 5.6.
Can you give me some help ?

deeky666 · 2014-07-29T09:13:04Z

tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php

+
+        $row = $this->_conn->fetchAssoc('SELECT bool_col FROM dbal630_allow_nulls WHERE id = ?', array($id));
+
+        $this->assertEquals($databaseConvertedValue, $row['bool_col']);


You have to use assertSame() here to enable type equality assertion.

deeky666 · 2014-07-29T09:15:32Z

tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php

+            // statement value, database converted value result
+            array(true, true),
+            array(false, false),
+            array(null, false)


IMO this assumption is wrong and is exactly the issue I was talking about. Why should I expect to get false if I have a NULL value in the database? There is no distinction between NULL and false anymore. Is this maybe even a PDO issue of some kind?

We aren't getting from the database, we are inserting into the database.
I'm just using the PDO default behavior. If we tell the PDO that the value is a boolean (PDO::PARAM_BOOL), isn't supposed that the NULL is converted to false? Why can't we rely on that assumption?

Because "NULL" is neither "true" or "false".

Update from main repository

Update from master repository

Update from main repository

edigu · 2014-10-27T14:17:26Z

Any progress on this? Currently "doctrine/dbal": "~2.5@dev" with PHP 5.6 can't properly handling nulls when using PostgreSQL92Platform. (PostgreSQL version is 9.3)

edigu · 2014-10-28T11:04:23Z

Yes i tried. I'm using DBAL over Doctrine orm, really don't understand problem's layer. ORM, DBAL, PDO or PostgreSQLPlatform?

I think problem only affects NULLABLE bool fields. I want to summarize my experience:

$conn     = $this->getServiceLocator()->get('doctrine.connection.orm_default');
$p = $conn->getDatabasePlatform();
$conn->exec('CREATE TABLE test_bool (id SERIAL, bool_col BOOLEAN NULL);');

$stmt = $conn->prepare('INSERT INTO test_bool (bool_col) VALUES (?)');
$stmt->bindValue(1, SEE_SNIPPET)
$stmt->execute();
// UBTFS : UseBooleanTrueFalseStrings
// PREPARE : ATTR_EMULATE_PREPARES

Snippet	UBTFS	PREPARE	Persisted value in DB
$p->convertBooleansToDatabaseValue('false')	true	true	`false`
$p->convertBooleansToDatabaseValue(false)	true	true	`false`
$p->convertBooleansToDatabaseValue(true)	true	true	`true`
$p->convertBooleansToDatabaseValue('true')	true	true	`true`
$p->convertBooleansToDatabaseValue(null)	true	true	`false`
null	true	true	`null`
$p->convertBooleans(null)	true	true	`false`
$p->convertBooleansToDatabaseValue('false')	false	true	`false`
$p->convertBooleansToDatabaseValue(false)	false	true	`false`
$p->convertBooleansToDatabaseValue(true)	false	true	`true`
$p->convertBooleansToDatabaseValue('true')	false	true	`true`
$p->convertBooleansToDatabaseValue(null)	false	true	`null`
null	false	true	`null`
$p->convertBooleans(null)	false	true	`null`
$p->convertBooleansToDatabaseValue('false')	false	false	`false`
$p->convertBooleansToDatabaseValue(false)	false	false	`false`
$p->convertBooleansToDatabaseValue(true)	false	false	`true`
$p->convertBooleansToDatabaseValue('true')	false	false	`true`
$p->convertBooleansToDatabaseValue(null)	false	false	`null`
null	false	false	`null`
$p->convertBooleans(null)	false	false	`null`

tlfbrito · 2014-10-28T11:34:30Z

@edigu Your snippet looks good to me. Did you run it against which PHP version?

edigu · 2014-10-28T12:36:33Z

PHP 5.6.1

tlfbrito · 2014-10-28T21:13:02Z

I've confirmed, this PR was failing in PHP 5.6.0 due a bug in that version, this is now fixed in PHP 5.6.1 and 5.6.2

deeky666 · 2014-10-28T21:25:23Z

@TiagoBrito I am about to merge this. Still not sure why PDO_PGSQL converts null to false when using emulated prepares and PDO::PARAM_BOOL. Other drivers don't behave like that and allow real null values to be inserted into DB. I have tested this. But I guess this is an implication on PHP side which we can't change. What do you think?

tlfbrito · 2014-10-28T22:08:35Z

@deeky666 I actually think that using emulated prepares and PDO::PARAM_BOOL should convert null to false. Because IMO, the PDO::PARAM_BOOL is used in situations like:

Column type is boolean and NOT NULL

In this case, the PDO::PARAM_BOOL is used to guarantee that the NOT NULL restriction isn't violated.
If we use it without PDO::PARAM_BOOL
$stmt->bindValue(1, $platform->convertBooleansToDatabaseValue($statementValue));
the PDO will save NULL in the database. Is the PDO that is actually making that casting, not we. I think we are good to merge.

if we call bindValue directly without using $platform->convertBooleansToDatabaseValue method
$stmt->bindValue(1, VALUE, TYPE)

VALUE	TYPE	RESULT in DB
true	PDO::PARAM_BOOL	true
false	PDO::PARAM_BOOL	true
null	PDO::PARAM_BOOL	false
true	null	true
false	null	true
null	null	null

Add test to verify null cast in boolean type

deeky666 · 2014-10-29T16:12:39Z

Okay merging then. Thank you all for your investigation.

deeky666 · 2014-10-29T16:50:05Z

wtf it is failing again in master oO
https://travis-ci.org/doctrine/dbal/builds/39399031

Is there some sort of randomness going on here?

tlfbrito · 2014-10-29T16:56:47Z

@deeky666 I've run it yesterday a few times and didn't notice anything random.

deeky666 · 2014-10-29T16:58:25Z

@TiagoBrito but obviously it is failing again in Travis. I have no idea what's going on here...

tlfbrito · 2014-10-29T19:56:15Z

I'm running 100 times this test suite

Add unit test to check boolean conversion with and without PDO boolean type

PowerKiKi · 2014-11-13T02:47:31Z

Hey, I noticed this PR failed on Travis. And I could successfully reproduce the failure on my local machine. Maybe I could help you sorting this thing out.

My versions:

$ php --version
PHP 5.6.2-1+deb.sury.org~trusty+2 (cli) (built: Oct 19 2014 12:56:17) 
Copyright (c) 1997-2014 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2014 Zend Technologies
    with Zend OPcache v7.0.4-dev, Copyright (c) 1999-2014, by Zend Technologies
    with Xdebug v2.2.5, Copyright (c) 2002-2014, by Derick Rethans

$ phpunit --version
PHPUnit 4.1.6 by Sebastian Bergmann.

$ psql --version 
psql (PostgreSQL) 9.3.5

And the failing test:

$ phpunit --configuration  tests/travis/pgsql.travis.xml --filter testBooleanConversionNullParamEmulatedPreparesWithBooleanTypeInBindValue
PHPUnit 4.1.1 by Sebastian Bergmann.

Configuration read from /sites/gims/vendor/doctrine/dbal/tests/travis/pgsql.travis.xml

..F

Time: 2.16 seconds, Memory: 34.25Mb

There was 1 failure:

1) Doctrine\Tests\DBAL\Functional\Ticket\DBAL630Test::testBooleanConversionNullParamEmulatedPreparesWithBooleanTypeInBindValue with data set #2 (NULL, false)
Failed asserting that true matches expected false.

Would you have any hints where to investigate ?

PowerKiKi · 2014-11-13T04:30:33Z

Oh, forgot to mention an extract from php -i showing pdo_pgsql version:

pdo_pgsql

PDO Driver for PostgreSQL => enabled
PostgreSQL(libpq) Version => 9.3.5
Module version => 1.0.2
Revision =>  $Id: fe003f8ab9041c47e97784d215c2488c4bda724d $

Would you have any suggestions as to how to dig further ? I had quick look but did not find anything useful...

edigu · 2014-11-13T07:24:55Z

See #625

PowerKiKi · 2014-11-13T07:44:34Z

thanks for letting me know :)

…g standard

…value Add unit test to check boolean conversion with and without PDO boolean type

Tiago Brito added 2 commits July 18, 2014 21:23

add test to verify null cast in boolean type

f624876

resolves #632 Fix null cast in boolean type in Postgres platform

031f38c

tlfbrito referenced this pull request Jul 18, 2014

Merge pull request #625 from lucasvanlierop/fix-pg-boolean-conversion

5eb36c7

Fix pg boolean conversion

stof reviewed Jul 18, 2014
View reviewed changes

refs #632 Remove is_null to be consistent with doctrine coding standard

43d13fc

Ocramius reviewed Jul 22, 2014
View reviewed changes

Update PostgreSqlPlatform.php

4ebdb8c

Add new line after an IF statement to be consistent with the coding standard

Merge pull request #1 from doctrine/master

24bf3bc

Update from master

Tiago Brito added 2 commits July 24, 2014 23:29

Test to check null conversion with boolean type

fd3b116

Add functional test to check null conversion with boolean type. This test required NULL to be allowed in the table column.

Merge pull request #2 from doctrine/master

1a7fa0c

Update from master repository

deeky666 reviewed Jul 28, 2014
View reviewed changes

Tiago Brito added 4 commits July 29, 2014 00:38

refs #632 check boolean conversion with and without PDO type value

7b29bd5

Add unit test to check boolean conversion with and without PDO boolean type

Merge pull request #3 from doctrine/master

907513f

Update from master repository

Fix boolean conversion with php 5.6

598503d

Revert change to fix php 5.6

e8d7820

deeky666 reviewed Jul 29, 2014
View reviewed changes

Change assertEquals to assertSame

8065c83

deeky666 reviewed Jul 29, 2014
View reviewed changes

Tiago Brito added 4 commits August 30, 2014 12:36

Merge pull request #4 from doctrine/master

8eb2ddc

Update from main repository

Merge pull request #5 from doctrine/master

9c91857

Update from master repository

Merge pull request #6 from doctrine/master

1f5456a

Update from master repository

Merge pull request #7 from doctrine/master

f2aa22c

Update from main repository

deeky666 added a commit that referenced this pull request Oct 29, 2014

Merge pull request #632 from tiagobrito/master

d126728

Add test to verify null cast in boolean type

deeky666 merged commit d126728 into doctrine:master Oct 29, 2014

deeky666 pushed a commit that referenced this pull request Nov 5, 2014

resolves #632 Fix null cast in boolean type in Postgres platform

6b09709

deeky666 pushed a commit that referenced this pull request Nov 5, 2014

refs #632 Remove is_null to be consistent with doctrine coding standard

b1cc410

deeky666 pushed a commit that referenced this pull request Nov 5, 2014

refs #632 check boolean conversion with and without PDO type value

437f416

Add unit test to check boolean conversion with and without PDO boolean type

sarcher pushed a commit to sarcher/dbal that referenced this pull request Jan 1, 2015

resolves doctrine#632 Fix null cast in boolean type in Postgres platform

1611a2f

sarcher pushed a commit to sarcher/dbal that referenced this pull request Jan 1, 2015

refs doctrine#632 Remove is_null to be consistent with doctrine codin…

3ba6cde

…g standard

sarcher pushed a commit to sarcher/dbal that referenced this pull request Jan 1, 2015

refs doctrine#632 check boolean conversion with and without PDO type …

5832ede

…value Add unit test to check boolean conversion with and without PDO boolean type

Ocramius mentioned this pull request Mar 2, 2015

PDO, PostgreSQL and bool zendframework/zendframework#7284

Closed

doctrinebot mentioned this pull request Dec 7, 2015

DBAL-942: [GH-632] Add test to verify null cast in boolean type #2186

Closed

GeeH mentioned this pull request Jun 28, 2016

PDO, PostgreSQL and bool zendframework/zend-db#134

Open

Ocramius mentioned this pull request Oct 1, 2016

Normalize method signatures for fetch() and fetchAll(), ensuring compatibility with the PDOStatement signature #2527

Merged

1 task

aivus mentioned this pull request Dec 14, 2016

Impossible to save null to nullable boolean field #2580

Closed

michalbundyra mentioned this pull request Jan 16, 2020

PDO, PostgreSQL and bool laminas/laminas-db#114

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test to verify null cast in boolean type #632

Add test to verify null cast in boolean type #632

tlfbrito commented Jul 18, 2014

stof Jul 18, 2014

tlfbrito Jul 18, 2014

tlfbrito commented Jul 22, 2014

Ocramius Jul 22, 2014

Ocramius commented Jul 22, 2014

deeky666 commented Jul 23, 2014

tlfbrito commented Jul 24, 2014

deeky666 Jul 28, 2014

deeky666 commented Jul 28, 2014

tlfbrito commented Jul 29, 2014

deeky666 Jul 29, 2014

deeky666 Jul 29, 2014

tlfbrito Jul 29, 2014

davividal Oct 27, 2014

edigu commented Oct 27, 2014

edigu commented Oct 28, 2014

tlfbrito commented Oct 28, 2014

edigu commented Oct 28, 2014

tlfbrito commented Oct 28, 2014

deeky666 commented Oct 28, 2014

tlfbrito commented Oct 28, 2014

deeky666 commented Oct 29, 2014

deeky666 commented Oct 29, 2014

tlfbrito commented Oct 29, 2014

deeky666 commented Oct 29, 2014

tlfbrito commented Oct 29, 2014

PowerKiKi commented Nov 13, 2014

PowerKiKi commented Nov 13, 2014

edigu commented Nov 13, 2014

PowerKiKi commented Nov 13, 2014


		$row = $this->_conn->fetchAssoc('SELECT bool_col FROM dbal630_allow_nulls WHERE id = ?', array($id));

		$this->assertEquals($databaseConvertedValue, $row['bool_col']);

Add test to verify null cast in boolean type #632

Add test to verify null cast in boolean type #632

Conversation

tlfbrito commented Jul 18, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlfbrito commented Jul 22, 2014

Choose a reason for hiding this comment

Ocramius commented Jul 22, 2014

deeky666 commented Jul 23, 2014

tlfbrito commented Jul 24, 2014

Choose a reason for hiding this comment

deeky666 commented Jul 28, 2014

tlfbrito commented Jul 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edigu commented Oct 27, 2014

edigu commented Oct 28, 2014

tlfbrito commented Oct 28, 2014

edigu commented Oct 28, 2014

tlfbrito commented Oct 28, 2014

deeky666 commented Oct 28, 2014

tlfbrito commented Oct 28, 2014

deeky666 commented Oct 29, 2014

deeky666 commented Oct 29, 2014

tlfbrito commented Oct 29, 2014

deeky666 commented Oct 29, 2014

tlfbrito commented Oct 29, 2014

PowerKiKi commented Nov 13, 2014

PowerKiKi commented Nov 13, 2014

edigu commented Nov 13, 2014

PowerKiKi commented Nov 13, 2014