On Thu, Jan 5, 2023 at 4:59 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > I wrote: > > After further thought: maybe we should get radical and postpone this > > work all the way to executor startup. The downside of that is having > > to do it over again on each execution of a prepared plan. But the > > upside is that when the UPDATE targets a many-partitioned table, > > we would have a chance at not doing the work at all for partitions > > that get pruned at runtime. I'm not sure if that win would emerge > > immediately or if we still have executor work to do to manage pruning > > of the target table. I'm also not sure that this'd be a net win > > overall. But it seems worth considering. > > Here's a draft patch that does it like that. This seems like a win > for more reasons than just pruning, because I was able to integrate > the calculation into runtime setup of the expressions, so that we > aren't doing an extra stringToNode() on them.
Thanks for the patch. This looks pretty neat and I agree that this seems like a net win overall. As an aside, I wonder why AttrDefault (and other things in RelationData that need stringToNode() done on them to put into a Query or a plan tree) doesn't store the expression Node tree to begin with? > There's still a code path that does such a calculation at plan time > (get_rel_all_updated_cols), but it's only used by postgres_fdw which > has some other rather-inefficient behaviors in the same area. > > I've not looked into what it'd take to back-patch this. We can't > add a field to ResultRelInfo in released branches (cf 4b3e37993), > but we might be able to repurpose RangeTblEntry.extraUpdatedCols. I think we can make that work. Would you like me to give that a try? -- Thanks, Amit Langote EDB: http://www.enterprisedb.com
