From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Letting plpgsql in on the fun with the new expression eval stuff
Date: Wed, 20 Dec 2017 13:13:21 -0500
Message-ID: <24880.1513793601@sss.pgh.pa.us> (raw)
In-Reply-To: <20171220174243.n4y3hgzf7xd3mm5e@alap3.anarazel.de>
References: <32589.1513706441@sss.pgh.pa.us>
<20171220135602.4ixfvaqpeb6z7dil@alap3.anarazel.de>
<27674.1513789968@sss.pgh.pa.us>
<20171220174243.n4y3hgzf7xd3mm5e@alap3.anarazel.de>
Andres Freund <andres@anarazel.de> writes:
> On 2017-12-20 12:12:48 -0500, Tom Lane wrote:
>> I'm using several different test cases, but one that shows up the problem
>> is [...]
> Which certainly seems interesting. The outer ExecInterpExpr() indeed
> doesn't do that much, it's the inner call that's the most relevant
> piece. That so much time is spent in SPI_fnumber() and the functions it
> calls, namely strcmp(), certainly doesn't seem right. I suspect that
> without addressing that cost, a lot of the other potential improvements
> aren't going to mean much.
Right, that's mostly coming from the fact that exec_eval_datum on
a RECFIELD does SPI_fnumber() every time. I have a patch in the
pipeline to address that, but this business with the expression eval
API is a separable issue (and it stands out a lot more with that
patch in place than it does in HEAD ;-)).
>> Um, you left out something here? I don't mind changing the callback
>> signature, but it seems like it generally ought to look the same as
>> the other out-of-line eval functions.
> Yes, I did. Intercontinental travel does wonders.
> I was thinking that it might be better not to expose the exact details
> of the expression evaluation step struct to plpgsql etc. I'm kinda
> forseeing a bit of further churn there that I don't think other parts of
> the code necessarily need to be affected by. We could have the callback
> have a signature roughly like
> Datum callback(void *private_data, ExprContext econtext, bool *isnull);
I don't especially like that, because it forces the callback to use a
separately allocated private_data area even when the available space
in the op step struct would serve perfectly well. In my draft patch
I have
--- 338,352 ----
Oid paramtype; /* OID of parameter's datatype */
} param;
+ /* for EEOP_PARAM_CALLBACK */
+ struct
+ {
+ ExecEvalSubroutine paramfunc; /* add-on evaluation subroutine */
+ void *paramarg; /* private data for same */
+ int paramid; /* numeric ID for parameter */
+ Oid paramtype; /* OID of parameter's datatype */
+ } cparam;
+
/* for EEOP_CASE_TESTVAL/DOMAIN_TESTVAL */
struct
{
and it turns out that plpgsql isn't bothering with paramarg because
paramid and paramtype are all it needs. I doubt that whatever you
have in mind to do to struct ExprEvalStep is likely to be so major
that it changes what we can keep in these fields.
>> I'm not seeing anything that needs to be reset?
> I was kind of thinking of the params_dirty business in
> plpgsql_param_fetch(), setup_param_list() etc. But I'm not quite sure
> how you're thinking of representing parameters on the plpgsql side after
> changing the callbacks style.
Turns out we can just get rid of that junk altogether. I've redesigned
the ParamListInfo API a bit in service of this, and there no longer seems
to be a need for plpgsql to use a mutable ParamListInfo at all.
Will send a patch in a bit. I need to write an explanation of what all
I changed.
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-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, pgsql-hackers@lists.postgresql.org
Subject: Re: Letting plpgsql in on the fun with the new expression eval stuff
In-Reply-To: <24880.1513793601@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