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 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 commentedYeah... quite some oversight for SQLite.
Comment #4
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 commentedI think we do. Patch attached (it's a big patch).
Comment #6
damien tournoud commentedComment #7
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 commentedComment #21
daffie commented