Re: [PATCH] binary heap implementation

On Wed, Nov 14, 2012 at 8:11 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> Comments? Suggestions?

It could use a run through pgindent. And the header comments are
separated by a blank line from the functions to which they are
attached, which is not project style.

+ if (heap->size + 1 == heap->space)
+ Assert("binary heap is full");

This doesn't really make any sense. Assert("binary heap is full")
will never fail, because that expression is a character string which
is, therefore, not NULL. I think you want Assert(heap->size <
heap->space). Or you could use (heap->size + 1 >= heap->space)
elog(LEVEL, message) if there's some reason this needs to be checked
in non-debug builds.

+ {
+ sift_down(heap, i);
+ }

Project style is to omit braces for a single-line body. This comes up
a few other places as well.

Other than the coding style issues, I think this looks fine. If you
can fix that up, I believe I could go ahead and commit this, unless
anyone else objects.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company