Message-ID: <b0ed6a97-ab8c-4a84-ea12-85445dd5e3d7@enterprisedb.com>
Date: Tue, 25 Jan 2022 11:54:29 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.5.0
Subject: Re: PublicationActions - use bit flags.
Content-Language: en-US
To: Greg Nancarrow <gregn4422@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Peter Smith <smithpb2250@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <3b7f2bdd-77ed-00d7-437c-dd9c37e6a2fa@enterprisedb.com>
 <202112201714.ql732mnfdrtc@alvherre.pgsql>
 <CAJcOf-d0T+9jy3ois4eytkX_NPpfWMiSA5a=23_5TXSiU26BEg@mail.gmail.com>
 <2683410.1640048186@sss.pgh.pa.us>
 <CAJcOf-cXspVxTe7pe6QGyCOq6tmftgrxXZ_yRe53mx8L9wqXPw@mail.gmail.com>
 <CAJcOf-d0=vQx1Pzbf+LVarywejJFS5W+M6uR+2d0oeEJ2VQ+Ew@mail.gmail.com>
 <6b8191a0-e0f6-b204-e6c0-f7f1a5f349c0@enterprisedb.com>
 <CAJcOf-eb1wvGCVzMBQH++ubQ3TDwwmqva0uy5w20Y2KUpWkN=A@mail.gmail.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <CAJcOf-eb1wvGCVzMBQH++ubQ3TDwwmqva0uy5w20Y2KUpWkN=A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/b0ed6a97-ab8c-4a84-ea12-85445dd5e3d7%40enterprisedb.com>
Precedence: bulk

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.