Re: moving extraUpdatedCols out of RangeTblEntry (into ModifyTable)

On Thu, Jan 5, 2023 at 4:59 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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