Re: [HACKERS] Schizophrenic coding in gin_extract_jsonb(_hash)

2014-05-07 Thread Tom Lane
Heikki Linnakangas  writes:
> gin_extract_jsonb recursively extracts all the elements, keys and values 
> of any sub-object too, but JB_ROOT_COUNT only counts the top-level elements.

Got it.  So if the top level is empty, we can exit early, but otherwise we
use its length * 2 as a guess at how big the output will be; which will
be right if it's an object without further substructure, and otherwise
might need enlargement.

> (I hope this is made a bit more clear in the comments I added in the 
> patch I posted this morning)

Didn't read that yet, but will incorporate this info into the jsonb_gin
patch I'm working on.

regards, tom lane


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


Re: [HACKERS] Schizophrenic coding in gin_extract_jsonb(_hash)

2014-05-07 Thread Heikki Linnakangas

On 05/07/2014 06:27 PM, Tom Lane wrote:

I think you're just proving the point that this code is woefully
underdocumented.  If there were, somewhere, some comment explaining
what the heck JB_ROOT_COUNT actually counts, maybe I wouldn't be asking
this question.  jsonb.h is certainly not divulging any such information.


After having reverse-engineered the convertJsonb code, I think I can 
explain what JB_ROOT_COUNT is.


If the root of the Jsonb datum is an array, it's the number of elements 
in that top-level array. If it's an object, it's the number of key/value 
pairs in that top-level object. Some of the elements of that array (or 
values of the object) can be arrays or objects themselves.


gin_extract_jsonb recursively extracts all the elements, keys and values 
of any sub-object too, but JB_ROOT_COUNT only counts the top-level elements.


(I hope this is made a bit more clear in the comments I added in the 
patch I posted this morning)


- Heikki


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


Re: [HACKERS] Schizophrenic coding in gin_extract_jsonb(_hash)

2014-05-07 Thread Tom Lane
Peter Geoghegan  writes:
> On Tue, May 6, 2014 at 8:08 PM, Tom Lane  wrote:
>> The early-exit code path supposes that JB_ROOT_COUNT is absolutely
>> reliable as an indicator that there's nothing in the jsonb value.
>> On the other hand, the realloc logic inside the iteration loop implies
>> that JB_ROOT_COUNT is just an untrustworthy estimate.  Which theory is
>> correct?  And why is there not a comment to be seen anywhere?  If the code
>> is correct then this logic is certainly worthy of a comment or three.

> JsonbIteratorNext() is passed "false" as its skipNested argument. It's
> recursive.

And?

I think you're just proving the point that this code is woefully
underdocumented.  If there were, somewhere, some comment explaining
what the heck JB_ROOT_COUNT actually counts, maybe I wouldn't be asking
this question.  jsonb.h is certainly not divulging any such information.

regards, tom lane


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


Re: [HACKERS] Schizophrenic coding in gin_extract_jsonb(_hash)

2014-05-06 Thread Peter Geoghegan
On Tue, May 6, 2014 at 8:08 PM, Tom Lane  wrote:
> The early-exit code path supposes that JB_ROOT_COUNT is absolutely
> reliable as an indicator that there's nothing in the jsonb value.
> On the other hand, the realloc logic inside the iteration loop implies
> that JB_ROOT_COUNT is just an untrustworthy estimate.  Which theory is
> correct?  And why is there not a comment to be seen anywhere?  If the code
> is correct then this logic is certainly worthy of a comment or three.

JsonbIteratorNext() is passed "false" as its skipNested argument. It's
recursive.

-- 
Peter Geoghegan


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


[HACKERS] Schizophrenic coding in gin_extract_jsonb(_hash)

2014-05-06 Thread Tom Lane
Would someone care to defend this code?

inttotal = 2 * JB_ROOT_COUNT(jb);

...

if (total == 0)
{
*nentries = 0;
PG_RETURN_POINTER(NULL);
}

...

while ((r = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
{
if (i >= total)
{
total *= 2;
entries = (Datum *) repalloc(entries, sizeof(Datum) * total);
}

The early-exit code path supposes that JB_ROOT_COUNT is absolutely
reliable as an indicator that there's nothing in the jsonb value.
On the other hand, the realloc logic inside the iteration loop implies
that JB_ROOT_COUNT is just an untrustworthy estimate.  Which theory is
correct?  And why is there not a comment to be seen anywhere?  If the code
is correct then this logic is certainly worthy of a comment or three.

regards, tom lane


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