Skip to content
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

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

Open
CHItA opened this issue Jan 13, 2021 · 4 comments

Comments

@CHItA
Copy link
Contributor

CHItA commented Jan 13, 2021

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.

@morozov
Copy link
Member

morozov commented Jan 23, 2021

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.

@morozov morozov closed this as completed Jan 23, 2021
@morozov morozov reopened this Jan 23, 2021
@morozov
Copy link
Member

morozov commented Jan 23, 2021

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 #3744.

@CHItA
Copy link
Contributor Author

CHItA commented Jan 24, 2021

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.

@morozov
Copy link
Member

morozov commented Jan 25, 2021

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 (Support named parameters in MySQLi connection->prepare() #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants