From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Joseph Adams <joeyadams3(dot)14159(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: patch: Add JSON datatype to PostgreSQL (GSoC, WIP) |
Date: | 2010-07-23 18:47:19 |
Message-ID: | AANLkTi=V6SY_acc22cc5wOUga5wvvDHS7c6d6t63WqbH@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Jul 23, 2010 at 2:34 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> - elog() must be used except for "can't happen" situations. Compare
> the way numeric_in() throws an error message versus json_in().
Er... that should have said "elog() must NOT be used except for can't
happen situations".
Also, I was just looking at json_delete(). While the existing coding
there is kind of fun, I think this can be written more
straightforwardly by doing something like this (not tested):
while (1)
{
while (is_internal(node) && node->v.children.head)
node = node->v.children.head;
if (node->next)
next = node->next;
else if (node->parent)
next = node->parent;
else
break;
free_node(node);
node = next;
}
That gets rid of all of the gotos and one of the local variables and,
at least IMO, is easier to understand... though it would be even
better still if you also added a comment saying something like "We do
a depth-first, left-to-right traversal of the tree, freeing nodes as
we go. We need not bother clearing any of the pointers, because the
traversal order is such that we're never in danger of revisiting a
node we've already freed."
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company
From | Date | Subject | |
---|---|---|---|
Next Message | Joshua D. Drake | 2010-07-23 18:48:50 | permission inconsistency with functions |
Previous Message | Robert Haas | 2010-07-23 18:34:28 | Re: patch: Add JSON datatype to PostgreSQL (GSoC, WIP) |