Hi,

On 2015-08-12 16:46:01 -0400, Stephen Frost wrote:
> From ab44660e9b8fc5b10f84ce6ba99a8340047ac8a5 Mon Sep 17 00:00:00 2001
> From: Stephen Frost <sfr...@snowman.net>
> Date: Wed, 12 Aug 2015 15:50:54 -0400
> Subject: [PATCH] In AlterRole, make bypassrls an int
> 
> When reworking bypassrls in AlterRole to operate the same way the other
> attribute handling is done, I missed that the variable was incorrectly a
> bool rather than an int.  This meant that on platforms with an unsigned
> char, we could end up with incorrect behavior during ALTER ROLE.
> 
> Pointed out by Andres thanks to tests he did changing our bool to be the
> one from stdbool.h which showed this and a number of other issues.
> 
> Add regression tests to test CREATE/ALTER role for the various role
> attributes.  Arrange to leave roles behind for testing pg_dumpall, but
> none which have the LOGIN attribute.
> 
> In passing, to avoid confusion, rename CreatePolicyStmt's 'cmd' to
> 'cmd_name', parse_policy_command's 'cmd' to 'polcmd', and
> AlterPolicy's 'cmd_datum' to 'polcmd_datum', per discussion with Noah
> and as a follow-up to his correction of copynodes/equalnodes handling of
> the CreatePolicyStmt 'cmd' field.

I'd rather see those split into separate commits. Doing polishing and
active bugs in one commit imo isn't a good idea if the polishing goes
beyond a line or two.

Otherwise this looks ok to me.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to