On Wed, Dec 7, 2022 at 7:43 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > On 2022-Dec-06, Alvaro Herrera wrote: > > I have pushed this finally. > > > > I made two further changes: > > Actually, I made one further change that I forgot to mention -- I > changed the API of CombineRangeTables once again; the committed patch > has it this way: > > +/* > + * CombineRangeTables > + * Adds the RTEs of 'src_rtable' into 'dst_rtable' > + * > + * This also adds the RTEPermissionInfos of 'src_perminfos' (belonging to the > + * RTEs in 'src_rtable') into *dst_perminfos and also updates perminfoindex > of > + * the RTEs in 'src_rtable' to now point to the perminfos' indexes in > + * *dst_perminfos. > + * > + * Note that this changes both 'dst_rtable' and 'dst_perminfo' destructively, > + * so the caller should have better passed safe-to-modify copies. > + */ > +void > +CombineRangeTables(List **dst_rtable, List **dst_perminfos, > + List *src_rtable, List *src_perminfos) > > The original one had the target rangetable first, then the source > RT+perminfos, and the target perminfos at the end. This seemed > inconsistent and potentially confusing. I also changed the argument > names (from using numbers to "dst/src" monikers) and removed the > behavior of returning the list: ISTM it did turn out to be a bad idea > after all.
This looks better to me too. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com
