On Wed, Dec 7, 2022 at 7:43 PM Alvaro Herrera <alvherre@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
