DDC-1394: Boolean literals in DQL queries get translated wrongly on PostgreSQL (PDOException) #2015

Closed
doctrinebot opened this Issue Sep 26, 2011 · 11 comments

2 participants

@doctrinebot

Jira issue originally created by user dalvarez:

Using boolean literals in a DQL query results in a PDOException on PostgreSQL, when used together with the equals comparison operator.

Here's an example DQL query involving a boolean literal and an equals comparison operator:

SELECT run
FROM \persistentData\model\core\Run run
WHERE run.isClosed = false
ORDER BY run.timestamp DESC

On PostgreSQL 8.4 this query gets compiled down to:

SELECT r0.timestamp AS timestamp0, r0_.isClosed AS isClosed1, r0_.dbID AS dbID2, r0_.invoiceCreatorResult_dbID AS invoiceCreatorResult_dbID3, r0_.commissionNoteCreatorResult_dbID AS commissionNoteCreatorResult_dbID4, r0_.consumerInvoiceExporterResult_dbID AS consumerInvoiceExporterResultdbID5
FROM Run r0* WHERE r0*.isClosed = 0
ORDER BY r0_.timestamp DESC

When this query gets processed by PostgreSQL, it results in a PDOException, as 0 is not a boolean literal. For boolean literals supported by PostgreSQL, see http://www.postgresql.org/docs/9.1/static/datatype-boolean.html. Interestingly, '0' (meaning the string literal 0) would be a valid boolean literal.

Here is what the log says:

2011-09-27 01:04:03 CEST ERROR: operator does not exist: boolean = integer at character 329
2011-09-27 01:04:03 CEST HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
2011-09-27 01:04:03 CEST STATEMENT: SELECT r0.timestamp AS timestamp0, r0_.isClosed AS isClosed1, r0_.dbID AS dbID2, r0_.invoiceCreatorResult_dbID AS invoiceCreatorResult_dbID3, r0_.commissionNoteCreatorResult_dbID AS commissionNoteCreatorResult_dbID4, r0_.consumerInvoiceExporterResult_dbID AS consumerInvoiceExporterResult_dbID5 FROM Run r0_ WHERE r0_.isClosed = 0 ORDER BY r0.timestamp DESC

Sidenote: For a while I used to wrongly use 0 and 1 as boolean placeholders in DQL, and, running on MySQL 5.0 for a while on customer request, it somehow worked without any problem whatsoever, in spite of the fact that the Doctrine reference documentation requires boolean literals to be either "true" or "false". MySQL silently accepted the wrong code, generally interpreting the boolean datatype as an alias for TINYINT(1). PostgreSQL, however, implements booleans according to the SQL 2003 standard, and correctly complains about a non-safe comparison between a boolean column and an integer literal. So basically, the compilation of boolean literals is generally broken, but on MySQL it simply does not matter, while on PostgreSQL (and possibly other systems as well) it does.

After changing the DQL literal from "0" to "false", as required by the Doctrine reference documentation, I still get the same error, and I can verify (as shown above) that the value "false" in DQL gets compiled to the integer literal "0" in SQL, which it should not.

@doctrinebot

Comment created by @guilhermeblanco:

Is this issue still valid in latest trunk?

Also, it seems PostgreSQL still supports '0', 'false', 0, f. 'off' in 8.4: http://www.postgresql.org/docs/8.4/static/datatype-boolean.html

Cheers,

@doctrinebot

Comment created by dalvarez:

I can not say anything about the latest dev snapshot, but in the 2.1.1 release it is valid. That should make it valid for the 2.1.2 release, too, which AFAIK is a pure security update to release 2.1.1.

The PostgreSQL docs say '0' (string value) is supported, while 0 (integer value) is not, even for Version 8.4. The single quotes are significant.

@doctrinebot

Comment created by @beberlei:

What doctrine type is the "runClosed" column?

The following test here compiles the boolean to 'false':

    /****
     * @group [DDC-1394](http://www.doctrine-project.org/jira/browse/DDC-1394)
     */
    public function testBoolean()
    {
        $bool = new BooleanModel();
        $bool->booleanField = true;

        $this->_em->persist($bool);
        $this->_em->flush();
        $this->_em->clear();

        $dql = "SELECT b FROM Doctrine\Tests\Models\Generic\BooleanModel b WHERE b.booleanField = true";
        $bool = $this->_em->createQuery($dql)->getSingleResult();

        $this->assertTrue($bool->booleanField);

        $bool->booleanField = false;

        $this->_em->flush();
        $this->_em->clear();

        $dql = "SELECT b FROM Doctrine\Tests\Models\Generic\BooleanModel b WHERE b.booleanField = false";
        $query = $this->_em->createQuery($dql);
        var_dump($query->getSQL());
        $bool = $query->getSingleResult();

        $this->assertFalse($bool->booleanField);
    }
string(111) "SELECT b0*.id AS id0, b0_.booleanField AS booleanField1 FROM boolean_model b0_ WHERE b0*.booleanField = 'false'"
@doctrinebot

Comment created by @beberlei:

I think this fixes the problem 772b413

Can you verify?

This is strange, for me this snippet of code encodes correctly (see above).

Can you print your PDO PGSQL Version? (php --info):

Mine is:

pdo_pgsql

PDO Driver for PostgreSQL => enabled
PostgreSQL(libpq) Version => 8.4.8
Module version => 1.0.2
Revision =>  $Id: pdo_pgsql.c 306939 2011-01-01 02:19:59Z felipe $ 
@doctrinebot

Comment created by dalvarez:

Here is the information you requested:

Run.isClosed is declared this way:

   /****
    * @Column(type="boolean")
    */

   protected $isClosed;

And the php --info output regarding pdo_pgsql:

pdo_pgsql

PDO Driver for PostgreSQL => enabled
PostgreSQL(libpq) Version => 8.4.7
Module version => 1.0.2
Revision =>  $Id: pdo_pgsql.c 300351 2010-06-10 12:11:19Z iliaa $

pg_sql

PostgreSQL Support -> enabled
PostgreSQL(libpq) Version => 8.4.7
Multibyte character support => enabled
SSL support => enabled
Active Persistent Links => 0
Active Links => 0

Looking at the fix, there could still be situations where wrapping a boolean literal into quotes could be necessary, e. g. when the representation returned by $this->_conn->getDatabasePlatform()->convertBooleans($bool) is 't'. This would depend on the value returned by convertBooleans(). Then again, this could still be moved into convertBooleans() in cases where it is necessary.

I applied the fix, and again ran the code on PostgreSQL 8.4. It worked.

Now, funnily, after reverting the change to really prove it depended on the change, it still worked. So I am currently unable to reproduce the exact circumstances under which the error happened, and I am unhappy with this. The queries are still the same.

@doctrinebot

Comment created by @beberlei:

The place where guilherme removed the quote was only ever reachable for using the literal constants "true" and "false" in DQL. The convertBoolean methods returns the data correctly already from my first glance.

I would rather know what exactly the problem is myself though. This is not satisfactory otherwise.

@doctrinebot

Comment created by @beberlei:

You are on the latest version aswell, the diff between my pdo_pgsql.c and yours is just the fixed year.

@doctrinebot

Comment created by dalvarez:

I have tried to reproduce the behaviour, but with no success. Without the github fix the boolean literal ends up with quotes in the SQL query, and with the fix it ends up without them, but since both false and 'false' are valid boolean literals, this does not matter to PostgreSQL, and it should be valid both ways.

How boolean literals could have been compiled to numeric literals is beyond me. Regarding the github fix, I do not think it cannot have any influence on this except for quoting values.

I can only speculate that I must have goofed up somewhere while syncing the online code with the IDE, causing old code (with numeric literals in the DQL queries) to get executed,
while the code in the IDE was already updated to use proper boolean literals. This is pure speculation however. Reproducing what exactly happened at the time of reporting the bug is impossible, since I executed the code it in a VM, which was reset several times since then. But since right now I cannot myself reproduce the behaviour in any way, I must assume I messed up.

Feel free to close this as invalid for lack of reproducibility. I am sorry, if I wasted your time.

If I should run into something similiar in the future I'll update this.

@doctrinebot

Comment created by @beberlei:

Closing for now, hopefully for good :-)

@doctrinebot

Issue was closed with resolution "Invalid"

@beberlei beberlei was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment