Re: [PATCH] binary heap implementation

Robert Haas escribió:
> On Wed, Nov 21, 2012 at 9:46 AM, Robert Haas <robertmhaas(at)gmail(dot)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