Hi,
On 2015-08-12 16:46:01 -0400, Stephen Frost wrote:
> From ab44660e9b8fc5b10f84ce6ba99a8340047ac8a5 Mon Sep 17 00:00:00 2001
> From: Stephen Frost <sfrost@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
