In the current KNNGIST patch, the indexable ORDER BY clauses are
transmitted to the executor by cramming them in with the index qual
conditions (the IndexScan plan node's indexqual list), from whence
they become part of the ScanKey array passed to the index AM.
Robert complained that this was an ingenious way to minimize the
number of lines touched by the patch but utterly ugly from any other
standpoint, and I quite agree. An ORDER BY clause is a completely
different thing from a WHERE qual, so mixing them together doesn't
seem like a good idea.
However, if we hold to that principle then we need to modify the indexAM
API to pass the ordering operators separately. This is no big deal as
far as the built-in AMs are concerned, particularly because 3 of the 4
need only assert that the additional list is empty. The only reason it
would be a problem is if there were third-party index AMs that would be
affected to a larger degree; but I don't know of any. Does anyone have
an objection to that?
(Another thing that might be worth changing, as long as we have to touch
the beginscan and rescan APIs anyway, is to refactor the handling of
the initial set of scan keys. It never made any sense to me for
RelationGetIndexScan to call index_rescan: that seems to accomplish
little except making it difficult for AM beginscan routines to do things
in a sane order. I'm inclined to take that out and let the AM call
rescan internally if it wants to.)
Lastly, I'm pretty un-thrilled with the way that the KNNGIST patch
implements the interface to the opclass-specific hook functions.
Seems like it would be cleaner to leave the Consistent function alone
and invent a new, separate hook function for processing ORDER BY.
Is there a strong reason for having both things done in one call,
or was that just done as a byproduct of trying to cram all the data
into one ScanKey array?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
