Deprecate support for unused \PDO::FETCH_* modes

Let's see this - curious to know how much it fails.

Changed to MR workflow, and made use of the new FetchTrait.

Could a CR be written for this?

Change looks good and assuming since nothing broke (test wise) that additional tests aren't needed?

Added draft CR https://www.drupal.org/node/3377999 →

CR LGTM

All code changes look good to me.
The IS and the CR are in order.
For me it is RTBC.

Re #4/#5 there doesn't seem to be any discussion of which modes we should actually support, nor any evidence that the deprecated modes are not used in contrib. It would at least be good to know we aren't entirely disrupting something widely used in contrib.

My intention was to remove also

• \PDO::FETCH_CLASS
• \PDO::FETCH_CLASS | \PDO::FETCH_PROPS_LATE

since it's only used once in core runtime, but trying to remove that stalled at 🐛 View's ResultRow uses deprecated dynamic properties Needs work . So I went for keeping them.

I'm leaving checking contrib for the next person; I'm not very familiar on querying across multiple projects. I tried a bit but ended up in zillions of D7 projects + zillion of dupes of the base classes part of distributions.

I searched contrib and could not find uses of FETCH_DEFAULT, FETCH_LAZY, FETCH_BOUND, FETCH_INTO, FETCH_NAMED, FETCH_FUNC or FETCH_CLASSTYPE.

FETCH_BOTH is used by excel2webpage and w3c_validator.

FETCH_KEY_PAIR is used by gtfs.

I suspect that all these usages could be replaced by a different fetch option and as there are only three I am not particularly worried about this change any longer.

Thanks @longwave! Would you mind sharing your search steps? It would be really useful.

Usage of \PDO::FETCH_KEY_PAIR is problematic with current HEAD because AFAICS it means the module cannot run on SQLite.

I use http://grep.xnddx.ru/ which seems to be updated every few weeks, and has been more reliable for me.

GitLab search is also available, which in this case brings up a different contrib module plus some false positives in vendor code: https://git.drupalcode.org/search?scope=blobs&search=FETCH_KEY_PAIR&grou...

Thanks for #21!

Nothing has changed and #19 sounds like a signoff then I think I can put this back to RTBC.

Haven't reviewed this in enough depth to commit it, but general +1 to reducing the surface area here. I had a quick look at the three usages @longwave found. FETCH_BOTH looks like it's unnecessary and they could switch to more or less any of the standard ones. FETCH_KEY_PAIR would require a bit of refactoring, but reasonable to require that for a major release given it's a single usage.

I'm triaging RTBC issues → .

I looked at this late last night and after reading the IS and comments I don't see anything left to do. There may be a follow up for project gtfs per #19.

Leaving at RTBC.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased and added @phpstan-ignore-next-line to calls of deprecated methods that are left in for BC purposes. Back to RTBC.

This looks great.

As discussed above, there are 3-4 contrib modules affected by the deprecations, and they can probably just switch to a different fetch mode and be fine. Worst case we could undeprecate one or two if there's a really good reason to use them.

mondrake → closed merge request !4468

Automatically closed - issue fixed for 2 weeks with no activity.