On 12/4/14 3:32 PM, Stephen Frost wrote:
On reflection, this seemed odd because of how the code was written but
perhaps it was intentional after all. In general, superuser should be
able to bypass permissions restrictions and I don't see why this case
should be different.
In general, I
* Robert Haas (robertmh...@gmail.com) wrote:
On Thu, Dec 4, 2014 at 3:59 PM, Stephen Frost sfr...@snowman.net wrote:
I have a hard time wrapping my head around what a *lot* of our users ask
when they see a given error message, but if our error message is 'you
must have a pear-shaped object
On 11/21/14 10:28 AM, Stephen Frost wrote:
Specifically:
---
One curious item to note is that the
current if(!superuser()) {} block approach has masked an inconsistency
in at least the FDW code which required a change to the regression
tests- namely, we normally force FDW
On Tue, Dec 2, 2014 at 4:11 PM, Stephen Frost sfr...@snowman.net wrote:
It's pretty clear that the current message is complaining about a
permissions problem, so the fact that it doesn't specifically include
the words permission and denied doesn't bother me. Let me ask the
question again:
* Peter Eisentraut (pete...@gmx.net) wrote:
On 11/21/14 10:28 AM, Stephen Frost wrote:
Specifically:
---
One curious item to note is that the
current if(!superuser()) {} block approach has masked an inconsistency
in at least the FDW code which required a change to the
On 11/26/14 10:24 AM, Stephen Frost wrote:
The implementation detail is that it's not part of the normal
GRANT/REVOKE privilege system, which is why it's useful to note it in
the detail and why we don't need to add an errdetail along the lines of
'You must have SELECT rights on relation X to
* Robert Haas (robertmh...@gmail.com) wrote:
On Tue, Dec 2, 2014 at 4:11 PM, Stephen Frost sfr...@snowman.net wrote:
It's pretty clear that the current message is complaining about a
permissions problem, so the fact that it doesn't specifically include
the words permission and denied
* Peter Eisentraut (pete...@gmx.net) wrote:
I will produce a generic 'permission denied' error, and if the reason
for the lack of permission is anything other than GRANT/REVOKE, then I
will add it to the detail message.
That's what I had been thinking, on the assumption that individuals with
On 2014-12-04 15:59:17 -0500, Stephen Frost wrote:
I have a hard time wrapping my head around what a *lot* of our users ask
when they see a given error message, but if our error message is 'you
must have a pear-shaped object to run this command' then I imagine that
a new-to-PG user might think
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-12-04 15:59:17 -0500, Stephen Frost wrote:
I have a hard time wrapping my head around what a *lot* of our users ask
when they see a given error message, but if our error message is 'you
must have a pear-shaped object to run this
Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-12-04 15:59:17 -0500, Stephen Frost wrote:
I just don't understand why you want to pointlessly tinker with this.
Because I don't feel it's pointless to improve the consistency of the
error messaging and
On 2014-12-04 17:06:02 -0500, Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-12-04 15:59:17 -0500, Stephen Frost wrote:
I have a hard time wrapping my head around what a *lot* of our users ask
when they see a given error message, but if our error message is
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-12-04 17:06:02 -0500, Stephen Frost wrote:
Would you agree with Peter that, rather than focus on if the errdetail()
involves an implementation detail or not, we should go ahead and add the
You must have SELECT rights ... to the
Alvaro,
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
Several dozens messages ago in this thread I would have dropped this
item, TBH.
I'm getting to that point, though it's quite frustrating. I didn't much
care initially but it's gotten to the point where the current situation
just
On Thu, Dec 4, 2014 at 3:59 PM, Stephen Frost sfr...@snowman.net wrote:
I have a hard time wrapping my head around what a *lot* of our users ask
when they see a given error message, but if our error message is 'you
must have a pear-shaped object to run this command' then I imagine that
a
On Wed, Nov 26, 2014 at 10:12 AM, Stephen Frost sfr...@snowman.net wrote:
* Tom Lane (t...@sss.pgh.pa.us) wrote:
In the context at hand, I think most of the messages in question are
currently phrased like must be superuser to do X. I'd be fine with
changing that to permission denied to do X,
* Robert Haas (robertmh...@gmail.com) wrote:
On Wed, Nov 26, 2014 at 10:12 AM, Stephen Frost sfr...@snowman.net wrote:
* Tom Lane (t...@sss.pgh.pa.us) wrote:
In the context at hand, I think most of the messages in question are
currently phrased like must be superuser to do X. I'd be fine
On Tue, Dec 2, 2014 at 11:29 AM, Stephen Frost sfr...@snowman.net wrote:
* Robert Haas (robertmh...@gmail.com) wrote:
On Wed, Nov 26, 2014 at 10:12 AM, Stephen Frost sfr...@snowman.net wrote:
* Tom Lane (t...@sss.pgh.pa.us) wrote:
In the context at hand, I think most of the messages in
* Robert Haas (robertmh...@gmail.com) wrote:
On Tue, Dec 2, 2014 at 11:29 AM, Stephen Frost sfr...@snowman.net wrote:
It includes the actual error message, unlike what we have today which is
guidence as to what's required to get past the permission denied error.
In other words, I disagree
On Tue, Dec 2, 2014 at 2:35 PM, Stephen Frost sfr...@snowman.net wrote:
* Robert Haas (robertmh...@gmail.com) wrote:
On Tue, Dec 2, 2014 at 11:29 AM, Stephen Frost sfr...@snowman.net wrote:
It includes the actual error message, unlike what we have today which is
guidence as to what's
* Robert Haas (robertmh...@gmail.com) wrote:
On Tue, Dec 2, 2014 at 2:35 PM, Stephen Frost sfr...@snowman.net wrote:
* Robert Haas (robertmh...@gmail.com) wrote:
On Tue, Dec 2, 2014 at 11:29 AM, Stephen Frost sfr...@snowman.net wrote:
It includes the actual error message, unlike what we
On Sun, Nov 23, 2014 at 3:38 PM, Andres Freund and...@2ndquadrant.com wrote:
I'm not really particular about which way we go with the specific
wording (suggestions welcome..) but the inconsistency should be dealt
with.
Meh.
+1 for meh. I don't mind making things consistent if it can be done
* Robert Haas (robertmh...@gmail.com) wrote:
On Sun, Nov 23, 2014 at 3:38 PM, Andres Freund and...@2ndquadrant.com wrote:
I'm not really particular about which way we go with the specific
wording (suggestions welcome..) but the inconsistency should be dealt
with.
Meh.
+1 for meh. I
On 2014-11-26 08:33:10 -0500, Stephen Frost wrote:
* Robert Haas (robertmh...@gmail.com) wrote:
On Sun, Nov 23, 2014 at 3:38 PM, Andres Freund and...@2ndquadrant.com
wrote:
I'm not really particular about which way we go with the specific
wording (suggestions welcome..) but the
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-11-26 08:33:10 -0500, Stephen Frost wrote:
Doesn't that argument then apply to the other messages which I pointed
out in my follow-up to Andres, where the detailed info is in the hint
and the main error message is essentially
Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-11-26 08:33:10 -0500, Stephen Frost wrote:
Doesn't that argument then apply to the other messages which I pointed
out in my follow-up to Andres, where the detailed info is in the hint
and the main error
* Tom Lane (t...@sss.pgh.pa.us) wrote:
In the context at hand, I think most of the messages in question are
currently phrased like must be superuser to do X. I'd be fine with
changing that to permission denied to do X, but not to just
permission denied.
Apologies for the terseness of my
On 2014-11-26 11:53:40 -0300, Alvaro Herrera wrote:
Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-11-26 08:33:10 -0500, Stephen Frost wrote:
Doesn't that argument then apply to the other messages which I pointed
out in my follow-up to Andres, where the
* Andres Freund (and...@2ndquadrant.com) wrote:
I don't see how you read the contrary from the guidelines:
The primary message should be short, factual, and avoid reference to
implementation details such as specific function names. Short means
should fit on one line under normal conditions.
On 2014-11-26 10:18:20 -0500, Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
I don't see how you read the contrary from the guidelines:
The primary message should be short, factual, and avoid reference to
implementation details such as specific function names. Short
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-11-26 10:18:20 -0500, Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
I don't see how you read the contrary from the guidelines:
The primary message should be short, factual, and avoid reference to
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-11-21 10:12:40 -0500, Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
I still think this change makes the error message more verbose, without
any win in clarity.
Can we agree that there should be
On 2014-11-21 10:12:40 -0500, Stephen Frost wrote:
* Andres Freund (and...@2ndquadrant.com) wrote:
I still think this change makes the error message more verbose, without
any win in clarity.
Can we agree that there should be consistency?
Consistency with what? Are you thinking of the
* Andres Freund (and...@2ndquadrant.com) wrote:
I still think this change makes the error message more verbose, without
any win in clarity.
Can we agree that there should be consistency? I'm not really
particular about which way we go with the specific wording (suggestions
welcome..) but the
Peter,
* Peter Eisentraut (pete...@gmx.net) wrote:
On 11/5/14 5:10 PM, Adam Brightwell wrote:
Attached is two separate patches to address previous
comments/recommendations:
* superuser-cleanup-shortcuts_11-5-2014.patch
Seeing that the regression tests had to be changed in this patch
All,
It was brought up for discussion- see
http://www.postgresql.org/message-id/20141015052259.gg28...@tamriel.snowman.net
Specifically:
---
One curious item to note is that the
current if(!superuser()) {} block approach has masked an inconsistency
in at least the FDW
On 11/5/14 5:10 PM, Adam Brightwell wrote:
Attached is two separate patches to address previous
comments/recommendations:
* superuser-cleanup-shortcuts_11-5-2014.patch
Seeing that the regression tests had to be changed in this patch
indicates that there is a change of functionality, even
On 2014-11-05 17:10:17 -0500, Adam Brightwell wrote:
Attached is two separate patches to address previous
comments/recommendations:
* superuser-cleanup-shortcuts_11-5-2014.patch
* has_privilege-cleanup_11-5-2014.patch
-Adam
--
Adam Brightwell -
Attached is two separate patches to address previous
comments/recommendations:
* superuser-cleanup-shortcuts_11-5-2014.patch
* has_privilege-cleanup_11-5-2014.patch
-Adam
--
Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
diff --git
On 10/27/14 11:40 AM, Stephen Frost wrote:
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
As I started looking at this, there are multiple other places where
these types of error messages occur (opclasscmds.c, user.c,
postinit.c, miscinit.c are just a few), not just around the changes in
Thanks for looking at this patch.
I suggest moving the rest of the changes into separate patches.
Hmmm... perhaps the following?
* superuser-cleanup - contains above mentioned superuser shortcuts only.
* has_privilege-cleanup - contains has_*_priviledge cleanup only.
Would that also require
All,
* Stephen Frost (sfr...@snowman.net) wrote:
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
As I started looking at this, there are multiple other places where
these types of error messages occur (opclasscmds.c, user.c,
postinit.c, miscinit.c are just a few), not just around the
On 2014-10-28 09:43:35 -0400, Stephen Frost wrote:
All,
* Stephen Frost (sfr...@snowman.net) wrote:
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
As I started looking at this, there are multiple other places where
these types of error messages occur (opclasscmds.c, user.c,
* Andres Freund (and...@2ndquadrant.com) wrote:
For one I'm less than convinced that the new messages are an
improvement. They seem to be more verbose without a corresponding
improvement in clarity.
The goal with the changes is to improve consistency of messaging. These
messages are not at
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
Brightwell, Adam wrote:
If we were to make it consistent and use the old wording, what do you
think about providing an errhint as well?
Perhaps for example in slotfuncs.c#pg_create_physical_replication_stot:
errmsg - permission
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
As I started looking at this, there are multiple other places where
these types of error messages occur (opclasscmds.c, user.c,
postinit.c, miscinit.c are just a few), not just around the changes in
this patch. If we change them in one
On 10/23/14, 6:23 PM, Alvaro Herrera wrote:
Brightwell, Adam wrote:
If we were to make it consistent and use the old wording, what do you
think about providing an errhint as well?
Perhaps for example in slotfuncs.c#pg_create_physical_replication_stot:
errmsg - permission denied to create
Alvaro,
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
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
Brightwell, Adam wrote:
If we were to make it consistent and use the old wording, what do you
think about providing an errhint as well?
Perhaps for example in slotfuncs.c#pg_create_physical_replication_stot:
errmsg - permission denied to create physical replication slot
errhint -
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
All,
Thanks! Please add it to the next commitfest.
Sounds good. I'll update the patch and add accordingly.
Attached is an updated patch.
-Adam
--
Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
diff --git
Adam,
* Brightwell, Adam (adam.brightw...@crunchydatasolutions.com) wrote:
We, as a community, have gotten flak from time-to-time about the
superuser role. We also tend to wish to avoid unnecessary
optimization as it complicates the code base and makes folks reviewing
the code
53 matches
Mail list logo
