From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, Andrew Dunstan <andrew(at)dunslane(dot)net> |
Subject: | Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size |
Date: | 2022-07-06 00:32:47 |
Message-ID: | 20220706003247.jookwitqhjfjygqd@awork3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2022-06-29 11:40:45 +1200, David Rowley wrote:
> On Sat, 18 Jun 2022 at 08:06, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > I also attached my heavily-WIP patches for the ExprEvalStep issues, I
> > accidentally had only included a small part of the contents of the json fix.
>
> I've now looked at the 0003 patch. I like the idea you have about
> moving some of the additional fields into ScalarArrayOpExprHashTable.
> I think the patch can even go a little further and move the hash_finfo
> into there too. This means we don't need to dereference the "op" in
> saop_element_hash().
Makes sense.
> To make this work, I did need to tag the ScalarArrayOpExpr into the
> ExprEvalStep. That's required now since some of the initialization of
> the hash function fields is delayed until
> ExecEvalHashedScalarArrayOp(). We need to know the
> ScalarArrayOpExpr's hashfuncid and inputcollid.
Makes sense.
> Another small thing which I considered doing was to put the
> hash_fcinfo_data field as the final field in
> ScalarArrayOpExprHashTable so that we could allocate the memory for
> the hash_fcinfo_data in the same allocation as the
> ScalarArrayOpExprHashTable. This would reduce the pointer
> dereferencing done in saop_element_hash() a bit further. I just
> didn't notice anywhere else where we do that for FunctionCallInfo, so
> I resisted doing this.
I think that'd make sense - it does add a bit of size calculation magic, but
it shouldn't be a problem. I'm fairly sure we do this in other parts of the
code.
> (There was also a small bug in your patch where you mistakenly cast to
> an OpExpr instead of ScalarArrayOpExpr when you were fetching the
> inputcollid)
Ooops.
Are you good pushing this? I'm fine with you doing so wether you adapt it
further or not.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | David G. Johnston | 2022-07-06 00:43:19 | Re: doc: Fix description of how the default user name is chosen |
Previous Message | Tom Lane | 2022-07-06 00:20:25 | Re: doc: Fix description of how the default user name is chosen |