Hello, thank you for the comment.
> Looking at this a bit, I'm not sure completely replacing RoleId with a
> node is the best idea; some of the users of that production in the
> grammar are okay with accepting only normal strings as names, and don't
> need all the CURRENT_USER etc stuff.
You're right. the productions don't need RoleSpec already uses
char* for the role member in its *Stmt structs. I might have done
a bit too much.
Adding new nonterminal RoleId and using it makes a bit duplicate
of check code for "public"/"none" and others but removes other
redundant check for "!= CSTRING" from some production, CREATE
ROLE/USER/GROUP, ALTER ROLE/USER/GROUP RENAME.
RoleId in the patch still has rule components for CURRENT_USER,
SESSION_USER, and CURRENT_ROLE. Without them, the parser prints
an error ununderstandable to users.
| =# alter role current_user rename to "PuBlic";
| ERROR: syntax error at or near "rename"
| LINE 1: alter role current_user rename to "PuBlic";
| ^
With the components, the error message becomes like this.
| =# alter role current_role rename to "PuBlic";
| ERROR: reserved word cannot be used
| LINE 1: alter role current_role rename to "PuBlic";
| ^
> Maybe we need a new production,
> say RoleSpec, and we modify the few productions that need the extra
> flexibility? For instance we could have ALTER USER RoleSpec instead of
> ALTER USER RoleId. But we would keep CREATE USER RoleId, because it
> doesn't make any sense to accept CREATE USER CURRENT_USER.
> I think that would lead to a less invasive patch also.
> Am I making sense?
I think I did what you expected. It was good as the code got
simpler but the invasive-ness dosn't seem to be reduced..
What do you think about this?
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center