Grammer Cleanup

Started by Stephen Frostover 21 years ago12 messagespatches
Jump to latest
#1Stephen Frost
sfrost@snowman.net

Greetings,

Small patch to clean up the grammer a bit by adding 'GroupId',
'SchemaName' and 'SavePointId'. I noticed these issues while
beginning work on 'Group Ownership'. Any questions/comments, please
let me know.

Thanks,

Stephen

Attachments:

gram_cleanup.difftext/plain; charset=us-asciiDownload+18-15
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#1)
Re: Grammer Cleanup

Stephen Frost <sfrost@snowman.net> writes:

Small patch to clean up the grammer a bit by adding 'GroupId',
'SchemaName' and 'SavePointId'.

I don't particularly see the value of this --- especially since the
direction of future development is likely to be to remove the
distinction between user and group names, rather than make it stronger.

regards, tom lane

#3Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#2)
Re: Grammer Cleanup

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Small patch to clean up the grammer a bit by adding 'GroupId',
'SchemaName' and 'SavePointId'.

I don't particularly see the value of this --- especially since the
direction of future development is likely to be to remove the
distinction between user and group names, rather than make it stronger.

Alright, I can redo it w/ UserId in place of GroupId everywhere. I'd
like your comment on the 'pg_class changes for group ownership' on
-hackers, which might play into these changes too. Previously 'GRANT'
had 'ColId' instead of either, which isn't really appropriate there...
Do you agree with the other changes (ColId -> SchemaName, ColId ->
SavePointId) ?

Thanks,

Stephen

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#3)
Re: Grammer Cleanup

Stephen Frost <sfrost@snowman.net> writes:

Do you agree with the other changes (ColId -> SchemaName, ColId ->=20
SavePointId) ?

I don't really see the value of them. They add some marginal
documentation I suppose, but they also make the grammar bigger and
slower. A more substantial objection to the practice is that it can
introduce needless shift/reduce conflicts, by forcing the parser into
making unnecessary decisions before it has enough context to
determine what kind of name a particular token really is.

(I don't claim that your patch as it stands has any such problem,
because it doesn't touch any particularly hairy parts of the grammar.
I'm just saying why I don't necessarily believe in a separate production
for every kind of name.)

regards, tom lane

#5Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#4)
Re: Grammer Cleanup

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Do you agree with the other changes (ColId -> SchemaName, ColId ->=20
SavePointId) ?

I don't really see the value of them. They add some marginal
documentation I suppose, but they also make the grammar bigger and
slower. A more substantial objection to the practice is that it can
introduce needless shift/reduce conflicts, by forcing the parser into
making unnecessary decisions before it has enough context to
determine what kind of name a particular token really is.

Perhaps the name of 'ColId' should be changed then. At least to me that
comes across as 'Column Identifier', and apparently some others thought
it wasn't appropriate for user names (UserId existed and was mapped to
ColId prior to my patch). Personally, I'd just like it to be
consistent, when I was looking at how to add the grammar for group
ownership group names were identified in one place as 'ColId' and another
as 'UserId', iirc.

Stephen

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#5)
Re: Grammer Cleanup

Stephen Frost <sfrost@snowman.net> writes:

Personally, I'd just like it to be
consistent, when I was looking at how to add the grammar for group
ownership group names were identified in one place as 'ColId' and another
as 'UserId', iirc.

Oh, I had forgotten we already had a UserId production. Yes, I agree
that if we're going to have that at all, it should be used consistently
in the places it applies to.

Given other discussion, it might be best to rename it to RoleId and use
that for both users and groups.

regards, tom lane

#7Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#6)
Re: Grammer Cleanup

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Given other discussion, it might be best to rename it to RoleId and use
that for both users and groups.

Ok, should I change SchemaName & SavePointId back to ColId, leave them
as in the patch, change them to RoleId, or something else? Neither
ColId nor RoleId seems appropriate for those..

Thanks,

Stephen

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#7)
Re: Grammer Cleanup

Stephen Frost <sfrost@snowman.net> writes:

Ok, should I change SchemaName & SavePointId back to ColId,

I'd just leave them as ColId. I don't think much would be gained by
introducing those productions.

regards, tom lane

#9Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#8)
Re: Grammer Cleanup

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Ok, should I change SchemaName & SavePointId back to ColId,

I'd just leave them as ColId. I don't think much would be gained by
introducing those productions.

Done, here's the patch.

Thanks,

Stephen

Attachments:

gram_cleanup2.difftext/plain; charset=us-asciiDownload+28-28
#10Bruce Momjian
bruce@momjian.us
In reply to: Stephen Frost (#9)
Re: Grammer Cleanup

This has been saved for the 8.1 release:

http:/momjian.postgresql.org/cgi-bin/pgpatches2

---------------------------------------------------------------------------

Stephen Frost wrote:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Ok, should I change SchemaName & SavePointId back to ColId,

I'd just leave them as ColId. I don't think much would be gained by
introducing those productions.

Done, here's the patch.

Thanks,

Stephen

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#11Bruce Momjian
bruce@momjian.us
In reply to: Stephen Frost (#9)
Re: Grammer Cleanup

I am removing this patch from the queue because without the "role"
feature I don't think it makes sense to rename the grammer tokens.

---------------------------------------------------------------------------

Stephen Frost wrote:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Ok, should I change SchemaName & SavePointId back to ColId,

I'd just leave them as ColId. I don't think much would be gained by
introducing those productions.

Done, here's the patch.

Thanks,

Stephen

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#12Stephen Frost
sfrost@snowman.net
In reply to: Bruce Momjian (#11)
Re: Grammer Cleanup

* Bruce Momjian (pgman@candle.pha.pa.us) wrote:

I am removing this patch from the queue because without the "role"
feature I don't think it makes sense to rename the grammer tokens.

Sure. I've rolled this patch into my role tree so these changes will be
included when the CREATE ROLE, etc is introduced.

Thanks,

Stephen

Show quoted text

Stephen Frost wrote:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Ok, should I change SchemaName & SavePointId back to ColId,

I'd just leave them as ColId. I don't think much would be gained by
introducing those productions.

Done, here's the patch.

Thanks,

Stephen

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
Bruce Momjian                        |  http://candle.pha.pa.us
pgman@candle.pha.pa.us               |  (610) 359-1001
+  If your life is a hard drive,     |  13 Roberts Road
+  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073