Thread: make Gather node projection-capable
The Gather node, as currently committed, is neither projection-capable nor listed as an exception in is_projection_capable_plan. Amit discovered this in testing, and I hit it in my testing as well. We could just mark it as being not projection-capable, but I think it might be better to go the other way and give it projection capabilities. Otherwise, we're going to start generating lots of plans like this: Result -> Gather -> Partial Seq Scan While that's not the end of the world, it seems to needlessly fly in the face of the general principle that nodes should generally try to support projection. So attached is a patch to make Gather projection-capable (gather-project.patch). It has a slight dependency on my patch to fix up the tqueue machinery for record types, so I've attached that patch here as well (tqueue-record-types.patch). Comments? Reviews? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
Robert Haas <robertmhaas@gmail.com> writes: > The Gather node, as currently committed, is neither projection-capable > nor listed as an exception in is_projection_capable_plan. Amit > discovered this in testing, and I hit it in my testing as well. We > could just mark it as being not projection-capable, but I think it > might be better to go the other way and give it projection > capabilities. Um ... why would you not want the projections to happen in the child nodes, where they could be parallelized? Or am I missing something? > While that's not the end of the world, it seems to needlessly fly in > the face of the general principle that nodes should generally try to > support projection. I'm not sure there is any such principle. regards, tom lane
On Thu, Oct 22, 2015 at 1:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> The Gather node, as currently committed, is neither projection-capable >> nor listed as an exception in is_projection_capable_plan. Amit >> discovered this in testing, and I hit it in my testing as well. We >> could just mark it as being not projection-capable, but I think it >> might be better to go the other way and give it projection >> capabilities. > > Um ... why would you not want the projections to happen in the child > nodes, where they could be parallelized? Or am I missing something? You probably would, but sometimes that might not be possible; for example, the tlist might contain a parallel-restricted function (which therefore has to run in the leader). >> While that's not the end of the world, it seems to needlessly fly in >> the face of the general principle that nodes should generally try to >> support projection. > > I'm not sure there is any such principle. I just inferred that this was the principle from reading the code; it doesn't seem to be documented anywhere. In fact, what projection actually means doesn't seem to be documented anywhere. Feel free to set me straight. That having been said, I hope there's SOME principle other than "whatever we happened to implement". All of our scan and join nodes seem to have projection capability - I assume that's not an accident. It would simplify the executor code if we ripped all of that out and instead had a separate Project node (or used Result), but for some reason we have not. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Thu, Oct 22, 2015 at 1:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Robert Haas <robertmhaas@gmail.com> writes: > >> The Gather node, as currently committed, is neither projection-capable > >> nor listed as an exception in is_projection_capable_plan. Amit > >> discovered this in testing, and I hit it in my testing as well. We > >> could just mark it as being not projection-capable, but I think it > >> might be better to go the other way and give it projection > >> capabilities. > > > > Um ... why would you not want the projections to happen in the child > > nodes, where they could be parallelized? Or am I missing something? > > You probably would, but sometimes that might not be possible; for > example, the tlist might contain a parallel-restricted function (which > therefore has to run in the leader). I don't understand your reply. Failing to parallelize the child node does not prevent it from doing the projection itself, does it? That said, I don't understand Tom's comment either. Surely the planner is going to choose to do the projection in the innermost node possible, so that the children nodes are going to do the projections most of the time. But if for whatever reason this fails to happen, wouldn't it make more sense to do it at Gather than having to put a Result on top? > >> While that's not the end of the world, it seems to needlessly fly in > >> the face of the general principle that nodes should generally try to > >> support projection. > > > > I'm not sure there is any such principle. > > I just inferred that this was the principle from reading the code; it > doesn't seem to be documented anywhere. In fact, what projection > actually means doesn't seem to be documented anywhere. Feel free to > set me straight. That having been said, I hope there's SOME principle > other than "whatever we happened to implement". All of our scan and > join nodes seem to have projection capability - I assume that's not > an accident. It would simplify the executor code if we ripped all of > that out and instead had a separate Project node (or used Result), but > for some reason we have not. Projections are a weird construct as implemented, yeah. I imagine it's because of performance reasons, because having separate Result nodes everywhere would be a lot slower, wouldn't it? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Oct 22, 2015 at 3:18 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >> You probably would, but sometimes that might not be possible; for >> example, the tlist might contain a parallel-restricted function (which >> therefore has to run in the leader). > > I don't understand your reply. Failing to parallelize the child node > does not prevent it from doing the projection itself, does it? OK, now I'm confused. If you can't parallelize the child, there's no Gather node, and this discussion is irrelevant. The case that matters is something like: SELECT pg_backend_pid(), a, b, c FROM foo WHERE x = 1; What happens today is that the system first produces a SeqScan plan with an all-Var tlist, which may either be a physical tlist or the exact columns needed. Then, after join planning, which is trivial here, we substitute for that tlist the output tlist of the plan. Since SeqScan is projection-capable, we just emit a single-node plan tree: SeqScan. But say we choose a PartialSeqScan plan. The tlist can't be pushed down because pg_backend_pid() must run in the leader. So, if Gather can't do projection, we'll end up with Result->Gather->PartialSeqScan. If we make Gather projection-capable, we can just end up with Gather->PartialSeqScan. I prefer the second outcome. TBH, the major reason is that I've just been experimenting with injecting single-copy Gather nodes into Plan trees above the join nest and below any upper plan nodes that get stuck on top and seeing which regression tests fail. Since we do a lot of EXPLAIN-ing in the plan, I hacked EXPLAIN not to show the Gather nodes. But it does show the extra Result nodes which get generated because Gather isn't projection-capable, and that causes a huge pile of spurious test failures. Even with the patch I posted applies, there are some residual failures that don't look simple to resolve, because sometimes an initplan or subplan attaches to the Gather node since something is being projected there. So if you just have EXPLAIN look through those nodes and show their children instead, you still don't get the same plans you would without the test code in all cases, but it helps a lot. > That said, I don't understand Tom's comment either. Surely the planner > is going to choose to do the projection in the innermost node possible, > so that the children nodes are going to do the projections most of the > time. But if for whatever reason this fails to happen, wouldn't it make > more sense to do it at Gather than having to put a Result on top? The planner doesn't seem to choose to do projection in the innermost node possible. The final tlist only gets projected at the top of the join tree. Beneath that, it seems like we project in order to avoid carrying Vars through nodes where that would be a needless expense, but that's just dropping columns, not computing anything. That having been said, I don't think that takes anything away from your chain of reasoning here, and I agree with your conclusion. There seems to be little reason to force a Result node atop a Gather node when we don't do that for most other node types. Also, the patch I'm proposing I think actually makes things quite a bit cleaner than the status quo, because the current code initializes the projection info and then ignores the projection info itself, while using the slot that gets set up by initializing the projection info. That just seems goofy. If there's some reason not to do what I'm proposing here, I'm happy to do whatever we agree is better, but I don't think leaving it the way it is now makes any sense. > Projections are a weird construct as implemented, yeah. I imagine it's > because of performance reasons, because having separate Result nodes > everywhere would be a lot slower, wouldn't it? I'm not sure. It'd be my guess that this is why it wasn't made a separate node to begin with, but I don't know. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Oct 22, 2015 at 2:49 PM, Robert Haas <robertmhaas@gmail.com> wrote: > You probably would, but sometimes that might not be possible; for > example, the tlist might contain a parallel-restricted function (which > therefore has to run in the leader). Oh, also: pushing down the tlist is actually sorta non-trivial at the moment. I can stick a GatherPath on top of whatever the join planner kicks out (although I don't have a cost model for this yet, so right now it's just a hard-coded test), but the upper planner substitutes the tlist into whatever the topmost plan node is - and that's the Gather, not whatever's under it. Maybe I should have a special case for this: if the node into which we would insert the final tlist is a Gather, see whether it's parallel-safe, and if so, peel the Gather node off, apply the tlist to whatever's left (adding a gating Result node if need be) and put the Gather back on. This seems less important than a few other things I need to get done, but certainly worth doing. But do you know whether the upper planner path-ifaction work will be likely to render whatever code I might write here obsolete? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 22 October 2015 at 16:01, Robert Haas <robertmhaas@gmail.com> wrote:
--
If we make Gather projection-capable,
we can just end up with Gather->PartialSeqScan.
Is there a reason not to do projection in the Gather node? I don't see one.
> That said, I don't understand Tom's comment either. Surely the planner
> is going to choose to do the projection in the innermost node possible,
> so that the children nodes are going to do the projections most of the
> time. But if for whatever reason this fails to happen, wouldn't it make
> more sense to do it at Gather than having to put a Result on top?
The planner doesn't seem to choose to do projection in the innermost
node possible. The final tlist only gets projected at the top of the
join tree. Beneath that, it seems like we project in order to avoid
carrying Vars through nodes where that would be a needless expense,
but that's just dropping columns, not computing anything. That having
been said, I don't think that takes anything away from your chain of
reasoning here, and I agree with your conclusion. There seems to be
little reason to force a Result node atop a Gather node when we don't
do that for most other node types.
Presumably this is a performance issue then? If we are calculating something *after* a join which increases rows then the query will be slower than need be.
I agree the rule should be to project as early as possible.
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sun, Oct 25, 2015 at 11:59 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > On 22 October 2015 at 16:01, Robert Haas <robertmhaas@gmail.com> wrote: >> If we make Gather projection-capable, >> we can just end up with Gather->PartialSeqScan. > > Is there a reason not to do projection in the Gather node? I don't see one. I don't see one either. There may be some work that needs to be done to get the projection to happen in the Gather node in all of the cases where we want it to happen in the Gather node, but that's not an argument against having the capability. >> > That said, I don't understand Tom's comment either. Surely the planner >> > is going to choose to do the projection in the innermost node possible, >> > so that the children nodes are going to do the projections most of the >> > time. But if for whatever reason this fails to happen, wouldn't it make >> > more sense to do it at Gather than having to put a Result on top? >> >> The planner doesn't seem to choose to do projection in the innermost >> node possible. The final tlist only gets projected at the top of the >> join tree. Beneath that, it seems like we project in order to avoid >> carrying Vars through nodes where that would be a needless expense, >> but that's just dropping columns, not computing anything. That having >> been said, I don't think that takes anything away from your chain of >> reasoning here, and I agree with your conclusion. There seems to be >> little reason to force a Result node atop a Gather node when we don't >> do that for most other node types. > > Presumably this is a performance issue then? If we are calculating something > *after* a join which increases rows then the query will be slower than need > be. I don't think there will be a performance issue in most cases because in most cases the node immediately beneath the Gather node will be able to do projection, which in most cases is in fact better, because then the work gets done in the workers. However, there may be some cases where it is useful. After having mulled it over, I think it's likely that the reason why we didn't introduce a separate node for projection is that you generally want to project to remove unnecessary columns at the earliest stage that doesn't lose performance. So if we didn't have projection capabilities built into the individual nodes, then you'd end up with things like Aggregate -> Project -> Join -> Project -> Scan, which would start to get silly, and likely inefficient. > I agree the rule should be to project as early as possible. Cool. I'm not sure Tom was really disagreeing with the idea of making Gather projection-capable ... it seems like he may have just been saying that there wasn't as much of a rule as I was alleging. Which is fine: we can decide what is best here, and I still think this is it. Barring further objections, I'm going to commit this, because (1) the status quo is definitely weird because Gather is abusing the projection stuff to come up with an extra slot, so doing thing seems unappealing and (2) I need to make other changes that touch the same areas of the code, and I want to get this stuff done quickly so that we get a user-visible feature people can test without writing C code in the near future. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company