Documentation for QueryAggregateInterface::sortAggregate inadequate

There is a test that demonstrates the usage in:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/...

It's hard to understand exactly what you are trying to attempt here without more information. Do you have the code you used in step 3 try those guesses.

Given elapsed time and the above I'm going to mark Closed (cannot reproduce). Feel free to reopen.

Yes, here's the code. I thought I had made it clear in the original issue description what string I tried for identifying the values to be used for the sort.

$storage = \Drupal::entityTypeManager()->getStorage('ebms_packet');
$query = $storage->getAggregateQuery()->accessCheck(FALSE);
$query->condition('active', 1);
$query->exists('articles.entity.reviews');
$field = 'articles.entity.reviews.entity.posted';
$alias = 'updated';
$query->aggregate($field, 'MAX', NULL, $alias);
$query->sortAggregate($field, 'MAX', 'DESC');
$query->groupBy('id');
foreach ($query->execute() as $values) {
    echo "{$values['id']}: {$values['updated']}\n";                             
}                                                                               

And when I run vendor/bin/drush scr /var/www/bug-repros repro-3282364 I get

In ExceptionHandler.php line 52:
SQLSTATE[42000]: Syntax error or access violation: 1046 You have an error in your SQL syntax ...

If I comment out the sortAggregate() line, the query completes successfully, but of course the results aren't sorted correctly.

Even if you're saying that we can't expect the documentation to explain how to use the APIs correctly, and are instead supposed to dig into test code to glean the missing information (and I hope that's not what you're saying), the test code to which you linked handles the easy case of entities with primitive fields, but doesn't have anything for the more common case of custom entity types with structured values using entity references. That's why (as I explained in the original issue description) that I had to fall back on investigating methods whose documentation wasn't so sparse. Unfortunately the condition() method and the sortAggregate() method don't use the same syntax for identifying a structured field.

Here's an illustrative example for the structure of the custom entities involved:

> \Drupal\ebms_review\Entity\Packet::load(17687)
= Drupal\ebms_review\Entity\Packet {#7156
    translationLanguages: "und",
    id (integer): "17687",
    topic (entity_reference): "Topic::load(18)",
    created_by (entity_reference): "User::load(400)",
    created (datetime): "2019-12-31 08:12:16",
    title (string): "Childhood Acute Lymphoblastic Leukemia (January 2020)",
    articles (entity_reference) x2: "PacketArticle::load(63), PacketArticle::load(104)",
    reviewers (entity_reference) x6: "User::load(142), User::load(156), User::load(165), User::load(170), User::load(192), User::load(340)",
    last_seen (datetime): "2020-02-22 12:43:43",
    active (boolean): "1",
    starred (boolean): "0",
    summaries (entity_reference): "Doc::load(8573)",
    reviewer_docs (entity_reference): [],
  }

> \Drupal\ebms_review\Entity\PacketArticle::load(63)
= Drupal\ebms_review\Entity\PacketArticle {#6153
    translationLanguages: "und",
    id (integer): "63",
    article (entity_reference): "Article::load(650479)",
    dropped (boolean): "0",
    archived (datetime): [],
    reviews (entity_reference) x3: "Review::load(69422), Review::load(69570), Review::load(69382)",
  }

> \Drupal\ebms_review\Entity\Review::load(69422)
= Drupal\ebms_review\Entity\Review {#7399
    translationLanguages: "und",
    id (integer): "69422",
    reviewer (entity_reference): "User::load(142)",
    posted (datetime): "2020-01-23 16:34:01",
    comments (text_long): [
      [
        "value" => null,
        "format" => null,
      ],
    ],
    loe_info (string_long): [],
    dispositions (entity_reference): "Term::load(98)",
    reasons (entity_reference): [],
  }

If you still need more information, please ask. And let's use the "needs more information" status instead of "can't reproduce" in that case.

Thanks for clarifying ( I was just filling in some time by trying to close out some old stale issues.)

I couldn't reproduce your steps above to run the command and see the error messages at which point I figured that unless there was still interest in this there was probably better things for me to be doing with my evening like sleep! :)

Thanks for the additional information.

Just to help reproduce what you are doing above.

You are referencing ebms_packet - are you able to reproduce the workflow you are trying to explain with something from Drupal Core? Or is this workflow dependent on having a custom entity?

For example could we set up a entity reference field on a node and try and do the same thing?

We have millions of entities in this application, but only a small number of those are instances of a core type, and none of those are used in an aggregation query. Let me see what I can come up with either using just core types (or custom types created in the repro code).

OK, I've got a script you should be able to run yourself. The SQL error message is not the same, complaining about an unknown column instead of a syntax error (likely because my original repro case had multiple levels of nested entity references, whereas this has a single level), but I believe it adequately illustrates what this issue is reporting, namely that the documentation does not provide enough information to know exactly what the method expects for the first argument in the general case. The code is attached (inside a zipfile, as this interface won't allow me to post a PHP script directly).

root@8fca212939df:/var/www# vendor/bin/drush scr --script-path=/var/www/bug-repros repro-3282364-2 
created tag with ID 206
created tag with ID 207
created tag with ID 208
created article with ID 52
created article with ID 53
52: Tag for bug 3282364 (1)
53: Tag for bug 3282364 (3)
failure: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'field_tags.entity.name_max' in 'order clause': SELECT "node_field_data_2"."nid" AS "nid", MAX("taxonomy_term_field_data"."name") AS "name", MAX("taxonomy_term_field_data_2"."name") AS "field_tagsentityname_max"
FROM
"node" "base_table"
LEFT JOIN "node__field_tags" "node__field_tags" ON "node__field_tags"."entity_id" = "base_table"."nid"
LEFT OUTER JOIN "taxonomy_term_data" "taxonomy_term_data" ON "taxonomy_term_data"."tid" = "node__field_tags"."field_tags_target_id"
LEFT JOIN "taxonomy_term_field_data" "taxonomy_term_field_data" ON "taxonomy_term_field_data"."tid" = "taxonomy_term_data"."tid"
LEFT OUTER JOIN "taxonomy_term_data" "taxonomy_term_data_2" ON "taxonomy_term_data_2"."tid" = "node__field_tags"."field_tags_target_id"
LEFT JOIN "taxonomy_term_field_data" "taxonomy_term_field_data_2" ON "taxonomy_term_field_data_2"."tid" = "taxonomy_term_data_2"."tid"
INNER JOIN "node_field_data" "node_field_data" ON "node_field_data"."nid" = "base_table"."nid"
INNER JOIN "node__field_tags" "node__field_tags_2" ON "node__field_tags_2"."entity_id" = "base_table"."nid"
LEFT JOIN "node_field_data" "node_field_data_2" ON "node_field_data_2"."nid" = "base_table"."nid"
WHERE ("node_field_data"."status" = :db_condition_placeholder_0) AND ("node__field_tags_2"."field_tags_target_id" IS NOT NULL)
GROUP BY "node_field_data_2"."nid"
ORDER BY "field_tags"."entity"."name_max" DESC; Array
(
    [:db_condition_placeholder_0] => 1
)

cleaning up term 206
cleaning up term 207
cleaning up term 208
cleaning up article 52
cleaning up article 53

Well that was strange. As you can see from the screenshot, I add attached the repro code file before submitting my previous comment, but the issue tracker discarded it. Trying again.

Seem like the issue tracker is itself having issues. I tried editing a typo in my previous comment and I got a server error message (see second attached screenshot). Second attempts for each failure worked. 🤔

I'm wondering if this is actually not possible! or a bug.

We can search some of contrib -> https://git.drupalcode.org/search?group_id=2&scope=blobs&search=sortAggr... but I'm not convinced that any of these examples are trying to do what you are doing.

First from looking at the code I don't think condition is a good example of what the syntax might be.

web/core/lib/Drupal/Core/Entity/Query/QueryBase.php

We see that aggregation was added -> https://www.drupal.org/project/drupal/issues/1854708 →
And it looks like they chose option 3.

So it would suggest we should be able to use the alias but this generates an error.

If we look at what the code does:

It first generates an alias by appending _MAX onto the end of the field name.
Then stores the result indexed by that alias in $this->sortAggregate
And finally adds an aggregate using the passed in field, function and langcode.

So the syntax must align with what aggregate is using.
However when we do this we get an error because the order by ends up on: 'EntityOne'.'EntityTwo'.'field_aggregration_function'
where as the generated column is aliased as 'EntityOneEntityTwofield_aggregrationfunction'

So if you agree with the above I think we should shift this off documentation to be a bug report against the code base?

Sure. I have switched the component to "entity system" (don't see an option for "entity query" so I assume it's lumped in with entities in general). I'll edit the issue description. If what you say is true (i.e., the API is broken), then there will be two "remaining tasks" instead of just one:

1. Finish the documentation
2. Fix the code

In our own software, that's the order in which we like to do things. If you fully document what the code is supposed to do, it's much easier to write the code, as well as know when the code is correct. 😉

I don't think condition is a good example of what the syntax might be.

Well, narrowing a query by the value of a field, aggregating the values of a field, and sorting based on the aggregated values of a field all share the requirement that the programmer has to answer the same question: "Which field?", right? If one follows the age-old principle of not multiplying entities beyond necessity, it would seem reasonable to use the same syntax for meeting this requirement. It's possible that there is some valid reason why that won't work here, and that a different syntax is needed for at least one of these actions, but I'd want to see compelling arguments to that effect before I'd be convinced. In any case, the required syntax needs to be documented, and that would be even more true if the surprising decision was made to use different syntax for these similar operations.

Do you agree?