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

PDO::FETCH_KEY_PAIR Missing from FetchMode Constants #3111

Closed
chrisguitarguy opened this issue Apr 18, 2018 · 17 comments
Closed

PDO::FETCH_KEY_PAIR Missing from FetchMode Constants #3111

chrisguitarguy opened this issue Apr 18, 2018 · 17 comments

Comments

@chrisguitarguy
Copy link
Contributor

There's no constant in Doctrine\DBAL\FetchMode for PDO::FETCH_KEY_PAIR.

Was this intentional? Could it be added? Happy to send a PR for this, but wasn't sure if the omission was intentional.

See also #3082 where FETCH_KEY_PAIR was also mentioned.

@morozov
Copy link
Member

morozov commented Apr 18, 2018

@chrisguitarguy it is intentional because this mode is only supported in PDO but not other underlying drivers. We may want to implement it across platforms in 3.0 but it requires some refactoring of the driver layer first.

@Majkl578
Copy link
Contributor

You can still use the constant in 2.x though and it should work as long as you use PDO-based driver. Note that it'll emit deprecation warning now in 2.x since it's not explicitly supported a fetch mode across drivers.
See #2996 + #2958.

Marking as feature request for 3.0 where it may eventually become a supported FetchMode.

@chrisguitarguy
Copy link
Contributor Author

it is intentional because this mode is only supported in PDO but not other underlying drivers.

Ah, I gotcha. Didn't think about non-PDO drivers.

For what it's worth, I "fixed" my own deprecation warning by just doing FetchMode::ASSOCIATIVE and using array_column to pull out what I needed. That may be a solution for other drivers, but DBAL probably doesn't have enough info about the query to do that without parsing SQL I'd guess.

@morozov
Copy link
Member

morozov commented Apr 18, 2018

@chrisguitarguy you probably can use FetchMode::NUMERIC and the first two columns instead without depending on the column names.

@chrisguitarguy
Copy link
Contributor Author

Looked into this a bit more and I think I pretty much understand how it would get implemented. PDO stuff just works, but things like the mysqli, oci, and sql server drivers would need a supported added in their {DriverName}Statement classes.

Is that on the right track?

I've only every fixed stuff in DBAL for postgres and mysql, are there docs on how to run the OCI or other tests anywhere?

@morozov
Copy link
Member

morozov commented Apr 20, 2018

@chrisguitarguy, please hold off on implementing it. While it's technically possible, currently, there's no proper place to put the implementation. If it's implemented on the Driver\Statement level, it will have to be duplicated for every non-PDO driver (same as currently FetchMode::STANDARD_OBJECT and FetchMode::CUSTOM_OBJECT are).

As for testing on OCI, your options are:

  1. Don't bother. We have OCI covered by ContinuousPHP builds.
  2. Use wnameless/oracle-xe-11g as a server. You can find some instructions (for instance, this article) on building php_oci8 online.

@thomask
Copy link

thomask commented May 8, 2018

Would this be the proper place to also add that PDO::FETCH_UNIQUE is "missing"? If not, I can open a separate issue.

@morozov
Copy link
Member

morozov commented May 8, 2018

@thomask let's leave it here for now.

@dave-redfern
Copy link

I guess these changes mean that PDO::FETCH_PROPS_LATE would be difficult to implement across drivers?

@morozov
Copy link
Member

morozov commented Aug 21, 2018

@dave-redfern the implementation itself is not difficult. The point is that we don't want to reimplement it across drivers, we want things like that to be implementable once on the wrapper level. But it requires API redesign.

@Philipp91
Copy link

This seems to be the thread for missing fetch modes :-)

+1 for PDO::FETCH_UNIQUE. However, it's not trivial, as it's usually not used alone. It can reasonably be combined with FETCH_ASSOC, FETCH_CLASS and so on. So would the new FetchMode enum also allow such combinations?

Besides those mentioned above, I also often use \PDO::FETCH_UNIQUE | \PDO::FETCH_COLUMN to retrieve a single value, e.g. a sum or sth like that. In my opinion, that's more reasonably expressed as FetchMode::SINGLE_VALUE, as there's not really any concept of columns and uniqueness when there's only one cell.

And +1 for \PDO::FETCH_KEY_PAIR, which is closely related to FETCH_UNIQUE, I'd say.


In fact, another way to look at this is to split off the decision whether everything should be keyed by the first column or not, which I'd find more intuitive as an API user, though it's conceptually different from PHP's PDO API. For the "value" side, there's things like ASSOCIATIVE, NUMERIC, MIXED, STANDARD_OBJECT, etc. What's currently known as COLUMN could be renamed to SINGLE_COLUMN for clarity. And for the "key" side, there's REGULAR (or ENUMERATED or just 0 mode) for the usual numerically indexed array, then there's KEYED for an array indexed by the first returned column (similar to current FETCH_UNIQUE), and then there's SINGLE_ROW which asserts that there is only row in the result and returns the plain value (not an array of values, though of course the value itself is an array in the case of ASSOCIATIVE).

So FETCH_KEY_PAIR would be KEYED + SINGLE_COLUMN. My \PDO::FETCH_UNIQUE | \PDO::FETCH_COLUMN case above would become SINGLE_ROW + SINGLE_COLUMN. And so on.

@Ocramius
Copy link
Member

Ocramius commented Oct 1, 2018

I don't have my computer at hand, so I can't find the original thread for this, but we are actually trying to reduce the supported fetch modes, as they are simple function composition patterns complicated by bitmasking, and they are trivial to implement in userland.

@Philipp91
Copy link

So when I'm using plain PDO in PHP, is there no performance benefit when I'm using FETCH_KEY_PAIR compared to just fetchin ASSOC and manually converting the result into an array mapping keys to values?

@Ocramius
Copy link
Member

Ocramius commented Oct 1, 2018

Generally not noticeable: still, I wouldn't rely on the underlying PDO layer to arrange data fir you, as the various PDO implementations are extremely quirky. Seriously, the less PDO-specific details you use, the better.

@jwage
Copy link
Member

jwage commented Apr 17, 2019

I am re-evaluating all issues currently associated with the doctrine/dbal 3.0.0 milestone. Removing the milestone for now on this issue while we try to narrow in on the most critical issues required to release 3.0.0. This issue is still important but it may not happen in 3.0.0

@jwage jwage removed this from the 3.0.0 milestone Apr 17, 2019
@morozov
Copy link
Member

morozov commented Jul 26, 2021

Fixed in #4293.

@morozov morozov closed this as completed Jul 26, 2021
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants