On 25.01.22 07:14, Greg Nancarrow wrote:
>
> On Tue, Jan 25, 2022 at 7:31 AM Peter Eisentraut
> <peter.eisentraut@enterprisedb.com
> <mailto:peter.eisentraut@enterprisedb.com>> wrote:
> >
> > Why can't GetRelationPublicationActions() have the PublicationActions as
> > a return value, instead of changing it to an output argument?
>
> That would be OK too, for now, for the current (small size, typically
> 4-byte) PublicationActions struct.
> But if that function was extended in the future to return more
> publication information than just the PublicationActions struct (and I'm
> seeing that in the filtering patches [1]), then using return-by-value
> won't be as efficient as pass-by-reference, and I'd tend to stick with
> pass-by-reference in that case.
>
> [1]
> https://postgr.es/m/OS0PR01MB5716B899A66D2997EF28A1B3945F9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
> <https://postgr.es/m/OS0PR01MB5716B899A66D2997EF28A1B3945F9%40OS0PR01MB5716.jpnprd01.prod.outlook.com>
By itself, this refactoring doesn't seem worth it. The code is actually
longer at the end, and we haven't made it any more extensible or
anything. And AFAICT, this is not called in a performance-sensitive way.
The proposed changes in [1] change this function more significantly, so
adopting the present change wouldn't really help there either except
create the need for one more rebase.
So I think we should leave this alone here and let [1] make the changes
it needs.