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.

Files: 
CommentFileSizeAuthor
#5 1266694-remove-fetch-field.patch212.13 KBDamien Tournoud
PASSED: [[SimpleTest]]: [MySQL] 32,952 pass(es).
[ View ]
#1 1266694-missing-fetchfield.patch1.16 KBDamien Tournoud
PASSED: [[SimpleTest]]: [MySQL] 32,907 pass(es).
[ View ]

Comments

Damien Tournoud’s picture

Status:Active» Needs review
Issue tags:+needs backport to D7
StatusFileSize
new1.16 KB
PASSED: [[SimpleTest]]: [MySQL] 32,907 pass(es).
[ View ]

Here is the quick-fix patch.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Seems to look fine for me. This is definitive no api change for d7

chx’s picture

Yeah... quite some oversight for SQLite.

Dries’s picture

Title:DatabaseStatementPrefetch doesn't implement fetchColumn()» DatabaseStatementPrefetch has redudant fetchColumn() and fetchField()
Priority:Major» Normal
Status:Reviewed & tested by the community» Needs work
Issue tags:-Quick fix, -needs backport to D7

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.

Damien Tournoud’s picture

Status:Needs work» Needs review
StatusFileSize
new212.13 KB
PASSED: [[SimpleTest]]: [MySQL] 32,952 pass(es).
[ View ]

I think we do. Patch attached (it's a big patch).

Damien Tournoud’s picture

Title:DatabaseStatementPrefetch has redudant fetchColumn() and fetchField()» DatabaseStatementInterface has redudant fetchColumn() and fetchField()
Crell’s picture

Status:Needs review» Needs work

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.