On Tue, Oct 4, 2022 at 12:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Langote <amitlangote09@gmail.com> writes:
> > On Thu, Jul 28, 2022 at 6:18 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> ... One more thing: maybe we should rethink where to put
> >> extraUpdatedCols. Between the facts that it's not used for
> >> actual permissions checks, and that it's calculated by the
> >> rewriter not parser, it doesn't seem like it really belongs
> >> in RelPermissionInfo. Should we keep it in RangeTblEntry?
> >> Should it go somewhere else entirely? I'm just speculating,
> >> but now is a good time to think about it.
>
> > I've kept extraUpdatedCols in RangeTblEntry in the latest patch, but
> > perhaps it makes sense to put that into Query?
>
> That's got morally the same problem as keeping it in RangeTblEntry:
> those are structures that are built by the parser. Hacking on them
> later isn't terribly clean.
>
> I wonder if it could make sense to postpone calculation of the
> extraUpdatedCols out of the rewriter and into the planner, with
> the idea that it ends up $someplace in the finished plan tree
> but isn't part of the original parsetree.
Looking at PlannerInfo.update_colnos, something that's needed for
execution but not in Query, maybe we can make preprocess_targetlist()
also populate an PlannerInfo.extraUpdatedCols?
> A different aspect of this is that putting it in Query doesn't
> make a lot of sense unless there is only one version of the
> bitmap per Query. In simple UPDATEs that would be true, but
> I think that inherited/partitioned UPDATEs would need one per
> result relation, which is likely the reason it got dumped in
> RangeTblEntry to begin with.
Yeah, so if we have PlannerInfos.extraUpdatedCols as the root table's
version of that, grouping_planner() can make copies for all result
relations and put the list in ModifyTable.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
