Boolean conversion #527

birko · 2014-02-12T09:49:08Z

Added platform specific conversion form database Boolean type to PHP bool type

Default Boolean conversion behaviour

added postgresql literals to Boolean conversion

Changed to call platform specific Boolean conversion

added postgresql platform conversion test

doctrinebot · 2014-02-12T09:49:16Z

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-811

We use Jira to track the state of pull requests and the versions they got
included in.

fixed bug when $item was set as bool true value added 'off' literal

added literals off and on to the testcase

Ocramius · 2014-02-14T07:32:26Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

+    */
+    public function convertFromBoolean($item)
+    {
+        if(in_array((string)$item, array('false', 'f', 'n', 'no', 'off')))


Can you fix the CS here? We follow PSR-2 with some minor additions

You can avoid the call to in_array if the returned value is either null, true or false

Changes after Ocramius comments

fixed parameter order in conditions

birko · 2014-02-14T09:01:45Z

@Ocramius hope I made the changes correctly in the way you wanted

Description fix

Ocramius · 2014-02-14T09:42:19Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

+     */
+    public function convertFromBoolean($item)
+    {
+        if((null !== $item) && (false !== $item) && (true !== $item) && in_array((string)$item, array('false', 'f', 'n', 'no', 'off')))


Break this line

you mean the "if" statement?

birko · 2014-02-14T18:04:52Z

Ok now I don't know what more I could do for the CS.

Ocramius · 2014-02-14T19:55:47Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

+            in_array((string)$item, array('false', 'f', 'n', 'no', 'off'))
+        ) {
+            return false;
+        } else {


You can avoid the else clause completely, since you're returning early

updated convert function to include uppercased values

Updated test for uppercased values

birko · 2014-02-15T08:55:12Z

@Ocramius You were right I forgot to think about uppercased values

Ocramius · 2014-02-15T09:02:40Z

@deeky666 think another method in the abstract platform is ok? Otherwise, looks good to me.

Could need squashing of the commits, but I can do that before someone merges

birko · 2014-02-15T09:50:42Z

@Ocramius Sorry for too much commints. I had problems cloning the repo into my PC so most of the changes I made here via Github edit function

Ocramius · 2014-02-15T09:57:50Z

@birko that can be dealt with, no problem

deeky666 · 2014-02-15T10:39:11Z

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

+     */
+    public function convertFromBoolean($item)
+    {
+        return (null === $item) ? null : (bool) $item;


Please use the short ternary expression: return null === $item ?: (bool) $item

deeky666 · 2014-02-15T11:11:25Z

@birko Can you please give more details about the issue here? I can see that PostgreSQL allows different boolean values but I don't understand why we need to check for those when retrieving them from the database. AFAIK pdo_pgsql already converts them internally to the proper PHP boolean type. I did some quick tests about this and my assumption seems to be true. Is there another issue I am missing here?

birko · 2014-02-17T07:59:05Z

@deeky666:
The main reason I created this pull request was when I wanted to solve this issue for my project https://github.com/l3pp4rd/DoctrineExtensions/issues/980

After a bit self brainstorming I thought it would be better if there was support for all Boolean literals (http://www.postgresql.org/docs/9.3/static/datatype-boolean.html), in case someone would like to use DBAL to parse database data that come not directly from database but in a kind of CSV export. (Like export to csv from pgadmin.)

And since in the Doctrine\DBAL\Types\BooleanType::convertToPHPValue was the $platform parameter I used it to make it more universal too.

@deeky666

reverted @deeky666 suggestion. Since it failed on testBooleanNullConvertsToPHPValue test. with message Failed asserting that true is null. Since I moved this code from BoolenType::convertToPHPValue in the first place, I presume the code there correct.

stof · 2014-03-04T23:56:48Z

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

+    {
+        if ((null !== $item) && 
+            (false !== $item) && 
+            (true !== $item) && 


All these cases can be removed, because they will not in be in the array

@stof I added this additional conditions after one of previous hints here, That I should add them there to avoid unnecessary in_array check

It avoids unnessecary in_array() checks but adds new separate checks in the if statement and makes the code more complicated which reduces readability.

lucasvanlierop · 2014-06-26T17:15:20Z

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

+     *
+     * @param mixed $item
+     *
+     * @return bool


Please add the null as return value also, so: @return bool|null

@return

Updated from @return bool -> @return bool|null since it returns null too

sorry

Ocramius · 2014-06-30T14:50:03Z

Merged in #625

birko added 4 commits February 12, 2014 10:43

Update AbstractPlatform.php

4c7b2b3

Default Boolean conversion behaviour

Update PostgreSqlPlatform.php

9125d0f

added postgresql literals to Boolean conversion

Update BooleanType.php

b45504a

Changed to call platform specific Boolean conversion

Update AbstractPostgreSqlPlatformTestCase.php

ac35c31

added postgresql platform conversion test

birko added 2 commits February 12, 2014 12:31

Update PostgreSqlPlatform.php

f070373

fixed bug when $item was set as bool true value added 'off' literal

Update AbstractPostgreSqlPlatformTestCase.php

a679776

added literals off and on to the testcase

Ocramius reviewed Feb 14, 2014
View reviewed changes

birko added 2 commits February 14, 2014 09:54

Update PostgreSqlPlatform.php

2371e96

Changes after Ocramius comments

Update PostgreSqlPlatform.php

c11b1e5

fixed parameter order in conditions

Update AbstractPlatform.php

578d749

Description fix

Ocramius reviewed Feb 14, 2014
View reviewed changes

birko added 2 commits February 14, 2014 12:16

Update PostgreSqlPlatform.php

e2490b4

Update PostgreSqlPlatform.php

fb8017e

Ocramius reviewed Feb 14, 2014
View reviewed changes

birko added 2 commits February 15, 2014 09:48

Update PostgreSqlPlatform.php

4de9331

updated convert function to include uppercased values

Update AbstractPostgreSqlPlatformTestCase.php

f5036ed

Updated test for uppercased values

deeky666 reviewed Feb 15, 2014
View reviewed changes

birko added 3 commits February 21, 2014 17:11

Update PostgreSqlPlatform.php

1cb2447

Update AbstractPlatform.php

2133f37

Update AbstractPlatform.php

936d52d

reverted @deeky666 suggestion. Since it failed on testBooleanNullConvertsToPHPValue test. with message Failed asserting that true is null. Since I moved this code from BoolenType::convertToPHPValue in the first place, I presume the code there correct.

stof reviewed Mar 4, 2014
View reviewed changes

deeky666 mentioned this pull request Apr 9, 2014

[DBAL-630] Incorrect PostgreSQL boolean handling #564

Closed

lucasvanlierop reviewed Jun 26, 2014
View reviewed changes

lucasvanlierop mentioned this pull request Jun 26, 2014

Fix pg boolean conversion #625

Merged

birko added 2 commits June 30, 2014 08:51

updated return annotation

51842a3

Updated from @return bool -> @return bool|null since it returns null too

just styling fix

fb9c961

sorry

Ocramius closed this Jun 30, 2014

Ocramius self-assigned this Jun 30, 2014

Ocramius mentioned this pull request Mar 2, 2015

PDO, PostgreSQL and bool zendframework/zendframework#7284

Closed

This was referenced Dec 6, 2015

DDC-2180: [GH-527] Fix documentation for Doctrine\ORM\Query\AST\Node::dispatch() doctrine/orm#2873

Closed

DBAL-811: [GH-527] Birko boolean conversion #2044

Closed

GeeH mentioned this pull request Jun 28, 2016

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

Open

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 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Boolean conversion #527

Boolean conversion #527

birko commented Feb 12, 2014

doctrinebot commented Feb 12, 2014

Ocramius Feb 14, 2014

Ocramius Feb 14, 2014

birko commented Feb 14, 2014

Ocramius Feb 14, 2014

birko Feb 14, 2014

birko commented Feb 14, 2014

Ocramius Feb 14, 2014

birko commented Feb 15, 2014

Ocramius commented Feb 15, 2014

birko commented Feb 15, 2014

Ocramius commented Feb 15, 2014

deeky666 Feb 15, 2014

deeky666 commented Feb 15, 2014

birko commented Feb 17, 2014

stof Mar 4, 2014

birko Mar 5, 2014

lucasvanlierop Jun 26, 2014

lucasvanlierop Jun 26, 2014

Ocramius commented Jun 30, 2014

Boolean conversion #527

Boolean conversion #527

Conversation

birko commented Feb 12, 2014

doctrinebot commented Feb 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

birko commented Feb 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

birko commented Feb 14, 2014

Choose a reason for hiding this comment

birko commented Feb 15, 2014

Ocramius commented Feb 15, 2014

birko commented Feb 15, 2014

Ocramius commented Feb 15, 2014

Choose a reason for hiding this comment

deeky666 commented Feb 15, 2014

birko commented Feb 17, 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

Ocramius commented Jun 30, 2014