Open ncalexan opened 6 years ago
I wonder if there are additional wrinkles handling
[:find
?e
:where
[?e :mentions ?x]
:order
(count ?x)]
That is, handling situations where the aggregate we're sorting by isn't projected at all. That might not be valid (for reasons I don't comprehend right now), or it might make it difficult to follow @rnewman's approach above.
The existing ordering code handles ordering by non-projected variables already:
mentat=> .explain_query [:find ?person :where [?person :person/pet ?p] :order ?p]
SQL: SELECT DISTINCT `datoms00`.e AS `?person`, `datoms00`.v AS `?p` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 65538 ORDER BY `?p` ASC
Plan: select id | order | from | detail
0|0|0|SEARCH TABLE datoms AS datoms00 USING COVERING INDEX idx_datoms_aevt (a=?)
0|0|0|USE TEMP B-TREE FOR ORDER BY
You'll see it makes sure to project ?p
in the SQL query so it can be used for ordering.
(There's perhaps a separate bug there, which is that ?p
is always a ref, so it's not super meaningful to order, but we'll ignore that for now!)
Right now, it's not possible to order by aggregates, so that queries like
are not possible. (They don't parse, due to
:order
expecting bare variables.)@rnewman suggests this is simply not implemented, and says: the obvious solution is to take each
:order
that’s a valid aggregate expression and push it into the projector, just as we do now with order-vars. It would perhaps be worthwhile to make sure thatgenerates
rather than mentioning
sum
twice. Otherwise this should mostly be a matter of extending the parser and altering some type choices in the projector/ordering hookup, which already exists.