Обсуждение: [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:pavel@gf.microolap.com
Pavel Golub <pavel@microolap.com> 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
Hello, Tom. You wrote: TL> Pavel Golub <pavel@microolap.com> 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:pavel@gf.microolap.com
Hello, Tom. You wrote: TL> Pavel Golub <pavel@microolap.com> 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:pavel@gf.microolap.com
On Thu, Jul 27, 2017 at 2:52 AM, Pavel Golub <pavel@microolap.com> 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
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
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 <pavel@microolap.com> 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:pavel@gf.microolap.com
On Tue, Aug 1, 2017 at 8:42 AM, Pavel Golub <pavel@microolap.com> 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
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