Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, Andrew Dunstan <andrew(at)dunslane(dot)net>, David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size
Date: 2022-06-17 00:16:41
Message-ID: 20220617001641.h2xwscyc7z7vg5be@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2022-06-16 16:31:30 -0700, Andres Freund wrote:
> The EEOP_JSONEXPR stuff was added during 15 development in:
>
> commit 1a36bc9dba8eae90963a586d37b6457b32b2fed4
> Author: Andrew Dunstan <andrew(at)dunslane(dot)net>
> Date: 2022-03-03 13:11:14 -0500
>
> SQL/JSON query functions

I'm quite confused about part of the struct definition of this:

struct JsonCoercionsState
{
struct JsonCoercionState
{
JsonCoercion *coercion; /* coercion expression */
ExprState *estate; /* coercion expression state */
} null,
string,
numeric ,
boolean,
date,
time,
timetz,
timestamp,
timestamptz,
composite;
} coercions; /* states for coercion from SQL/JSON item
* types directly to the output type */

Why on earth do we have coercion state for all these different types? That
really adds up:

struct {
JsonExpr * jsexpr; /* 24 8 */
struct {
FmgrInfo func; /* 32 48 */
/* --- cacheline 1 boundary (64 bytes) was 16 bytes ago --- */
Oid typioparam; /* 80 4 */
} input; /* 32 56 */

/* XXX last struct has 4 bytes of padding */

NullableDatum * formatted_expr; /* 88 8 */
NullableDatum * res_expr; /* 96 8 */
NullableDatum * coercion_expr; /* 104 8 */
NullableDatum * pathspec; /* 112 8 */
ExprState * result_expr; /* 120 8 */
/* --- cacheline 2 boundary (128 bytes) --- */
ExprState * default_on_empty; /* 128 8 */
ExprState * default_on_error; /* 136 8 */
List * args; /* 144 8 */
void * cache; /* 152 8 */
struct JsonCoercionsState coercions; /* 160 160 */
} jsonexpr; /* 24 296 */

And why is FmgrInfo stored inline in the struct? Everything else just stores
pointers to FmgrInfo.

Now that I look at this: It's a *bad* idea to have subsidiary ExprState inside
an ExprState. Nearly always the correct thing to do is to build those
expressions. There's plenty memory and evaluation overhead in jumping to a
different expression. And I see no reason for doing it that way here?

This stuff doesn't look ready.

Greetings,

Andres Freund

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2022-06-17 00:43:58 Re: fix stats_fetch_consistency value in postgresql.conf.sample
Previous Message Tom Lane 2022-06-16 23:37:14 Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size