Thread: Grammer Cleanup
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
Attachment
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
* 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
Attachment
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
* 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
Attachment
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
* 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
Attachment
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
* 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
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
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
* 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
> 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