Posted by Damien Tournoud on September 1, 2011 at 2:08pm
5 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | database system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
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.
Comments
#1
Here is the quick-fix patch.
#2
Seems to look fine for me. This is definitive no api change for d7
#3
Yeah... quite some oversight for SQLite.
#4
Committed 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.
#5
I think we do. Patch attached (it's a big patch).
#6
#7
fetchField() 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.