Re: [PATCH] binary heap implementation - Mailing list pgsql-hackers

From Will Crawford
Subject Re: [PATCH] binary heap implementation
Date
Msg-id CAJDxst7COoQd0CMdw9GSMePp=66f7msA=QKgnhoD3TLxyGhtHA@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] binary heap implementation  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Assert(!"description of error") is an idiom I've seen more than once, although I'm not sure I understand why it's not
writtenas Robert says with the condition in the brackets (or as a print to STDERR followed by abort() instead).<div
class="gmail_extra"><br/><br /><div class="gmail_quote">On 15 November 2012 15:11, Robert Haas <span dir="ltr"><<a
href="mailto:robertmhaas@gmail.com"target="_blank">robertmhaas@gmail.com</a>></span> wrote:<br /><blockquote
class="gmail_quote"style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Wed, Nov 14, 2012 at 8:11
AM,Abhijit Menon-Sen <<a href="mailto:ams@2ndquadrant.com">ams@2ndquadrant.com</a>> wrote:<br /> > Comments?
Suggestions?<br/><br /> It could use a run through pgindent.  And the header comments are<br /> separated by a blank
linefrom the functions to which they are<br /> attached, which is not project style.<br /><br /> +       if
(heap->size+ 1 == heap->space)<br /> +               Assert("binary heap is full");<br /><br /> This doesn't
reallymake any sense.  Assert("binary heap is full")<br /> will never fail, because that expression is a character
stringwhich<br /> is, therefore, not NULL.  I think you want Assert(heap->size <<br /> heap->space).  Or you
coulduse (heap->size + 1 >= heap->space)<br /> elog(LEVEL, message) if there's some reason this needs to be
checked<br/> in non-debug builds.<br /><br /> +       {<br /> +               sift_down(heap, i);<br /> +       }<br
/><br/> Project style is to omit braces for a single-line body.  This comes up<br /> a few other places as well.<br
/><br/> Other than the coding style issues, I think this looks fine.  If you<br /> can fix that up, I believe I could
goahead and commit this, unless<br /> anyone else objects.<br /><span class="HOEnZb"><font color="#888888"><br /> --<br
/>Robert Haas<br /> EnterpriseDB: <a href="http://www.enterprisedb.com"
target="_blank">http://www.enterprisedb.com</a><br/> The Enterprise PostgreSQL Company<br /></font></span><div
class="HOEnZb"><divclass="h5"><br /><br /> --<br /> Sent via pgsql-hackers mailing list (<a
href="mailto:pgsql-hackers@postgresql.org">pgsql-hackers@postgresql.org</a>)<br/> To make changes to your
subscription:<br/><a href="http://www.postgresql.org/mailpref/pgsql-hackers"
target="_blank">http://www.postgresql.org/mailpref/pgsql-hackers</a><br/></div></div></blockquote></div><br /></div> 

pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: [v9.3] Extra Daemons (Re: elegant and effective way for running jobs inside a database)
Next
From: Heikki Linnakangas
Date:
Subject: Re: [PATCH 02/14] Add support for a generic wal reading facility dubbed XLogReader