Problem/Motivation
In #3345938: Deprecate support for unused \PDO::FETCH_* modes, we normalized the fetch modes used in Drupal's database operations. However, we fell short of introducing our own logic to indicate the fetch mode, and kept using \PDO::FETCH* constants for the purpose.
This issue is to define fetch modes independently from PDO so that non-PDO db libraries can implement their statement objects without needing to deal with constants that are not in their scope (thinking of the mysqli driver, mostly).
Steps to reproduce
Proposed resolution
- Introduce an enumeration to define Drupal's supported fetch modes
- Replace \PDO::FETCH_* constants with the enumeration where used
- Introduce a trait that introspects calls to PDO methods so that Drupal's statement objects can be abstract from the client library in use (and allow swapping them in contrib drivers)
- Deprecate usage of \PDO::FETCH_* constants in method calls outside of the above trait
Remaining tasks
User interface changes
Introduced terminology
API changes
The use of the \PDO::FETCH_* constants is deprecated and replaced with the new enum Drupal\Core\Database\Statement\FetchAs values:
- Drupal\Core\Database\Statement\FetchAs::Associative (\PDO::FETCH_ASSOC)
- Drupal\Core\Database\Statement\FetchAs::ClassObject (\PDO::FETCH_CLASS | \PDO::FETCH_PROPS_LATE)
- Drupal\Core\Database\Statement\FetchAs::Column (\PDO::FETCH_COLUMN)
- Drupal\Core\Database\Statement\FetchAs::List (\PDO::FETCH_NUM)
- Drupal\Core\Database\Statement\FetchAs::Object (\PDO::FETCH_OBJ)
Data model changes
Release notes snippet
Issue fork drupal-3487851
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mondrakeComment #4
mondrakeComment #5
mondrakeComment #6
daffie commentedFor the change record and the questions on the MR.
Comment #7
mondrakeComment #8
mondrakeFollow up: #3488467: Introduce a StatementBase abstract class, to create a common base class for statement objects.
Comment #9
daffie commentedThe CI pipeline is not happy.
Comment #10
mondrakeJuggled a bit with typehints.
Comment #11
daffie commentedAll the code change good to me.
I have updated the CR and the IS.
The CI pipeline is green for all by core supported databases.
All my questions have been answered.
For me it is RTBC.
Comment #12
mondrakeThanks for review @daffie!
Comment #13
mondrakeI'm still trying to do some tuning with the typehints in PdoTrait - to make them as accurate and PHPStan-proof as possible. It may turn out that CodeSniffer will be unhappy, in which case I think we should silence its failures.
Comment #14
mondrakeChanges to return typehints were OK for CodeSniffer, good - but now this needs review again I suppose, sorry @daffie
Comment #15
mondrakeStricter typing is highlighting a bug. On that.
Comment #16
mondrakeReviewable again.
Comment #17
daffie commentedAll looks good to me.
Back to RTBC.
Comment #18
mondrakeFound a glitch while working on the follow up.
Comment #19
daffie commentedChange look good to me.
Back to RTBC.
Comment #20
mondrakeRebased after commit of #3489538: StatementPrefetchIterator::fetchField fails silently if the column index is out of range. I swapped responsability between ::fetchColumn() and ::fetchField() in StatementPrefetchIterator, very minor but I think this requires to be set back to NR. Sorry.
Comment #21
daffie commentedChange look good to me.
Back to RTBC.
Comment #22
mondrakeRebased after #3490200: StatementPrefetchIterator::fetchCol fails silently if the column index is out of range
Comment #24
longwaveCommitted feff767 and pushed to 11.x. Thanks!
Also published the change record.
Comment #27
cafuego commentedIt would be quite nice if the Drupal phpcs rules complained about the use of PDO:: contants in (custom) code.
And/or add support to use PDO::FETCH_GROUP via FetchAs. That one seems to be missing and now Drupal complains about an unsupported fetch mode that PHP supports just fine.
Comment #29
rob230 commentedWhy wasn't
FETCH_GROUPadded as an option? Is there a way to replicate that behaviour as it was in D10?Edit: found issue #3546916: Enhance database fetch mode to support PDO::FETCH_GROUP and combination with bitwise operator for dealing with that.