Assert(!"description of error") is an idiom I've seen more than once, although I'm not sure I understand why it's not written as Robert says with the condition in the brackets (or as a print to STDERR followed by abort() instead).
On Wed, Nov 14, 2012 at 8:11 AM, Abhijit Menon-Sen <ams@2ndquadrant.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
