Skip to content

PDO PostreSQL - parser is broken for '...\' #13958

New issue
New issue
Closed
Closed
PDO PostreSQL - parser is broken for '...\'#13958
Assignees
SakiTakamachi
Labels
BugExtension: pdo_pgsql

Description

@mvorisek
mvorisek
opened on Apr 13, 2024

Description

The following code:

<?php

$pdo = new PDO('pgsql:host=127.0.0.1;dbname=xxx', 'user', 'pass');

$sql = <<<'EOF'
    select :a x, '\' x2
    EOF;
$statement = $pdo->prepare($sql);
$statement->bindValue('a', 'va');

$statement->execute();
$res = $statement->fetchAll(PDO::FETCH_ASSOC);
var_dump($res);

$sql = <<<'EOF'
    select :a x, '\' x2, :b y, '\' y2
    EOF;
$statement = $pdo->prepare($sql);
$statement->bindValue('a', 'va');
$statement->bindValue('b', 'vb');

$statement->execute();
$res = $statement->fetchAll(PDO::FETCH_ASSOC);
var_dump($res);

Resulted in this output:

array(1) {
  [0]=>
  array(2) {
    ["x"]=>
    string(2) "va"
    ["x2"]=>
    string(1) "\"
  }
}

Warning: PDOStatement::bindValue(): SQLSTATE[HY093]: Invalid parameter number: :b in C:\...\repro.php on line 20
array(0) {
}

But I expected this output instead:

array(1) {
  [0]=>
  array(2) {
    ["x"]=>
    string(2) "va"
    ["x2"]=>
    string(1) "\"
  }
}
array(1) {
  [0]=>
  array(4) {
    ["x"]=>
    string(2) "va"
    ["x2"]=>
    string(1) "\"
    ["y"]=>
    string(2) "vb"
    ["y2"]=>
    string(1) "\"
  }
}

'\' is causing the issue - but it is absolutely correct string syntax.

Identifier escaping ("\") is broken as well.

It seems php-src parses the SQL using the old mode (default until PostgreSQL 9.1) - https://www.postgresql.org/docs/current/runtime-config-compatible.html#GUC-STANDARD-CONFORMING-STRINGS.

PHP Version

any (tested 7.4 and 8.3)

Operating System

any (tested Windows and linux)

Activity

  1. mvorisek
    added
    Bug
    Status: Needs Triage
    on Apr 13, 2024
  2. mvorisek
    changed the title [-]PDO_PGSQL - PDOException: SQLSTATE[HY093]: Invalid parameter number: :b[/-] [+]PDO PostreSQL - parser is broken for `'...\'`[/+] on Apr 13, 2024
  3. devnexen
    added
    Feature
    Extension: pdo_pgsql
    and removed
    Bug
    on Apr 14, 2024

  4. mvorisek commented on Apr 14, 2024

    @mvorisek
    mvorisek
    on Apr 14, 2024
    ContributorAuthor

    @devnexen this is definitely a bug, the SQL is parsed somehow by php wrongly.

  5. devnexen commented on Apr 14, 2024

    @devnexen
    devnexen
    on Apr 14, 2024
    Member

    the extension does not support it (AFAIK part of this extension still rely on old postgresql versions behavior), I do not think it qualifies for php 8.2/8.3 though.

  6. mvorisek commented on Apr 14, 2024

    @mvorisek
    mvorisek
    on Apr 14, 2024
    ContributorAuthor

    The relation with PostgreSQL 9.1 is my pure guess, but if true, the version was released more than 12 years ago. So I would say we have a bug in SQL parsing affecting basically all pdo pgsql users.

  7. MorganLOCode commented on Apr 14, 2024

    @MorganLOCode
    MorganLOCode
    on Apr 14, 2024

    Note in contrast that the PostgreSQL extension does parse the string literal correctly:

    $connection = pg_connect('host=127.0.0.1 dbname=xxx user=xxx password=xxx');

$sql = <<<'EOF'
    select $1 x, '\' x2
    EOF;
$statement = pg_prepare($connection, 'q1', $sql);
$result = pg_fetch_all(pg_execute($connection, 'q1', ['va']));

var_dump($result);

$sql = <<<'EOF'
    select $1 x, '\' x2, $2 y, '\' y2
    EOF;
$statement = pg_prepare($connection, 'q2', $sql);
$result = pg_fetch_all(pg_execute($connection, 'q2', ['va', 'vb']));

var_dump($result);

  8. devnexen commented on Apr 14, 2024

    @devnexen
    devnexen
    on Apr 14, 2024
    Member

    pdo pgsql and pgsql are distinct code wise :)

  9. mvorisek commented on Apr 14, 2024

    @mvorisek
    mvorisek
    on Apr 14, 2024
    ContributorAuthor

    It seems the parsing is called in https://github.com/php/php-src/blob/php-8.3.0/ext/pdo_pgsql/pgsql_driver.c#L292 and done in https://github.com/php/php-src/blob/php-8.3.0/ext/pdo/pdo_sql_parser.re#L58 where it assumes MySQL behaviour instead of ANSI.

    here are two correct examples of how it should be done:

  10. MorganLOCode commented on Apr 14, 2024

    @MorganLOCode
    MorganLOCode
    on Apr 14, 2024

    pdo pgsql and pgsql are distinct code wise :)

    Yes, I know. One works correctly (it defers to libpq), the other doesn't.

  11. SakiTakamachi
    added
    Bug
    and removed
    Feature
    on Apr 15, 2024

  12. SakiTakamachi commented on Apr 15, 2024

    @SakiTakamachi
    SakiTakamachi
    on Apr 15, 2024
    Member

    After some testing, I think this might qualify as a bug.

    However, I think this is a bit of a gray area, right on the border between a bug and a feature request.

    As mentioned, this is due to PDO's common query parser not taking into account PostgreSQL-specific behavior.

    I'm not sure how much change will be needed until I work a bit more into this, but I'll get started.

  13. SakiTakamachi
    self-assigned this
    on Apr 15, 2024

  14. MorganLOCode commented on Apr 15, 2024

    @MorganLOCode
    MorganLOCode
    on Apr 15, 2024

    Well, as far as quoting goes, recognising '\' as a string literal containing a single backslash isn't so much PostgreSQL-specific as standard ISO/ANSI behaviour.

  15. mvorisek commented on Apr 16, 2024

    @mvorisek
    mvorisek
    on Apr 16, 2024
    ContributorAuthor

    Thank you for looking into this.

    Here is big testcase string: https://github.com/atk4/data/blob/c2baf07324/tests/Persistence/Sql/WithDb/SelectTest.php#L552-L582

    and testcase for identifiers: https://github.com/atk4/data/blob/c2baf07324/tests/Persistence/Sql/WithDb/SelectTest.php#L622-L648 (plus all cases prefixed /w \\ - https://github.com/atk4/data/blob/c2baf07324/tests/Persistence/Sql/WithDb/SelectTest.php#L675)

  16. mbeccati commented on Apr 17, 2024

    @mbeccati
    mbeccati
    on Apr 17, 2024
    Contributor

    Pls see https://wiki.php.net/rfc/pdo_driver_specific_parsers ;-)

  17. mvorisek commented on Apr 23, 2024

    @mvorisek
    mvorisek
    on Apr 23, 2024
    ContributorAuthor

    Can this be fixed for PHP 8.2/8.3?

  18. mbeccati commented on Apr 23, 2024

    @mbeccati
    mbeccati
    on Apr 23, 2024
    Contributor

    @mvorisek I'm afraid not. The required changes are too deep for anything but the next 8.x version.

  19. devnexen commented on Apr 23, 2024

    @devnexen
    devnexen
    on Apr 23, 2024
    Member

    @mvorisek See why I put the feature label first, it was not to bother you.

  20. mvorisek commented on Apr 23, 2024

    @mvorisek
    mvorisek
    on Apr 23, 2024
    ContributorAuthor

    We ended up with this workaround https://github.com/atk4/data/blob/c2baf07324/src/Persistence/Sql/Postgresql/ExpressionTrait.php#L28-L36

    however native fix would be much faster

    also for identifiers such workaround is not possible, possibly there is no workaround

    and for SQL server we have to workaround https://github.com/atk4/data/blob/c2baf07324/src/Persistence/Sql/Mssql/ExpressionTrait.php#L34-L43 - would that be fixed by #14035? The SQL driver calls the PDO parsing in https://github.com/microsoft/msphpsql/blob/v5.12.0/source/pdo_sqlsrv/pdo_dbh.cpp#L836.

  21. kamil-tekiela commented on Nov 9, 2026

    @kamil-tekiela
    kamil-tekiela
    on Nov 9, 2026
    Member

    I believe this has been solved by PHP 8.4 via #14035.

  22. kamil-tekiela
    closed this as completedon Nov 9, 2026
  23. kamil-tekiela
    removed
    Status: Needs Triage
    on Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

BugExtension: pdo_pgsql

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions