On Fri, Sep 10, 2021 at 12:22 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Tue, Sep 7, 2021 at 4:35 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > I think the idea that GetRelPermissionInfo always has to scan the
> > complete list by OID is a nonstarter. Maybe it would be possible to
> > store the list index of the PermissionInfo element in the RelOptInfo or
> > the RTE? Maybe use special negative values if unknown (it knows to
> > search the first time) or known non-existant (probably a coding error
> > condition, maybe not necessary to have this)
>
> I implemented this by adding an Index field in RangeTblEntry, because
> GetRelPermissionInfo() is used in all phases of query processing and
> only RTEs exist from start to end. I did have to spend some time
> getting that approach right (get `make check` to pass!), especially to
> ensure that the indexes remain in sync during the merging of
> RelPermissionInfo across subqueries. The comments I wrote around
> GetRelPermissionInfo(), MergeRelPermissionInfos() functions should
> hopefully make things clear. Though, I do have a slightly uneasy
> feeling around the fact that RTEs now store information that is
> computed using some non-trivial logic, whereas most other fields are
> simple catalog state or trivial details extracted from how the query
> is spelled out by the user.
>
> I also noticed that setrefs.c: add_rtes_to_flat_rtable() was still
> doing things -- adding dead subquery RTEs and any RTEs referenced in
> the underlying subquery to flat rtable -- that the new approach of
> permissions handling makes unnecessary. I fixed that oversight in the
> updated patch. A benefit from that simplification is that there is
> now a single loop over rtable in that function rather than two that
> were needed before.
Patch 0002 needed a rebase, because a conflicting change to
expected/rules.out has since been committed.
--
Amit Langote
EDB: http://www.enterprisedb.com