Hi,
On 2025-02-20 14:11:16 -0500, Tom Lane wrote:
> As a matter of style, I wonder if it'd be better to have these
> functions write into a caller-supplied variable.
I wondered about that too, it does however make some code more awkward,
e.g. because there's not a good surrounding block to put the variable in.
I mildly prefer the return-by-value approach, but not more.
> That seems more in keeping with most other places in Postgres, and it would
> save a copying step in cases where the caller needs the result on the heap.
I don't think there are many cases where we have to then copy the value to the
heap, fwiw, with a more complete patch.
It's perhaps worth noting that on most (all?) architectures the *caller* will
reserve space for the return value of functions that return structs by value.
In general it'll be easier for the compiler to optimize code where it knows
the lifetime of the relevant memory, which is harder for a function that gets
passed a "target" memory location than when it's returning by value. But of
course it's very hard to believe this ever matters for the case at hand.
The patch curently uses a hardcoded 6 for the length of MAX_BACKENDS. Does
anybody have a good idea for how to either
a) derive 6 from MAX_BACKENDS in a way that can be used in a C array size
b) Use a static assert to check that it fits?
Otherwise I'll just put it in the test that my prior version already
added. Not as good as compile-time, but should still make it easy to find the
issue, if we ever increase/decrease MAX_BACKENDS.
In the attached I
1) changed all the GetRelationPath()/relpath* uses to the new way, the names
now are unchanged
2) introduced a PROCNUMBER_CHARS, instead hardcoding 6 in the length
3) renamed RelPathString to RelPathStr and change the path member to str
I also added a second patch changing _mdfd_segpath() to do something similar,
because it felt somewhat silly to allocate memory after the prior change. Not
entirely sure it's worth it.
Greetings,
Andres Freund
