From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] binary heap implementation |
Date: | 2012-11-15 15:11:58 |
Message-ID: | CA+TgmoZb2P3=gy+qtpR02GfdsUfJUkDpv2=wz=GF+42oeTerzw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2012-11-15 15:12:14 | Re: [PATCH 02/14] Add support for a generic wal reading facility dubbed XLogReader |
Previous Message | Alvaro Herrera | 2012-11-15 15:10:58 | Re: [v9.3] Extra Daemons (Re: elegant and effective way for running jobs inside a database) |