Re: ExecRTCheckPerms() and many prunable partitions

On Wed, Nov 30, 2022 at 11:56 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Wed, Nov 30, 2022 at 3:04 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > On 2022-Nov-29, Amit Langote wrote:
> >
> > > Maybe, we should have the following there so that the PlannedStmt's
> > > contents don't point into the Query?
> > >
> > > newperminfo = copyObject(perminfo);
> >
> > Hmm, I suppose if we want a separate RTEPermissionInfo node, we should
> > instead do GetRTEPermissionInfo(rte) followed by
> > AddRTEPermissionInfo(newrte) and avoid the somewhat cowboy-ish coding
> > there.
>
> OK, something like the attached?

Thinking more about the patch I sent, which has this:

+ /* Get the existing one from this query's rtepermlist. */
perminfo = GetRTEPermissionInfo(rtepermlist, newrte);
- glob->finalrtepermlist = lappend(glob->finalrtepermlist, perminfo);
- newrte->perminfoindex = list_length(glob->finalrtepermlist);
+
+ /*
+ * Add a new one to finalrtepermlist and copy the contents of the
+ * existing one into it. Note that AddRTEPermissionInfo() also
+ * updates newrte->perminfoindex to point to newperminfo in
+ * finalrtepermlist.
+ */
+ newrte->perminfoindex = 0; /* expected by AddRTEPermissionInfo() */
+ newperminfo = AddRTEPermissionInfo(&glob->finalrtepermlist, newrte);
+ memcpy(newperminfo, perminfo, sizeof(RTEPermissionInfo));

Note that simple memcpy'ing would lead to the selectedCols, etc.
bitmapsets being shared between the Query and the PlannedStmt, which
may be considered as not good. But maybe that's fine, because the
same is true for RangeTblEntry members that do have substructure such
as the various Alias fields that are not reset? Code paths that like
to keep a PlannedStmt to be decoupled from the corresponding Query,
such as plancache.c, do copy the former, so shared sub-structure in
the default case may be fine after all.

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com