Re: Wanted: jsonb on-disk representation documentation - Mailing list pgsql-hackers

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