Refactor function pg_get_multixact_stats (src/backend/utils/adt/multixactfuncs.c)

Started by Ranier Vilela25 days ago5 messageshackers
Jump to latest
#1Ranier Vilela
ranier.vf@gmail.com

Hi.

pg_get_multixact_stats: avoid computing stats with unprivileged
users

Previously, pg_get_multixact_stats() called GetMultiXactInfo() and
computed the derived "members" and "membersBytes" values
unconditionally, before checking whether the caller holds
privileges of pg_read_all_stats. When the caller lacks those
privileges, the computed values are simply discarded and the
function returns an all-NULL row instead, so the work was wasted on
every unprivileged call.

Move the GetMultiXactInfo() call and the resulting computations
into the branch where the values are actually consumed, narrowing
the scope of the related local variables to that branch. This
matches the usual pattern for privileged stats functions, which
skip the underlying work entirely for callers who can't see the
result, and avoids touching shared MultiXact state when there is no
caller around to observe it.

While here, size the memset() that NULLs the output row off
sizeof(nulls) rather than *sizeof(bool) * tupdesc->natts*.
The two are equivalent today, since nulls is a fixed 4-element array
matching the function's declared return type, but relying the
memset() with *natts* is an unnecessary dependency.
If a column were ever added to the function's SQL definition without a
matching update to this array, the old code would silently write
past the end of nulls.

best regards,
Ranier Vilela

Attachments:

refactor-multixact-stats-function-variables-scope.patchapplication/octet-stream; name=refactor-multixact-stats-function-variables-scope.patchDownload+12-12
#2Michael Paquier
michael@paquier.xyz
In reply to: Ranier Vilela (#1)
Re: Refactor function pg_get_multixact_stats (src/backend/utils/adt/multixactfuncs.c)

On Tue, Jun 30, 2026 at 09:23:20AM -0300, Ranier Vilela wrote:

Move the GetMultiXactInfo() call and the resulting computations
into the branch where the values are actually consumed, narrowing
the scope of the related local variables to that branch. This
matches the usual pattern for privileged stats functions, which
skip the underlying work entirely for callers who can't see the
result, and avoids touching shared MultiXact state when there is no
caller around to observe it.

That's indeed wasteful, so applied as it is my business.

While here, size the memset() that NULLs the output row off
sizeof(nulls) rather than *sizeof(bool) * tupdesc->natts*.
The two are equivalent today, since nulls is a fixed 4-element array
matching the function's declared return type, but relying the
memset() with *natts* is an unnecessary dependency.
If a column were ever added to the function's SQL definition without a
matching update to this array, the old code would silently write
past the end of nulls.

But I left this one out. It's really impossible to miss.
--
Michael

#3Ranier Vilela
ranier.vf@gmail.com
In reply to: Michael Paquier (#2)
Re: Refactor function pg_get_multixact_stats (src/backend/utils/adt/multixactfuncs.c)

Hi Michael.

Em qua., 1 de jul. de 2026 às 00:18, Michael Paquier <michael@paquier.xyz>
escreveu:

On Tue, Jun 30, 2026 at 09:23:20AM -0300, Ranier Vilela wrote:

Move the GetMultiXactInfo() call and the resulting computations
into the branch where the values are actually consumed, narrowing
the scope of the related local variables to that branch. This
matches the usual pattern for privileged stats functions, which
skip the underlying work entirely for callers who can't see the
result, and avoids touching shared MultiXact state when there is no
caller around to observe it.

That's indeed wasteful, so applied as it is my business.

Thanks for the commit.

While here, size the memset() that NULLs the output row off
sizeof(nulls) rather than *sizeof(bool) * tupdesc->natts*.
The two are equivalent today, since nulls is a fixed 4-element array
matching the function's declared return type, but relying the
memset() with *natts* is an unnecessary dependency.
If a column were ever added to the function's SQL definition without a
matching update to this array, the old code would silently write
past the end of nulls.

But I left this one out. It's really impossible to miss.

I can send a separate patch.
What do you think?

best regards,
Ranier Vilela

#4Michael Paquier
michael@paquier.xyz
In reply to: Ranier Vilela (#3)
Re: Refactor function pg_get_multixact_stats (src/backend/utils/adt/multixactfuncs.c)

On Wed, Jul 01, 2026 at 07:44:59AM -0300, Ranier Vilela wrote:

I can send a separate patch.
What do you think?

I think that there is no point in doing such a change.
--
Michael

#5Ranier Vilela
ranier.vf@gmail.com
In reply to: Michael Paquier (#4)
Re: Refactor function pg_get_multixact_stats (src/backend/utils/adt/multixactfuncs.c)

Em qua., 1 de jul. de 2026 às 07:54, Michael Paquier <michael@paquier.xyz>
escreveu:

On Wed, Jul 01, 2026 at 07:44:59AM -0300, Ranier Vilela wrote:

I can send a separate patch.
What do you think?

I think that there is no point in doing such a change.

Thanks, let's leave it at that, then.

best regards,
Ranier Vilela

Show quoted text

--
Michael