Re: Gather Merge

Thanks Amit for raising this point. I was not at all aware of mark/restore.
I tried to come up with the case, but haven't found such case.

For now here is the patch with comment update.

Thanks,

On Sun, Feb 19, 2017 at 7:30 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:

> On Sun, Feb 19, 2017 at 2:22 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> > On Sat, Feb 18, 2017 at 6:43 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >> I think there is a value in supporting mark/restore position for any
> >> node which produces sorted results, however, if you don't want to
> >> support it, then I think we should update above comment in the code to
> >> note this fact. Also, you might want to check other places to see if
> >> any similar comment updates are required in case you don't want to
> >> support mark/restore position for GatherMerge.
> >
> > I don't think it makes sense to put mark/support restore into Gather
> > Merge. Maybe somebody else will come up with a test that shows
> > differently, but ISTM that with something like Sort it makes a ton of
> > sense to support mark/restore because the Sort node itself can do it
> > much more cheaply than would be possible with a separate Materialize
> > node. If you added a separate Materialize node, the Sort node would
> > be trying to throw away the exact same data that the Materialize node
> > was trying to accumulate, which is silly.
> >
>
> I am not sure but there might be some cases where adding Materialize
> node on top of Sort node could make sense as we try to cost it as well
> and add it if it turns out to be cheap.
>
> > Here with Gather Merge
> > there is no such thing happening; mark/restore support would require
> > totally new code - probably we would end up shoving the same code that
> > already exists in Materialize into Gather Merge as well.
> >
>
> I have tried to evaluate this based on plans reported by Rushabh and
> didn't find any case where GatherMerge can be beneficial by supporting
> mark/restore in the plans where it is being used (it is not being used
> on the right side of merge join). Now, it is quite possible that it
> might be beneficial at higher scale factors or may be planner has
> ignored such a plan. However, as we are not clear what kind of
> benefits we can get via mark/restore support for GatherMerge, it
> doesn't make much sense to take the trouble of implementing it.
>
> >
> > A comment update is probably a good idea, though.
> >
>
> Agreed.
>
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>

--
Rushabh Lathia