Hi,
On Fri, Sep 13, 2024 at 07:34:21AM +0900, Michael Paquier wrote:
> On Thu, Sep 12, 2024 at 01:37:52PM +0000, Bertrand Drouvot wrote:
> > There is also some manipulation around the 2 new uint32 fields (objid_hi and
> > objid_lo) in the xactdesc.c and pgstat_xact.c files that look good to me.
>
> Thanks for the reviews. The high and low manipulations are still kind
> of OK to me as a solution for the record constructions.
Agree.
> > But now we end up having functions that accept Oid as parameters to call
> > functions that accept uint64 as parameter (for the exact same parameter),
> > for
> > example:
> >
> > "
> > void
> > pgstat_create_function(Oid proid)
> > {
> > pgstat_create_transactional(PGSTAT_KIND_FUNCTION,
> > MyDatabaseId,
> > proid);
> > }
> > "
> >
> > as pgstat_create_transactional is now:
> >
> > -pgstat_create_transactional(PgStat_Kind kind, Oid dboid, Oid objoid)
> > +pgstat_create_transactional(PgStat_Kind kind, Oid dboid, uint64 objid)
> >
> > That's not an issue as both are unsigned and as we do those calls in that
> > order (Oid -> uint64).
>
> Yes, that's intentional. All the pgstats routines associated to a
> particular object that depends on an OID should still use an OID, and
> anything that's generic enough to be used for all stats kinds had
> better use a uint64.
Yeah, that sounds good to me.
> I was wondering if it would be better hiding
> that behind a dedicated type, but decided to stick with uint64.
That makes sense to me.
Overall, the patch LGTM.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
