Thanks Alvaro for taking a look at this.
On Tue, Sep 7, 2021 at 4:35 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Got this warning:
>
> /pgsql/source/master/contrib/postgres_fdw/postgres_fdw.c: In function 'GetResultRelCheckAsUser':
> /pgsql/source/master/contrib/postgres_fdw/postgres_fdw.c:1898:7: warning: unused variable 'result'
[-Wunused-variable]
> Oid result;
> ^~~~~~
Fixed.
> 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.
--
Amit Langote
EDB: http://www.enterprisedb.com