Hello,
Drupal has been using PDO for a long time now but as it doesn't have async
support, the project is adding a mysqli driver. A sticky point here is the
lack of support for named placeholders which are widely used in Drupal land
but mysqli doesn't support them. Currently the project is considering
parsing the SQL string in userspace to convert the named placeholders to
positional ones. This does seem like a waste because PDO already contains
battle tested, performant code to do this: pdo_parse_params. Exposing this
to userspace would solve the problem. What do you think? If the maintainers
of the project agree to the idea I can take a stab at coding this. It can't
be too hard (famous last words).
Thanks
Karoly Negyesi
Hi,
I have to say I am not a fan of this proposal. While definitely a super
nice feature in PDO, it's more of a hack rather than proper feature.
Certain RDBMSs support named parameters in prepared statements, but MySQL
doesn't. Therefore, the solution implemented in PDO is a hack. It's very
flawed and the current implementation has multiple bugs and shortcomings.
One could say that some bugs are security issues.
For the above reason, I don't think we should support this in mysqli. We
cannot reliably implement such feature. We could make a copy of
pdo_parse_params and fix as many MySQL issues as possible, but we would
never be able to fix it fully due to the nature of MySQL API.
If someone wants to implement this in userland, then be my guest. As long
as you are aware of the shortcomings and you know how to use it safely then
it's fine.
Unrelated, but I would not consider the async feature of mysqli useful or
even a good reason to abandon PDO. IMHO the async feature is a failed
experiment with limited applicability.
Regards,
Kamil Tekiela
That's rather concerning on both fronts.
Drupal doesn't plan on abandoning PDO, I believe. At least I haven't seen
that proposed.
Is there anything I could help with fixing pdo_parse_params ?
Also what's the problem with mysqli async? With fibers introduced in 8.1 it
seems a very good fit , where can I read / help on problems with this
pairing?
Thanks
Karoly Negyesi
Hi,
I have to say I am not a fan of this proposal. While definitely a super
nice feature in PDO, it's more of a hack rather than proper feature.
Certain RDBMSs support named parameters in prepared statements, but MySQL
doesn't. Therefore, the solution implemented in PDO is a hack. It's very
flawed and the current implementation has multiple bugs and shortcomings.
One could say that some bugs are security issues.For the above reason, I don't think we should support this in mysqli. We
cannot reliably implement such feature. We could make a copy of
pdo_parse_params and fix as many MySQL issues as possible, but we would
never be able to fix it fully due to the nature of MySQL API.If someone wants to implement this in userland, then be my guest. As long
as you are aware of the shortcomings and you know how to use it safely then
it's fine.Unrelated, but I would not consider the async feature of mysqli useful or
even a good reason to abandon PDO. IMHO the async feature is a failed
experiment with limited applicability.Regards,
Kamil Tekiela
That's rather concerning on both fronts.
Drupal doesn't plan on abandoning PDO, I believe. At least I haven't seen
that proposed.Is there anything I could help with fixing pdo_parse_params ?
Also what's the problem with mysqli async? With fibers introduced in 8.1 it
seems a very good fit , where can I read / help on problems with this
pairing?Thanks
Karoly Negyesi
Hi,
I have to say I am not a fan of this proposal. While definitely a super
nice feature in PDO, it's more of a hack rather than proper feature.
Certain RDBMSs support named parameters in prepared statements, but MySQL
doesn't. Therefore, the solution implemented in PDO is a hack. It's very
flawed and the current implementation has multiple bugs and shortcomings.
One could say that some bugs are security issues.For the above reason, I don't think we should support this in mysqli. We
cannot reliably implement such feature. We could make a copy of
pdo_parse_params and fix as many MySQL issues as possible, but we would
never be able to fix it fully due to the nature of MySQL API.If someone wants to implement this in userland, then be my guest. As long
as you are aware of the shortcomings and you know how to use it safely then
it's fine.Unrelated, but I would not consider the async feature of mysqli useful or
even a good reason to abandon PDO. IMHO the async feature is a failed
experiment with limited applicability.Regards,
Kamil Tekiela
There isn't a PHP event loop, so fibers are basically useless in
Drupal, unless Drupal is going to provide their own or use Revolt, or
something. In that case, you pretty much have to change your entire
programming model to work with it and accept the interrupts. It's
funny because fibers were intended to prevent the 'what color is your
function' problem, but instead it became the 'I have no idea what
color my function is' problem. So, just like with async/await, you
have to go all-in or none-at-all, or very carefully craft your stack
to handle multiple layers of fibers. Or maybe I've just been "doing it
wrong" which is certainly possible.
I have to say I am not a fan of this proposal. While definitely a super nice
feature in PDO, it's more of a hack rather than proper feature.
Certain RDBMSs support named parameters in prepared statements, but
MySQL doesn't. Therefore, the solution implemented in PDO is a hack. It's very
flawed and the current implementation has multiple bugs and shortcomings.
One could say that some bugs are security issues.
What is the big risk/challenge in converting named bound parameters to positional bound parameters? It seems straightforward.
-Jeff
Hi Karoly,
Il 11/10/2023 17:03, Karoly Negyesi ha scritto:
Drupal has been using PDO for a long time now but as it doesn't have async
support, the project is adding a mysqli driver. A sticky point here is the
lack of support for named placeholders which are widely used in Drupal land
but mysqli doesn't support them. Currently the project is considering
parsing the SQL string in userspace to convert the named placeholders to
positional ones. This does seem like a waste because PDO already contains
battle tested, performant code to do this: pdo_parse_params. Exposing this
to userspace would solve the problem. What do you think? If the maintainers
of the project agree to the idea I can take a stab at coding this. It can't
be too hard (famous last words).
TBH I'm not very excited to expose PDO (or other) internal functions to
userland.
The pdo_parse_params function works on a PDO statement, so I would
expect you need a PDO statement to call its userland equivalent, unless
some major refactoring takes place.
Also, tangentailly related is the fact that the SQL parser is currently
hardcoded to MySQL syntax[1] and I was planning to improve that. I had a
working PR[2] to allow drivers to have their own SQL parser, but haven't
yet managed to find time to go through the RFC process.
References:
[1] https://externals.io/message/114016
[2] https://github.com/php/php-src/pull/6852
Cheers
Matteo Beccati
Development & Consulting - http://www.beccati.com/