Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On Tue, Sep 14, 2021 at 12:57:47PM -0300, Alvaro Herrera wrote: > The parentheses that commit e3a87b4991cc removed the requirement for are > those that the committed code still has, starting at the errcode() line. > The ones in errmsg() were redundant and have never been necessary. Indeed, thanks! -- Michael signature.asc Description: PGP signature
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On 2021-Sep-14, Michael Paquier wrote: > On Mon, Sep 13, 2021 at 08:51:18AM +0530, Bharath Rupireddy wrote: > > On Mon, Sep 13, 2021 at 8:07 AM Michael Paquier wrote: > >> On Sun, Sep 12, 2021 at 10:14:36PM -0300, Euler Taveira wrote: > >>> On Sun, Sep 12, 2021, at 8:02 PM, Bossart, Nathan wrote: > nitpick: It looks like there's an extra set of parentheses around > errmsg(). > >>> > >>> Indeed. Even the requirement for extra parenthesis around auxiliary > >>> function > >>> calls was removed in v12 (e3a87b4991cc2d00b7a3082abb54c5f12baedfd1). > > Applied. Not using those extra parenthesis is the most common > pattern, so tweaked this way. The parentheses that commit e3a87b4991cc removed the requirement for are those that the committed code still has, starting at the errcode() line. The ones in errmsg() were redundant and have never been necessary. -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/ "El sabio habla porque tiene algo que decir; el tonto, porque tiene que decir algo" (Platon).
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On Mon, Sep 13, 2021 at 08:51:18AM +0530, Bharath Rupireddy wrote: > On Mon, Sep 13, 2021 at 8:07 AM Michael Paquier wrote: >> On Sun, Sep 12, 2021 at 10:14:36PM -0300, Euler Taveira wrote: >>> On Sun, Sep 12, 2021, at 8:02 PM, Bossart, Nathan wrote: nitpick: It looks like there's an extra set of parentheses around errmsg(). >>> >>> Indeed. Even the requirement for extra parenthesis around auxiliary function >>> calls was removed in v12 (e3a87b4991cc2d00b7a3082abb54c5f12baedfd1). Applied. Not using those extra parenthesis is the most common pattern, so tweaked this way. -- Michael signature.asc Description: PGP signature
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On Mon, Sep 13, 2021 at 8:07 AM Michael Paquier wrote: > > On Sun, Sep 12, 2021 at 10:14:36PM -0300, Euler Taveira wrote: > > On Sun, Sep 12, 2021, at 8:02 PM, Bossart, Nathan wrote: > >> nitpick: It looks like there's an extra set of parentheses around > >> errmsg(). > > > > Indeed. Even the requirement for extra parenthesis around auxiliary function > > calls was removed in v12 (e3a87b4991cc2d00b7a3082abb54c5f12baedfd1). > > Yes. The patch makes sense. I am not seeing any other places that > could be grouped, so that looks fine as-is. Thanks all for taking a look at the patch. Here's the CF entry - https://commitfest.postgresql.org/35/3319/ Regards, Bharath Rupireddy.
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On Mon, Sep 13, 2021 at 6:45 AM Euler Taveira wrote: > > On Sun, Sep 12, 2021, at 8:02 PM, Bossart, Nathan wrote: > > nitpick: It looks like there's an extra set of parentheses around > errmsg(). > > Indeed. Even the requirement for extra parenthesis around auxiliary function > calls was removed in v12 (e3a87b4991cc2d00b7a3082abb54c5f12baedfd1). The same commit says that the new code can be written in any way. Having said that, I will leave it to the committer to take a call on whether or not to remove the extra parenthesis. " While new code can be written either way, code intended to be back-patched will need to use extra parens for awhile yet. " Regards, Bharath Rupireddy.
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On Sun, Sep 12, 2021 at 10:14:36PM -0300, Euler Taveira wrote: > On Sun, Sep 12, 2021, at 8:02 PM, Bossart, Nathan wrote: >> nitpick: It looks like there's an extra set of parentheses around >> errmsg(). > > Indeed. Even the requirement for extra parenthesis around auxiliary function > calls was removed in v12 (e3a87b4991cc2d00b7a3082abb54c5f12baedfd1). Yes. The patch makes sense. I am not seeing any other places that could be grouped, so that looks fine as-is. -- Michael signature.asc Description: PGP signature
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On Sun, Sep 12, 2021, at 8:02 PM, Bossart, Nathan wrote: > nitpick: It looks like there's an extra set of parentheses around > errmsg(). Indeed. Even the requirement for extra parenthesis around auxiliary function calls was removed in v12 (e3a87b4991cc2d00b7a3082abb54c5f12baedfd1). -- Euler Taveira EDB https://www.enterprisedb.com/
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On 9/11/21, 1:31 AM, "Bharath Rupireddy"
wrote:
> We have two static check_permissions functions (one in slotfuncs.c
> another in logicalfuncs.c) with the same name and same code for
> checking the privileges for using replication slots. Why can't we have
> a single function CheckReplicationSlotPermissions in slot.c? This way,
> we can get rid of redundant code. Attaching a patch for it.
+1
+/*
+ * Check whether the user has privilege to use replication slots.
+ */
+void
+CheckReplicationSlotPermissions(void)
+{
+ if (!superuser() && !has_rolreplication(GetUserId()))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+(errmsg("must be superuser or replication role
to use replication slots";
+}
nitpick: It looks like there's an extra set of parentheses around
errmsg().
Nathan
Re: Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
On Sat, Sep 11, 2021, at 5:28 AM, Bharath Rupireddy wrote: > We have two static check_permissions functions (one in slotfuncs.c > another in logicalfuncs.c) with the same name and same code for > checking the privileges for using replication slots. Why can't we have > a single function CheckReplicationSlotPermissions in slot.c? This way, > we can get rid of redundant code. Attaching a patch for it. Good catch! Your patch looks good to me. -- Euler Taveira EDB https://www.enterprisedb.com/
Remove duplicate static function check_permissions in slotfuncs.c and logicalfuncs.c
Hi, We have two static check_permissions functions (one in slotfuncs.c another in logicalfuncs.c) with the same name and same code for checking the privileges for using replication slots. Why can't we have a single function CheckReplicationSlotPermissions in slot.c? This way, we can get rid of redundant code. Attaching a patch for it. Thoughts? Regards, Bharath Rupireddy. v1-0001-replication-slots-remove-duplicate-code-for-check.patch Description: Binary data
