Buggy logic in nodeIndexscan.c queue handling
От | Tom Lane |
---|---|
Тема | Buggy logic in nodeIndexscan.c queue handling |
Дата | |
Msg-id | 9273.1432568121@sss.pgh.pa.us обсуждение исходный текст |
Ответы |
Re: Buggy logic in nodeIndexscan.c queue handling
|
Список | pgsql-hackers |
When deciding whether to pop entries from the queue (line 191), the comparison is done against scandesc->xs_orderbyvals, which as the comment states are the last values physically returned by the index. I think this is wrong, or at least pretty inefficient, in the case that the last index tuple was lossy. What will frequently happen is that the comparison value will be too small so we'll not be able to pop the queue item, whereas if we'd compared against the corrected values in node->iss_OrderByValues, we would have popped the item. I don't think this can result in a visible failure, but for sure it can result in unnecessary queue traffic. So ISTM we ought to preserve the state currently represented only in the local variables lastfetched_vals/lastfetched_nulls, and use that in the pop comparison. OTOH, I've not yet consumed any caffeine today so maybe I missed something. One thing that is sadly lacking from this whole patch is any clear specification of the behavior required from the index AM. Is it expected to return items in the exact order implied by their reported (possibly lossy) distance values? Or would it be okay, for instance, to return them in the actually correct final order even though it returns lossy distance values that are then not in order? (And if the latter is disallowed, why?) Without such a specification it's impossible to construct an argument that this queuing mechanism delivers correct answers. regards, tom lane
В списке pgsql-hackers по дате отправления: