Robert Haas escribió:
> On Wed, Nov 21, 2012 at 9:46 AM, Robert Haas <robertmh...@gmail.com> wrote:
> > I guess I'll take another whack at it.
> 
> New version attached.

The following comments still talk about "key and value", thus need
an update:

+ * binaryheap_add_unordered
+ *
+ * Adds the given key and value to the end of the heap's list of nodes

+/*
+ * binaryheap_add
+ *
+ * Adds the given key and value to the heap in O(log n), while

+/*
+ * binaryheap_replace_first
+ *
+ * Change the key and/or value of the first (root, topmost) node and


This comment needs updated (s/comparator/compare/, and also add
has_heap_property and arg):

+/*
+ * binaryheap
+ *
+ *     size            how many nodes are currently in "nodes"
+ *     space           how many nodes can be stored in "nodes"
+ *     comparator      comparator to define the heap property
+ *     nodes           the first of a list of "space" nodes
+ */
+typedef struct binaryheap
+{
+   int         size;
+   int         space;
+   bool        has_heap_property;  /* debugging cross-check */
+   binaryheap_comparator compare;
+   void       *arg;
+   Datum       nodes[FLEXIBLE_ARRAY_MEMBER];
+}  binaryheap;


I would suggest to add prefixes to struct members, so bh_size, bh_space
and so on.  This makes it easier to grep for stuff later.

Do we really need the struct definition be public?  Couldn't it just
live in binaryheap.c?

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to