From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17800: ON CONFLICT DO UPDATE fails to detect incompatible fields that leads to a server crash
Date: Tue, 21 Feb 2023 15:55:15 -0500
Message-ID: <3995669.1677012915@sss.pgh.pa.us> (raw)
In-Reply-To: <20230221204201.idad3kjv3ue7gb6s@awork3.anarazel.de>
References: <17800-ff90866b3906c964@postgresql.org>
<Y/P49Yov2m+VzfFf@paquier.xyz>
<20230221193352.mvzpuvoujfmckvr3@awork3.anarazel.de>
<3990862.1677010571@sss.pgh.pa.us>
<20230221204201.idad3kjv3ue7gb6s@awork3.anarazel.de>
Andres Freund <andres@anarazel.de> writes:
> On 2023-02-21 15:16:11 -0500, Tom Lane wrote:
>> It occurs to me that one possible fix is to make MULTIEXPR_SUBLINK
>> and the associated output Params use a separate ParamExecData array;
>> instead of the query-wide es_param_exec_vals array, use one that
>> is local to the specific targetlist's ExprState. I'm not sure how
>> much violence that does to the current notion of an ExprState ---
>> do we think that is read-only during execution?
> I don't think you would need to modify ExprState - the information about
> params etc comes from the ExprContext, right? So we'd need to build a
> different ExprContext for partitions, and use that when evaluating the
> expressions.
> ...
> We don't currently have infrastructure for setting
> econtext->ecxt_param_exec_vals to something else, but that shouldn't be too
> hard to add.
No, that won't work, because many usages of PARAM_EXEC Params are
specifically intended to transmit datums from one expression (plan node)
to another. That's why that array was query-global to begin with.
What I'm wondering about is adding a separate array, and likely a separate
ParamKind, that would have a less-than-query-wide scope. We might be able
to get away with having that be plan-node-wide, but making it local to the
specific compiled expression feels safer and easier to reason about.
>> If we did have a local-to-the-expression ParamExecData array, maybe that
>> could be used to get a cleaner fix for things like the domain VALUES
>> and case-test-expression hacks.
> Hm, I'm not quite following along here.
I'm just arm-waving at this point, it's not real clear to me either.
But I do remember that we have some ugly hacks centered around the
fact that domain VALUES and CaseTestExpr are implemented with a single
datum slot per EContext. I'd rather convert them into something like
PARAM_EXEC with no sharing.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, michael@paquier.xyz, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17800: ON CONFLICT DO UPDATE fails to detect incompatible fields that leads to a server crash
In-Reply-To: <3995669.1677012915@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox