Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
On 7/31/17 20:42, Peter Eisentraut wrote: > That looks like a bug to me. ALTER USER also does not support the IN > DATABASE clause, so the code deviation might have started there already. > > I propose the attached patch to clean this up. > > For backpatching, I could develop some less invasive versions. done and done -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
On Tue, Aug 1, 2017 at 8:42 AM, Pavel Golub wrote: > Sorry, if I was rough. My English is not so excellent. My point is > that I was trying to distinguish behavior of EDB installer and > "build from source" PG. > > And the result is that EDB executes ALTER USER and I don't know why. I don't know why, either. Are you sure you haven't made some mistake in the testing? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
Hello, Robert. Sorry, if I was rough. My English is not so excellent. My point is that I was trying to distinguish behavior of EDB installer and "build from source" PG. And the result is that EDB executes ALTER USER and I don't know why. You wrote: RH> On Thu, Jul 27, 2017 at 2:52 AM, Pavel Golub wrote: >> One more notice. ALTER USER ALL works in EnterpriseDB 10beta2 >> installer. That's weird. I thought EnterpriseDB uses official sources. RH> I find it really hard to believe that we're doing anything else. It RH> wouldn't make any sense to patch the PostgreSQL source code and then RH> release the installers as PostgreSQL installers. And if we *were* RH> going to do that, wouldn't we patch something more interesting than RH> the ALTER USER command? I don't know what's going on here but I have RH> a feeling that EnterpriseDB secretly maintaining patch sets that we RH> inject into our PostgreSQL installers is not that thing. RH> Adding a few EDB people. -- With best wishes, Pavel mailto:pa...@gf.microolap.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
On 7/26/17 11:29, Tom Lane wrote:
> You'll notice that that statement fails in the regression tests:
>
> ALTER USER ALL SET application_name to 'SLAP';
> ERROR: syntax error at or near "ALL"
>
> The one that works is
>
> ALTER ROLE ALL SET application_name to 'SLAP';
>
> and the reason is that AlterRoleSetStmt has a separate production
> for ALL, but AlterUserSetStmt doesn't. This seems a tad bizarre
> though. Peter, you added that production (in commit 9475db3a4);
> is this difference intentional or just an oversight? If it's
> intentional, what's the reasoning?
That looks like a bug to me. ALTER USER also does not support the IN
DATABASE clause, so the code deviation might have started there already.
I propose the attached patch to clean this up.
For backpatching, I could develop some less invasive versions.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 48a7c95b7c8243626362c7845a45cdb036028f7e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 31 Jul 2017 20:36:32 -0400
Subject: [PATCH] Further unify ROLE and USER command grammar rules
ALTER USER ... SET did not support all the syntax variants of ALTER ROLE
... SET. Fix that, and to avoid further deviations of this kind, unify
many the grammar rules for ROLE/USER/GROUP commands.
Reported-by: Pavel Golub
---
doc/src/sgml/ref/alter_user.sgml| 8 +--
src/backend/parser/gram.y | 105 +++-
src/test/regress/expected/rolenames.out | 10 +--
3 files changed, 43 insertions(+), 80 deletions(-)
diff --git a/doc/src/sgml/ref/alter_user.sgml b/doc/src/sgml/ref/alter_user.sgml
index 9b8a39b376..411a6dcc38 100644
--- a/doc/src/sgml/ref/alter_user.sgml
+++ b/doc/src/sgml/ref/alter_user.sgml
@@ -38,10 +38,10 @@
ALTER USER name RENAME TO
new_name
-ALTER USER role_specification SET
configuration_parameter { TO | = } {
value | DEFAULT }
-ALTER USER role_specification SET
configuration_parameter FROM CURRENT
-ALTER USER role_specification
RESET configuration_parameter
-ALTER USER role_specification
RESET ALL
+ALTER USER { role_specification |
ALL } [ IN DATABASE database_name
] SET configuration_parameter { TO | = } {
value | DEFAULT }
+ALTER USER { role_specification |
ALL } [ IN DATABASE database_name
] SET configuration_parameter FROM CURRENT
+ALTER USER { role_specification |
ALL } [ IN DATABASE database_name
] RESET configuration_parameter
+ALTER USER { role_specification |
ALL } [ IN DATABASE database_name
] RESET ALL
where role_specification
can be:
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 4b1ce09c44..e20bf5ec55 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -250,7 +250,7 @@ static Node *makeRecursiveViewSelect(char *relname, List
*aliases, Node *query);
AlterObjectDependsStmt AlterObjectSchemaStmt AlterOwnerStmt
AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt
AlterForeignTableStmt
- AlterCompositeTypeStmt AlterUserStmt AlterUserMappingStmt
AlterUserSetStmt
+ AlterCompositeTypeStmt AlterUserMappingStmt
AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt ClosePortalStmt ClusterStmt CommentStmt
@@ -262,9 +262,9 @@ static Node *makeRecursiveViewSelect(char *relname, List
*aliases, Node *query);
CreateAssertStmt CreateTransformStmt CreateTrigStmt
CreateEventTrigStmt
CreateUserStmt CreateUserMappingStmt CreateRoleStmt
CreatePolicyStmt
CreatedbStmt DeclareCursorStmt DefineStmt DeleteStmt
DiscardStmt DoStmt
- DropGroupStmt DropOpClassStmt DropOpFamilyStmt DropPLangStmt
DropStmt
+ DropOpClassStmt DropOpFamilyStmt DropPLangStmt DropStmt
DropAssertStmt DropCastStmt DropRoleStmt
- DropUserStmt DropdbStmt DropTableSpaceStmt
+ DropdbStmt DropTableSpaceStmt
DropTransformStmt
DropUserMappingStmt ExplainStmt FetchStmt
GrantStmt GrantRoleStmt ImportForeignSchemaStmt IndexStmt
InsertStmt
@@ -841,8 +841,6 @@ stmt :
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
- | AlterUserSetStmt
- | AlterUserStmt
| AnalyzeStmt
| CheckPointStmt
| ClosePortalStmt
@@ -890,7 +888,6 @@ stmt :
| DoStmt
| DropAssertStmt
| DropCastStmt
- | DropGroupStmt
| DropOpClassStmt
| DropOpFamilyStmt
Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
On Thu, Jul 27, 2017 at 2:52 AM, Pavel Golub wrote: > One more notice. ALTER USER ALL works in EnterpriseDB 10beta2 > installer. That's weird. I thought EnterpriseDB uses official sources. I find it really hard to believe that we're doing anything else. It wouldn't make any sense to patch the PostgreSQL source code and then release the installers as PostgreSQL installers. And if we *were* going to do that, wouldn't we patch something more interesting than the ALTER USER command? I don't know what's going on here but I have a feeling that EnterpriseDB secretly maintaining patch sets that we inject into our PostgreSQL installers is not that thing. Adding a few EDB people. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
Hello, Tom. You wrote: TL> Pavel Golub writes: >> I need someone to throw some light on grammar (gram.y). >> I'm investigating beta2 regression tests, and found new statement >> `ALTER USER ALL SET application_name to 'SLAP';` >> ^^^ TL> You'll notice that that statement fails in the regression tests: TL> ALTER USER ALL SET application_name to 'SLAP'; TL> ERROR: syntax error at or near "ALL" One more notice. ALTER USER ALL works in EnterpriseDB 10beta2 installer. That's weird. I thought EnterpriseDB uses official sources. TL> The one that works is TL> ALTER ROLE ALL SET application_name to 'SLAP'; TL> and the reason is that AlterRoleSetStmt has a separate production TL> for ALL, but AlterUserSetStmt doesn't. This seems a tad bizarre TL> though. Peter, you added that production (in commit 9475db3a4); TL> is this difference intentional or just an oversight? If it's TL> intentional, what's the reasoning? TL> BTW, I'm quite confused as to why these test cases (in rolenames.sql) TL> seem to predate that commit, and yet it did not change their results. TL> regards, tom lane -- With best wishes, Pavel mailto:pa...@gf.microolap.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
Hello, Tom. You wrote: TL> Pavel Golub writes: >> I need someone to throw some light on grammar (gram.y). >> I'm investigating beta2 regression tests, and found new statement >> `ALTER USER ALL SET application_name to 'SLAP';` >> ^^^ TL> You'll notice that that statement fails in the regression tests: TL> ALTER USER ALL SET application_name to 'SLAP'; TL> ERROR: syntax error at or near "ALL" Oops! My bad! TL> The one that works is TL> ALTER ROLE ALL SET application_name to 'SLAP'; TL> and the reason is that AlterRoleSetStmt has a separate production TL> for ALL, but AlterUserSetStmt doesn't. Yeap, I see now separate rule for ALL in AlterRoleSetStmt. TL> This seems a tad bizarre TL> though. Peter, you added that production (in commit 9475db3a4); TL> is this difference intentional or just an oversight? If it's TL> intentional, what's the reasoning? TL> BTW, I'm quite confused as to why these test cases (in rolenames.sql) TL> seem to predate that commit, and yet it did not change their results. TL> regards, tom lane -- With best wishes, Pavel mailto:pa...@gf.microolap.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
Pavel Golub writes: > I need someone to throw some light on grammar (gram.y). > I'm investigating beta2 regression tests, and found new statement > `ALTER USER ALL SET application_name to 'SLAP';` > ^^^ You'll notice that that statement fails in the regression tests: ALTER USER ALL SET application_name to 'SLAP'; ERROR: syntax error at or near "ALL" The one that works is ALTER ROLE ALL SET application_name to 'SLAP'; and the reason is that AlterRoleSetStmt has a separate production for ALL, but AlterUserSetStmt doesn't. This seems a tad bizarre though. Peter, you added that production (in commit 9475db3a4); is this difference intentional or just an oversight? If it's intentional, what's the reasoning? BTW, I'm quite confused as to why these test cases (in rolenames.sql) seem to predate that commit, and yet it did not change their results. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] AlterUserStmt anmd RoleSpec rules in grammar.y
Hello, hackers. I need someone to throw some light on grammar (gram.y). I'm investigating beta2 regression tests, and found new statement `ALTER USER ALL SET application_name to 'SLAP';` ^^^ I know for sure that in beta1 this operator fails. So I decided to recheck gram.y: AlterUserStmt: ALTER USER RoleSpec SetResetClause; RoleSpec:NonReservedWord | CURRENT_USER | SESSION_USER; But *ALL is reserved word*! Thus "ALTER ROLE\USER ALL" should fail. OK, I checked in Pg10 beta2, installer provided by EDB. It worked. Then I asked someone to check this against fresh built server from 'master'. It failed. So, the situation is: 1. Docs say this is correct statement: https://www.postgresql.org/docs/devel/static/sql-alterrole.html 2. The sources in master don't support such production: https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/parser/gram.y;h=4b1ce09c445a5ee249a965ec0953b122df71eb6f;hb=refs/heads/master Line 1179 for AlterUserSetStmt rule; Line 14515 for RoleSpec rule; 3. EDB 10beta2 server supports it. What's going on? -- With best wishes, Pavel mailto:pa...@gf.microolap.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
