Hi,
> it seems to be a cheap hack
To me, it seems to be within the existing architecture. I agree that the
existing architecture has its flaws.
> What about backtick character ` ? What about NO_BACKSLASH_ESCAPES mode?
These are out of scope. I wasn't aware of the NO_BACKSLASH_ESCAPES but it
indeed seems to be impossible to support on the client side.
> Emulated prepares have a lot of issues and this is only one of them.
This specific issue doesn't seem to be related to the emulation. As far as
I understand, PDO parses SQL as well in order to replace positional
placeholders and vise versa depending on the input and the target platform
capabilities.
> If you want you can see the effect your PR has on this test case
I tried running the example on the stock PHP 8.0.3, and regardless of the
emulation mode and NO_BACKSLASH_ESCAPES mode I saw the same error:
> SQLSTATE[HY093]: Invalid parameter number: number of bound variables does
not match number of tokens in
With my patch, I don't expect to see any changes in parsing MySQL queries
so I don't understand how it's relevant. Am I missing something?
On Sat, Apr 10, 2021 at 3:33 PM Kamil Tekiela <tekiela246@gmail.com> wrote:
> This is an interesting solution to the problem, but I am unsure if it's
> the best one. I didn't look into it in detail yet, but at first glance, it
> seems to be a cheap hack to solve a bug. What about backtick character ` ?
> What about NO_BACKSLASH_ESCAPES mode? Any question mark within
> backtick-escaped column name would still cause a problem. In fact, when I
> tested it with NO_BACKSLASH_ESCAPES the PR didn't fix the problem. I
> got a different error, but the problem still exists.
> This is a naive solution that fixes one tiny bug in a single driver with
> no regards for tons of other issues. I am against adding such hacks into
> the PHP source code.
>
> What we need is a full SQL parser that is context-aware. This task should
> be delegated to the PDO drivers. PDO can't do it reliably at the base
> level. The problem is that PDO is never going to be able to reliably parse
> the SQL even if we have a separate parser for each driver. PDO is not the
> server and it doesn't know how the SQL will be parsed at the end of the
> day. At its root, it seems to be an unfixable problem. Emulated prepares
> have a lot of issues and this is only one of them. The solution is to use
> native prepares whenever they are available on the server.
>
> If you want you can see the effect your PR has on this test case:
>
> $pdo = new \PDO("mysql:host=localhost;dbname=test;charset=utf8mb4",
> 'user', 'password', [
> \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
> \PDO::ATTR_EMULATE_PREPARES => true
> ]);
> $pdo->exec("SET sql_mode = NO_BACKSLASH_ESCAPES;");
> $stmt = $pdo->prepare("SELECT * FROM foo WHERE text='\' and 1=? and
> text='\'");
> $stmt->execute([1]);
>
> I'm sorry but this is a hard pass from me.
>
--
Sergei Morozov
