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


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

Reply via email to