> On 17 Jul 2024, at 23:32, Nathan Bossart <nathandboss...@gmail.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
