On 17/7/2024 16:33, Matthias van de Meent wrote:
On Wed, 17 Jul 2024 at 05:29, Andrei Lepikhov <lepi...@gmail.com> wrote:
Thanks for the job! I guess you could be more brave and push down also
FILTER statement.
While probably not impossible, I wasn't planning on changing this code
with new optimizations; just expanding the applicability of the
current optimizations.
Got it>> As I see, the code:
aggsortop = fetch_agg_sort_op(aggref->aggfnoid);
if (!OidIsValid(aggsortop))
return false; /* not a MIN/MAX aggregate */
used twice and can be evaluated earlier to avoid duplicated code.
The code is structured like this to make sure we only start accessing
catalogs once we know that all other reasons to bail out from this
optimization indicate we can apply the opimization. You'll notice that
I've tried to put the cheapest checks that only use caller-supplied
information first, and catalog accesses only after that.
If the fetch_agg_sort_op clause would be deduplicated, it would either
increase code complexity to handle both aggref->aggorder paths, or it
would increase the cost of planning MAX(a ORDER BY b) because of the
newly added catalog access.
IMO it looks like a micro optimisation. But I agree, it is more about
code style - let the committer decide what is better.>> Also, I'm unsure
about the necessity of looking through the btree
classes. Maybe just to check the commutator to the sortop, like in the
diff attached? Or could you provide an example to support your approach?
I think it could work, but I'd be hesitant to rely on that, as
commutator registration is optional (useful, but never required for
btree operator classes' operators). Looking at the btree operator
class, which is the definition of sortability in PostgreSQL, seems
more suitable and correct.
Hm, I dubious about that. Can you provide an example which my variant
will not pass but your does that correctly?
--
regards, Andrei Lepikhov
