From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: parallelize queries containing initplans |
Date: | 2017-11-10 18:45:25 |
Message-ID: | CA+TgmoYqpxDKn8koHdW8BEKk8FMUL0=e8m2Qe=M+r0UBjr3tuQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Nov 7, 2017 at 4:45 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> As mentioned, changed the status of the patch in CF app.
I spent some time reviewing this patch today and found myself still
quite uncomfortable with the fact that it was adding execution-time
work to track the types of parameters - types that would usually not
even be used. I found the changes to nodeNestLoop.c to be
particularly objectionable, because we could end up doing the work
over and over when it is actually not needed at all, or at most once.
I decided to try instead teaching the planner to keep track of the
types of PARAM_EXEC parameters as they were created, and that seems to
work fine. See 0001, attached.
0002, attached, is my worked-over version of the rest of the patch. I
moved the code that serializes and deserializes PARAM_EXEC from
nodeSubplan.c -- which seemed like a strange choice - to
execParallel.c. I removed the type OID from the serialization format
because there's no reason to do that any more; the worker already
knows the types from the plan. I did some renaming of the functions
involved and some adjustment of the comments to refer to "PARAM_EXEC
parameters" instead of initPlan parameters, because there's no reason
that I can see why this can only work for initPlans. A Gather node on
the inner side of a nested loop doesn't sound like a great idea, but I
think this infrastructure could handle it (though it would need some
more planner work). I broke a lot of long lines in your version of
the patch into multiple lines; please try to be attentive to this
issue when writing patches in general, as it is a bit tedious to go
through and insert line breaks in many places.
Please let me know your thoughts on the attached patches.
Thanks,
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment | Content-Type | Size |
---|---|---|
0001-param-exec-types-v1.patch | application/octet-stream | 12.6 KB |
0002-pq-pushdown-initplan-rebased.patch | application/octet-stream | 26.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-11-10 18:48:18 | Re: Add some const decorations to prototypes |
Previous Message | Graham Leggett | 2017-11-10 18:34:05 | [Patch] Log SSL certificate verification errors |