New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PDO PostreSQL - parser is broken for '...\'
#13958
Comments
'...\'
@devnexen this is definitely a bug, the SQL is parsed somehow by php wrongly. |
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. |
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. |
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); |
pdo pgsql and pgsql are distinct code wise :) |
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: |
Yes, I know. One works correctly (it defers to libpq), the other doesn't. |
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. |
Well, as far as quoting goes, recognising |
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 |
Can this be fixed for PHP 8.2/8.3? |
@mvorisek I'm afraid not. The required changes are too deep for anything but the next 8.x version. |
@mvorisek See why I put the feature label first, it was not to bother you. |
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. |
Description
The following code:
Resulted in this output:
But I expected this output instead:
'\'
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)
The text was updated successfully, but these errors were encountered: