Re: Is tuplesort_heap_siftup() a misnomer? - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Is tuplesort_heap_siftup() a misnomer?
Date
Msg-id 24976.1473364006@sss.pgh.pa.us
Whole thread Raw
In response to Re: Is tuplesort_heap_siftup() a misnomer?  (Peter Geoghegan <pg@heroku.com>)
Responses Re: Is tuplesort_heap_siftup() a misnomer?  (Peter Geoghegan <pg@heroku.com>)
List pgsql-hackers
Peter Geoghegan <pg@heroku.com> writes:
> Attached patch does it that way, then. I stuck with the reference to
> "shift down", though, since I think we all agree that that is
> unambiguous.

I dunno.  What you've now got is
/* * The tuple at state->memtuples[0] has been removed from the heap.
- * Decrement memtupcount, and sift up to maintain the heap invariant.
+ * Decrement memtupcount, and shift down to maintain the heap invariant. */static void
-tuplesort_heap_siftup(Tuplesortstate *state, bool checkIndex)
+tuplesort_heap_delete_top(Tuplesortstate *state, bool checkIndex)

I don't find this clearer at all, because

(1) As noted in the comment, the heap top has *already* been removed,
we're just trying to re-establish the heap invariant.  So maybe
"delete_top" isn't the best name after all.

(2) "shift down" seems a bit weird, since we're moving data closer to
the heap top, not further away from it; why isn't that direction "up"?

(3) "shift" makes it sound like it ought to be a simple memmove
kind of operation, which it surely is not.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Corey Huinker
Date:
Subject: Re: Let file_fdw access COPY FROM PROGRAM
Next
From: Vladimir Gordiychuk
Date:
Subject: Re: Stopping logical replication protocol