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