From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: optimizing pg_upgrade's once-in-each-database steps |
Date: | 2024-07-18 07:57:23 |
Message-ID: | 4493D323-9726-48F7-A702-E71FB991F76B@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 17 Jul 2024, at 23:32, Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> On Wed, Jul 17, 2024 at 11:16:59PM +0200, Daniel Gustafsson wrote:
>> +static void
>> +dispatch_query(const ClusterInfo *cluster, AsyncSlot *slot,
>> ....
>> + pg_free(query);
>> +}
>>
>> A minor point, perhaps fueled by me not having played around much with this
>> patchset. It seems a bit odd that dispatch_query is responsible for freeing
>> the query from the get_query callback. I would have expected the output from
>> AsyncTaskGetQueryCB to be stored in AsyncTask and released by async_task_free.
>
> I don't see any problem with doing it the way you suggest.
>
> Tangentially related, I waffled a bit on whether to require the query
> callbacks to put the result in allocated memory. Some queries are the same
> no matter what, and some require customization at runtime. As you can see,
> I ended up just requiring allocated memory. That makes the code a tad
> simpler, and I doubt the extra work is noticeable.
I absolutely agree with this.
>> + char *query = pg_malloc(QUERY_ALLOC);
>>
>> Should we convert this to a PQExpBuffer?
>
> Seems like a good idea. I think I was trying to change as few lines as
> possible for my proof-of-concept. :)
Yeah, that's a good approach, I just noticed it while reading the hunks. We
can do that separately from this patchset.
In order to trim down the size of the patchset I think going ahead with 0003
independently of this makes sense, it has value by itself.
--
Daniel Gustafsson
From | Date | Subject | |
---|---|---|---|
Next Message | Alexander Lakhin | 2024-07-18 08:00:00 | Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop() |
Previous Message | Andy Fan | 2024-07-18 07:55:42 | Re: Optimize WindowAgg's use of tuplestores |