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.This comment and commented out code is just weird...
/**
* Executes the aggregate query.
*
* @return array
* A list of result row arrays. Each result row contains the aggregate
* results as keys and also the groupBy columns as keys:
* @code
* $result = $query
* ->aggregate('nid', 'count')
* ->condition('status', 1)
* ->groupby('type')
* ->executeAggregate();
* @endcode
* Will return:
* @code
* $result[0] = array('count_nid' => 3, 'type' => 'page');
* $result[1] = array('count_nid' => 1, 'type' => 'poll');
* $result[2] = array('count_nid' => 4, 'type' => 'story');
* @endcode
*/
// public function execute();
Proposed change per @alexpott in #3:
So the fix should be to remove reference to executeAggregate in comments and remove reference to poll node type as this no longer exists in core.
The commented out function was fixed in #1944768: Document QueryAggregateInterface::execute
| Comment | File | Size | Author |
|---|---|---|---|
| #28 | 1923816-28.patch | 1.35 KB | ayushmishra206 |
| #20 | 1923816-20.patch | 1.34 KB | quietone |
| #20 | diff-5-20.txt | 433 bytes | quietone |
| #5 | 1923816.doc-cleanup.5.patch | 1.36 KB | alexpott |
| #3 | 1923816.doc-cleanup.2.patch | 1.25 KB | alexpott |
Comments
Comment #1
alexpottPatch to remove code... and I guess this is documentation as it is just removing comments.
Comment #2
chx CreditAttribution: chx commentedNah. We will remove the // when we go 5.3.10
Comment #3
alexpottOk so talking to chx on IRC the reason execute is commented out is caused by https://bugs.php.net/bug.php?id=43200 which is fixed in php 5.3.10 also see #1800122: Bump minimum version of php required to 5.3.10
So the fix should be to remove reference to executeAggregate in comments and remove reference to poll node type as this no longer exists in core.
Comment #4
jhodgdonLet's get the changelog bit out of this patch.
Also, if you are going to fix up this doc block, please fix the indentation. The whole thing with that code example and the following text is part of the @return docs, and it should all be indented two spaces.
Comment #5
alexpottDoh! Sorry for including the changelog file...
I've indented 2 spaces after the @return and indented a further two spaces inside the @code/@endcode as this is the standard elsewhere.
Comment #6
jhodgdonThe two-space indent inside @code is not followed consistently in Core, but it's fine either way. So this patch looks fine to me, and I'll get it committed once it turns green (I realize it's just docs but I sleep better at night if I only commit patches after they turn green. :) ).
Thanks!
Comment #7
chx CreditAttribution: chx commentedI am postponing this on 5.3.10. It's a mere week away, we need to touch this code anyways, why do two when it can be done in one go?
Comment #8
jhodgdonOK, then fixing title and unassigning.
Comment #9
jhodgdonOh, and it won't be just documentation either.
Comment #10
chx CreditAttribution: chx commentedIt will be, the only reason this function exists on the interface is to document it. There won't be any functional changes. It's just that PHP freaks out from this sort of override.
Comment #11
mgiffordThis was postponed for a reason which @chx said expired a year ago (I think).
Comment #20
quietone CreditAttribution: quietone as a volunteer commentedJust needs a reroll.
Comment #21
quietone CreditAttribution: quietone as a volunteer commentedComment #22
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedApplied patch #20 . It works fine.The non-existent function is replaced with actual one
Comment #24
LendudeUpdated the IS to match the fix criteria outlined in #3 and the commented out function was fixed in #1944768: Document QueryAggregateInterface::execute so took that bit out of the title.
Comment #25
LendudeComment #26
catchWhile we're here, I think we should change 'story' to 'article' to match what the standard profile provides.
Comment #27
ayushmishra206 CreditAttribution: ayushmishra206 at OpenSense Labs commentedComment #28
ayushmishra206 CreditAttribution: ayushmishra206 at OpenSense Labs commentedRerolled for 9.2.x. Please review. Also made the change suggested in #26.
Comment #29
LendudeFeedback from #26 addressed, back to RTBC
Comment #31
catchCommitted 1ae9564 and pushed to 9.2.x. Thanks!