On Fri, Mar 17, 2023 at 10:40:06AM +0100, Peter Eisentraut wrote:
> committed
Thanks!
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On 17.03.23 00:47, Nathan Bossart wrote:
Here is a rebased patch in which I've addressed the latest feedback except
for the DropRole() part that is under discussion.
committed
Here is a rebased patch in which I've addressed the latest feedback except
for the DropRole() part that is under discussion.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From cd6a75109471e173869a15b39342ff4882eac61f Mon Sep 17 00:00:00 2001
From: Nathan Bossart
Date: Thu, 26 Ja
On Thu, Mar 16, 2023 at 04:59:53PM +0100, Peter Eisentraut wrote:
> On 16.03.23 16:48, Nathan Bossart wrote:
>> > I think the following change in DropRole() is incorrect:
>> >
>> > if (!is_admin_of_role(GetUserId(), roleid))
>> > ereport(ERROR,
>> > (errc
On 16.03.23 16:48, Nathan Bossart wrote:
I think the following change in DropRole() is incorrect:
if (!is_admin_of_role(GetUserId(), roleid))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-errmsg("must have admin option on
On Thu, Mar 16, 2023 at 04:24:07PM +0100, Peter Eisentraut wrote:
> I have committed two pieces that were not message changes separately.
Thanks!
> I think the following change in DropRole() is incorrect:
>
> if (!is_admin_of_role(GetUserId(), roleid))
> ereport(ERROR,
>
On 10.03.23 01:03, Nathan Bossart wrote:
By the way, I'm not sure what the separation between 0001 and 0002 is
supposed to be.
I'll combine them. I first started with user.c only, but we kept finding
new messages to improve.
I combined the patches in v7.
I have committed two pieces that were
On Thu, Mar 09, 2023 at 09:58:46AM -0800, Nathan Bossart wrote:
> On Thu, Mar 09, 2023 at 10:55:54AM +0100, Peter Eisentraut wrote:
>> On 20.02.23 23:58, Nathan Bossart wrote:
>>> For now, I've reworded these as "must inherit privileges of".
>>
>> I don't have a good mental model of all this role
On Thu, Mar 09, 2023 at 10:55:54AM +0100, Peter Eisentraut wrote:
> On 20.02.23 23:58, Nathan Bossart wrote:
>> For now, I've reworded these as "must inherit privileges of".
>
> I don't have a good mental model of all this role inheritance, personally,
> but I fear that this change makes the messa
On 20.02.23 23:58, Nathan Bossart wrote:
Similarly -- this is an existing issue but we might as well look at it -- in
something like
must be superuser or a role with privileges of the
pg_write_server_files role
the phrase "a role with the privileges of that other role" seems ambiguous
On Mon, Feb 20, 2023 at 11:02:10AM -0800, Nathan Bossart wrote:
> On Mon, Feb 20, 2023 at 08:54:48AM +0100, Peter Eisentraut wrote:
>> I'm concerned about the loose use of "privilege" here. A privilege is
>> something I can grant. So if someone doesn't have the "REPLICATION
>> privilege", as in t
On Mon, Feb 20, 2023 at 08:54:48AM +0100, Peter Eisentraut wrote:
> I'm concerned about the loose use of "privilege" here. A privilege is
> something I can grant. So if someone doesn't have the "REPLICATION
> privilege", as in the above example, I would expect to be able to do "GRANT
> REPLICATIO
On 07.02.23 21:10, Nathan Bossart wrote:
ERROR: permission denied to use replication slots
DETAIL: You must have REPLICATION privilege.
adding the operation to the end seems less awkward (i.e., "You must have
REPLICATION privilege to use replication slots."). I don't think the
On Fri, Jan 27, 2023 at 03:15:07PM -0800, Nathan Bossart wrote:
> One thing that feels a bit odd is how some of the DETAILs mention the
> operation being attempted while others do not. For example, we have
>
> ERROR: permission denied to drop role
> DETAIL: You must have SUPERUSER pr
On Fri, Jan 27, 2023 at 07:31:19PM +0100, Alvaro Herrera wrote:
> On 2023-Jan-26, Nathan Bossart wrote:
>> ereport(ERROR,
>>
>> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>> - errmsg("permission denied: bo
While we're here,
On 2023-Jan-26, Nathan Bossart wrote:
> @@ -838,7 +867,8 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
> if (!should_be_super && roleid == BOOTSTRAP_SUPERUSERID)
> ereport(ERROR,
>
> (errcode(ERRC
On 2023-Jan-27, Tom Lane wrote:
> Good point. My vote is for standardizing on *not* mentioning it.
> Error messages should say "you need privilege X". That is not
> the place to go into all the ways you could hold privilege X
> (one of which is being superuser).
+1
--
Álvaro Herrera
Robert Haas writes:
> I almost hate to bring this up since I'm not sure how far we want to
> go down this rat hole, but what should be our policy about mentioning
> superuser? I don't think we're entirely consistent right now, and I'm
> not sure whether every error message needs to mention that if
On Fri, Jan 27, 2023 at 10:52 AM Nathan Bossart
wrote:
> IMHO superuser should typically only be mentioned when it is the only way
> to do something. Since superusers have all privileges, I think logs like
> "superuser or privileges of X" are kind of redundant. If Robert has
> privileges of X, w
On Fri, Jan 27, 2023 at 08:31:32AM -0500, Robert Haas wrote:
> I almost hate to bring this up since I'm not sure how far we want to
> go down this rat hole, but what should be our policy about mentioning
> superuser? I don't think we're entirely consistent right now, and I'm
> not sure whether ever
On Fri, Jan 27, 2023 at 5:00 AM Peter Eisentraut
wrote:
> This is good. If I may assign some more work ;-), we have a bunch of
> error messages like
>
> errmsg("must be superuser or a role with privileges of the
> pg_write_server_files role to create backup stored on server")
>
> errmsg("must be
On 26.01.23 01:22, Nathan Bossart wrote:
Here is an early draft of some modest improvements to the user.c error
messages. I basically just tried to standardize the style of and add
context to the existing error messages. I used errhint() for this extra
context, but errdetail() would work, too.
On Thu, Jan 26, 2023 at 05:41:32PM -0500, Tom Lane wrote:
> Well, it's not a hint. I think the above is fine for non-password
> cases, but for passwords maybe
>
> ERROR: permission denied to alter role password
> DETAIL: To change another role's password, you must have CREATEROLE
>
Nathan Bossart writes:
> On Thu, Jan 26, 2023 at 03:07:43PM -0500, Tom Lane wrote:
>> I think the password case needs to be kept separate, because the
>> conditions for it are different (specifically the exception that
>> you can alter your own password). Lumping the rest together
>> seems OK to
On Thu, Jan 26, 2023 at 03:07:43PM -0500, Tom Lane wrote:
> Nathan Bossart writes:
>> On Thu, Jan 26, 2023 at 02:42:05PM -0500, Robert Haas wrote:
>>> Basically my question is whether having one error message for all of
>>> those cases is good enough, or whether we should be trying harder.
>
> I
Nathan Bossart writes:
> On Thu, Jan 26, 2023 at 02:42:05PM -0500, Robert Haas wrote:
>> Basically my question is whether having one error message for all of
>> those cases is good enough, or whether we should be trying harder.
I think the password case needs to be kept separate, because the
cond
On Thu, Jan 26, 2023 at 02:42:05PM -0500, Robert Haas wrote:
> @@ -758,16 +776,13 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
> {
> /* things an unprivileged user certainly can't do */
> if (dinherit || dcreaterole || dcreatedb || dcanlogin || dconnlimit ||
> - dvalidUntil || disrep
On Thu, Jan 26, 2023 at 2:14 PM Nathan Bossart wrote:
> Yeah, it's probably better to say "to alter roles with %s" to refer to
> roles that presently have the attribute and "to change the %s attribute"
> when referring to privileges for the attribute. I did this in v2, too.
>
> I've also switched
Thanks for taking a look.
On Thu, Jan 26, 2023 at 10:07:39AM +0100, Alvaro Herrera wrote:
> Please use
> errdetail("You must have %s privilege to create roles with %s.",
> "SUPERUSER", "SUPERUSER")));
>
> in this kind of message where multiple copies appear th
Please use
errdetail("You must have %s privilege to create roles with %s.",
"SUPERUSER", "SUPERUSER")));
in this kind of message where multiple copies appear that only differ in
the keyword to use, to avoid creating four copies of essentially the
same str
Nathan Bossart writes:
> On Thu, Jan 19, 2023 at 10:20:33AM -0500, Robert Haas wrote:
>> That would be great. I agree that it's good to try to improve the
>> error messages. It hasn't been entirely clear to me how to do that.
>> For instance, I don't think we want to say something like:
>>
>> ERR
moving this discussion to a new thread...
On Thu, Jan 19, 2023 at 10:20:33AM -0500, Robert Haas wrote:
> On Wed, Jan 18, 2023 at 6:17 PM Nathan Bossart
> wrote:
>> However, as the attribute
>> system becomes more sophisticated, I think we ought to improve the error
>> messages in user.c. IMHO m
32 matches
Mail list logo
