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.kapil...@gmail.com> wrote: > On Sun, Feb 19, 2017 at 2:22 PM, Robert Haas <robertmh...@gmail.com> > wrote: > > On Sat, Feb 18, 2017 at 6:43 PM, Amit Kapila <amit.kapil...@gmail.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
gather-merge-v8-update-comment.patch
Description: application/download
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
