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 <tekiela...@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