Re: [HACKERS] [PATCH] pageinspect function to decode infomasks

On Tue, Sep 17, 2019 at 6:14 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Mon, Sep 16, 2019 at 11:11:06AM -0300, Alvaro Herrera wrote:
> > On 2019-Sep-16, Michael Paquier wrote:
>
> Thanks, fixed.
>
> Amit, what do you think? Does the patch match with what you have in
> mind?
>

*
CREATE FUNCTION heap_tuple_infomask_flags(
t_infomask integer,
t_infomask2 integer,
- decode_combined boolean DEFAULT false)
-RETURNS text[]
+ raw_flags OUT text[],
+ combined_flags OUT text[])
+RETURNS SETOF record

We always return a single tuple/record from this function, so do we
really need to return SETOF record or just returning record is
sufficient?

*
+ pfree(flags);
+ values[0] = PointerGetDatum(a);

- pfree(d);
+ /*
+ * Build set of combined flags. Use the same size as previously for the
+ * allocation, this likely wastes a couple of bytes but it keeps the code
+ * simple.
+ */
+ cnt = 0;
+ flags = (Datum *) palloc0(sizeof(Datum) * bitcnt);

If you want to use the same size array, then you might want to just
memset the previous array rather than first freeing it and then again
allocating it. This is not a big point, so any which way is fine.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com