Brightwell, Adam wrote:
> All,
>
>
> > Thanks! Please add it to the next commitfest.
> >
> >
> > Sounds good. I'll update the patch and add accordingly.
> >
>
> Attached is an updated patch.
I noticed something strange while perusing this patch, but the issue
predates the patch. Some messages say "must be superuser or replication
role to foo", but our longstanding practice is to say "permission denied
to foo". Why do we have this inconsistency? Should we remove it? If
we do want to keep the old wording this patch should use "permission
denied to" in the places that it touches.
Other than that, since we already agreed that it's something we want,
the only comment I have about this patch is an empty line in variable
declarations here which should be removed:
> diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
> new file mode 100644
> index c9a9baf..ed89b23
> *** a/src/backend/commands/alter.c
> --- 807,848 ----
> bool *nulls;
> bool *replaces;
>
> ! AclObjectKind aclkind = get_object_aclkind(classId);
> !
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services