On Wed, Mar 19, 2014 at 9:28 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> I've pushed one commit with minor fixes, and one with several FIXMEs to
> http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/jsonb_and_hstore
Cool.
> * Jsonb vs JsonbValue is just bad, the latter needs to be renamed, and
> there needs to be a very clear explanation about why two forms exist
> and what each is used for.
Agreed. We should probably emphasize the distinction.
> * The whole datastructure doesn't have any sensible highlevel
> documentation.
I should probably go make some ascii art, per your comment.
> * I don't really like the JsonbSuperHeader name/abstraction. What does
> the "Super" mean? The only interpetation I see is that it's the
> toplevel header (why not Top then?), but JEntry has the comment: "/*
> May be accessed as superheader */"...
What it means is that you can pass a pointer to just past the varlena
Jsonb pointer in some contexts (a VARDATA() pointer), and a pointer
into a jbvBinary buffer in others. The only reason that functions
like findJsonbValueFromSuperHeader() take a super header rather than a
Jsonb pointer is that if they did take a Jsonb*, you'd be making a
representation that the varlena header wasn't garbage, which could not
be ensured in all circumstances, as when the would-be varlena bytes
are something else entirely, or perhaps are even at an address that is
undefined to access. Sites with jbvBinary pointers (e.g. the iterator
code) would have to worry about "faking varlena". That comment is
misleading, and the general idea does need to be explained better.
What you propose for jsonb_typeof() looks incorrect to me. Both of
those JsonbIteratorNext() calls are required, to "get past" the
container pseudo array to get to the underlying single scalar element.
--
Peter Geoghegan
