dbal icon indicating copy to clipboard operation
dbal copied to clipboard

Supporting both named and positional params in prepared stements in all drivers

Open CHItA opened this issue 4 years ago • 4 comments

Feature Request

Q A
New Feature yes
RFC yes
BC Break no

Summary

Currently it is driver dependent which parametrization is supported in prepared statements per driver when calling prepare() on the connection object. I would suggest adding the parsing already present when calling either executeUpdate() or executeSelect() to prepare as well. This would have the following advantages in my opinion:

  • It would make the supported drivers transparent to the user.
  • The parsing of SQL could be moved to the driver implementation, and there would be no need to parse all statements as it is the case currently with the methods supporting both.
  • Keeping BC

A possible disadvantage could be the overhead, however, this could be avoided for the most part by designing the API in a way that could side step any potential downside. For example, it would be possible to add an optional argument to prepare, let's say, $statementType which could default to native which would keep the current behaviour and would not introduce parsing. On the other hand, users could specify named or positional for that additional argument, and if the driver doesn't support it, it could parse and replace the correct placeholders, if the driver doesn't support both.

All in all, this would add only an extra branch as overhead to users who do not care for this feature, while allowing users, who do not know which driver they will use at runtime to specify what they do expect from the DBAL to do. In my opinion, that is as close to a win-win as anyone can reasonably expect to get when using any abstraction layer.

Alternatively, I would suggest placing the note stating this difference between driver in a more prominent place in the docs.

CHItA avatar Jan 13 '21 20:01 CHItA

Both named and positional parameters are supported at the wrapper level. The driver level only supports the parameters supported by the underlying driver. Please see the details in https://github.com/doctrine/dbal/pull/3891.

morozov avatar Jan 23 '21 20:01 morozov

The parsing of SQL could be moved to the driver implementation, and there would be no need to parse all statements as it is the case currently with the methods supporting both.

This resonates with to https://github.com/doctrine/dbal/issues/3744.

morozov avatar Jan 23 '21 20:01 morozov

Both named and positional parameters are supported at the wrapper level. The driver level only supports the parameters supported by the underlying driver. Please see the details in #3891.

My argument relating to this is that I would have assumed that prepare() is a wrapper level method. It differs from all other methods in that it is the only function which allows for reusing a statement, and this use case is also highlighted in the docs. Currently - my understanding is - one can only use positional parameters with prepare() if the actual driver used at runtime cannot be known. So basically I would consider this issue as a request to support both named and positional parameters at the wrapper level.

Either way, I understand that my use case is probably not the most common one, so feel free to close this if you don't see any value in adding this functionality.

CHItA avatar Jan 24 '21 06:01 CHItA

My argument relating to this is that I would have assumed that prepare() is a wrapper level method. It differs from all other methods in that it is the only function which allows for reusing a statement, and this use case is also highlighted in the docs.

It doesn't sound right. Any driver-level connection can prepare a statement that can be reused multiple times.

So basically I would consider this issue as a request to support both named and positional parameters at the wrapper level.

This is already supported.

Either way, I understand that my use case is probably not the most common one, so feel free to close this if you don't see any value in adding this functionality.

I'm looking at the named/positional parameter support at the driver level from a totally different perspective. The current approach has certain downsides:

  1. As mentioned before (https://github.com/doctrine/dbal/pull/3891), it may cause double parsing and conversion of the statements resulting in a no-op.
  2. At runtime, it's not clear whether a given statement has numeric or positional parameters, which is absolutely ugly from the types standpoint: $params is always something like array<string,T>|array<int,T>.

If the driver level declared explicit APIs for numeric and positional prepared statements, then the above issues wouldn't exist.

morozov avatar Jan 25 '21 04:01 morozov