On 25.01.22 07:14, Greg Nancarrow wrote:

On Tue, Jan 25, 2022 at 7:31 AM Peter Eisentraut <peter.eisentr...@enterprisedb.com <mailto:peter.eisentr...@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.


Reply via email to