Re: Wanted: jsonb on-disk representation documentation

On 2014-05-06 15:45:39 -0700, Peter Geoghegan wrote:
> I don't really know what to say to that. Lots of code in Postgres is
> complicated, especially if you look at one particular function without
> some wider context.
> Is your objection that the complexity is incidental rather than
> essential?

Yes.

> If so, how?

If you think the following is a solution of essential complexity in
*new* code for navigating one level down a relatively simple *new*
datastructure - then we have a disconnect that's larger than I am
willing to argue about.
I can live with the argument that this code is what we have; but calling
this only having the "essential complexity" is absurd.

JsonbValue *
findJsonbValueFromSuperHeader(JsonbSuperHeader sheader, uint32 flags,
uint32 *lowbound, JsonbValue *key)
{
uint32 superheader = *(uint32 *) sheader;
JEntry *array = (JEntry *) (sheader + sizeof(uint32));
int count = (superheader & JB_CMASK);
JsonbValue *result = palloc(sizeof(JsonbValue));

Assert((flags & ~(JB_FARRAY | JB_FOBJECT)) == 0);

if (flags & JB_FARRAY & superheader)
{
char *data = (char *) (array + (superheader & JB_CMASK));
int i;

for (i = 0; i < count; i++)
{
JEntry *e = array + i;

if (JBE_ISNULL(*e) && key->type == jbvNull)
{
result->type = jbvNull;
result->estSize = sizeof(JEntry);
}
else if (JBE_ISSTRING(*e) && key->type == jbvString)
{
result->type = jbvString;
result->val.string.val = data + JBE_OFF(*e);
result->val.string.len = JBE_LEN(*e);
result->estSize = sizeof(JEntry) + result->val.string.len;
}
else if (JBE_ISNUMERIC(*e) && key->type == jbvNumeric)
{
result->type = jbvNumeric;
result->val.numeric = (Numeric) (data + INTALIGN(JBE_OFF(*e)));

result->estSize = 2 * sizeof(JEntry) +
VARSIZE_ANY(result->val.numeric);
}
else if (JBE_ISBOOL(*e) && key->type == jbvBool)
{
result->type = jbvBool;
result->val.boolean = JBE_ISBOOL_TRUE(*e) != 0;
result->estSize = sizeof(JEntry);
}
else
continue;

if (compareJsonbScalarValue(key, result) == 0)
return result;
}
}
else if (flags & JB_FOBJECT & superheader)
{
/* Since this is an object, account for *Pairs* of Jentrys */
char *data = (char *) (array + (superheader & JB_CMASK) * 2);
uint32 stopLow = lowbound ? *lowbound : 0,
stopMiddle;

/* Object key past by caller must be a string */
Assert(key->type == jbvString);
...

I am not calling for a revert. I am just saying that it's imo below
project standards.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services