Re: remaining sql/json patches

I keep looking at 0001, and in the struct definition I think putting the
escontext at the bottom is not great, because there's a comment a few
lines above that says "XXX: following fields only needed during
"compilation"), could be thrown away afterwards".  This comment is not
strictly true, because innermost_caseval is actually used by
array_map(); yet it seems that ->escontext should appear before that
comment.

However, if you put it before steps_len, it would push members steps_len
and steps_alloc beyond the struct's first cache line(*).  If those
struct members are critical for expression init performance, then maybe
it's not a good tradeoff.  I don't know if this was struct laid out
carefully with that consideration in mind or not.

Also, ->escontext's own comment in ExprState seems to be saying too much
and not saying enough.  I would reword it as "For expression nodes that
support soft errors.  NULL if caller wants them thrown instead".  The
shortest I could make so that it fits in a single is "For nodes that can
error softly. NULL if caller wants them thrown", or "For
soft-error-enabled nodes.  NULL if caller wants errors thrown".  Not
sure if those are good enough, or just make the comment the whole four
lines ...


(*) This is what pahole says about the struct as 0001 would put it:

struct ExprState {
    NodeTag                    type;                 /*     0     4 */
    uint8                      flags;                /*     4     1 */
    _Bool                      resnull;              /*     5     1 */

    /* XXX 2 bytes hole, try to pack */

    Datum                      resvalue;             /*     8     8 */
    TupleTableSlot *           resultslot;           /*    16     8 */
    struct ExprEvalStep *      steps;                /*    24     8 */
    ExprStateEvalFunc          evalfunc;             /*    32     8 */
    Expr *                     expr;                 /*    40     8 */
    void *                     evalfunc_private;     /*    48     8 */
    int                        steps_len;            /*    56     4 */
    int                        steps_alloc;          /*    60     4 */
    /* --- cacheline 1 boundary (64 bytes) --- */
    struct PlanState *         parent;               /*    64     8 */
    ParamListInfo              ext_params;           /*    72     8 */
    Datum *                    innermost_caseval;    /*    80     8 */
    _Bool *                    innermost_casenull;   /*    88     8 */
    Datum *                    innermost_domainval;  /*    96     8 */
    _Bool *                    innermost_domainnull; /*   104     8 */
    ErrorSaveContext *         escontext;            /*   112     8 */

    /* size: 120, cachelines: 2, members: 18 */
    /* sum members: 118, holes: 1, sum holes: 2 */
    /* last cacheline: 56 bytes */
};

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)