> On Mar 6, 2026, at 14:21, Xuneng Zhou <[email protected]> wrote:
>
> Hi Yuanzhuo,
>
> Thanks for looking into it.
>
> On Fri, Mar 6, 2026 at 1:48 PM yangyz <[email protected]> wrote:
> I reviewed the patch you submitted and identified two issues.
>
> 1.In the Add pg_stat_recovery system view patch file, the documentation
> modification indicates that the lack of permissions only results in the
> inability
> to view a few specific columns. But the implementation of the code is:
>
> if (! has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
> PG_RETURN_NULL(); If there is no permission, return an empty line. This
> is inconsistent with the written document.
>
> Yeah, this is a mismatch in the patch v3. However, it's been removed by
> Michael in the commit. So it should be fine in HEAD.
> 2.In the function pg_stat_get_recovery(), the two arrays "Datum *values" and
> "bool *nulls" consist of a fixed set of seven elements, so there is no need
> for dynamic allocation.
>
> For a single-row function with ~8–10 columns, saving one palloc is a
> micro-optimization, not a major performance issue. I am not that sure of the
> benefit it brings, still preparing a small patch to turn the palloc into a
> fixed stack array.
> --
> Best,
> Xuneng
Hi Xuneng,
I was reviewing the patch. Obviously, Michael was lightning fast and has
already pushed 0001. I have one small additional comment on pushed 0001.
```
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
```
This uses elog(ERROR), while the other functions in the same file use
ereport(ERROR). I think ereport is generally preferred nowadays over elog. This
is a minor point, so only fix it if you have a chance.
I will send my comments on the remaining two patches separately.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
