Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
For some reason, we ended up implementing in the database layer a function DatabaseStatementInterface::fetchField()
that is nothing more then an alias of PDOStatement::fetchColumn()
.
Probably as a consequence of that, we forgot to implement DatabaseStatementPrefetch::fetchColumn()
in our user-space implementation of PDOStatement
. Bummer, because we recently started using it in core (in DateTimeFunctionalTest::testDateFormatStorage()
). Let's fix this in DatabaseStatementPrefetch
before considering removing this function altogether in Drupal 8.
Comment | File | Size | Author |
---|---|---|---|
#5 | 1266694-remove-fetch-field.patch | 212.13 KB | Damien Tournoud |
#1 | 1266694-missing-fetchfield.patch | 1.16 KB | Damien Tournoud |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedHere is the quick-fix patch.
Comment #2
dawehnerSeems to look fine for me. This is definitive no api change for d7
Comment #3
chx CreditAttribution: chx commentedYeah... quite some oversight for SQLite.
Comment #4
Dries CreditAttribution: Dries commentedCommitted to 7.x and 8.x. I'm marking it back as 'needs work' so we can figure out if we want to remove it in 8.x.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedI think we do. Patch attached (it's a big patch).
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #7
Crell CreditAttribution: Crell commentedfetchField() is there very deliberately, for DX. fetchColumn() is a badly named method. We have fetchCol(), too, which does something entirely different. Which is which? I wouldn't be able to keep them apart myself, frankly.
fetchField() was added specifically to avoid that confusion. We wanted the "fetch a single field out of every record, that is, the entire column of data" operation. So that got called fetchCol(), we added fetchField() to retrieve an individual field out of a record (basically an alias of fetchColumn()), and although PDO has fetchColumn() nothing in Drupal uses it, quite deliberately.
(Of course, column vs. field is a long-standing question. Sigh.)
So simply removing fetchField() would be a step backward for DX, as well as a non-small API break. I wouldn't support that unless we thought through the other related methods.
Comment #20
daffie CreditAttribution: daffie commentedComment #21
daffie CreditAttribution: daffie commented