Thread: [PATCH] minor doc fix for create-role

[PATCH] minor doc fix for create-role

From
tara@anne.cat
Date:
Hi,

I'm not subscribed to the list, so please include me directly if you want my attention. This is a "drive-by" patch as
itwere. 

Attached is a minor patch to fix the name param documentation for create role, just adding a direct quote from
user-manag.sgmltalking about what the role name is allowed to be.  I was searching for this information and figured the
referencepage should have it as well. 

Hope this helps,
-Tara

Attachment

Re: [PATCH] minor doc fix for create-role

From
Tom Lane
Date:
tara@anne.cat writes:
> Attached is a minor patch to fix the name param documentation for create role, just adding a direct quote from
user-manag.sgmltalking about what the role name is allowed to be.  I was searching for this information and figured the
referencepage should have it as well. 

Hm, I guess my reaction to this proposal is "why just here?".  We have
an awful lot of different CREATE commands, and none of them say more
about the target name than this one does.  (Not to mention ALTER, DROP,
etc.)  Perhaps it's worth adding some boilerplate text to all those
places, but I'm dubious.

Also, the specific text proposed for addition doesn't seem that helpful,
since it doesn't define which characters are "special characters".
I'd rather see something like "The name must be a valid SQL identifier
as defined in <link to section 4.1.1>."  But, while that would work fine
in HTML output, it would not be particularly short or useful in man-page
output.

Perhaps the ideal solution would be further markup on the synopsis
sections that somehow identifies each term as an "identifier" or
other appropriate syntactic category, and provides a hyperlink to
a definition (in output formats that are friendly to that).  Seems
like a lot of work though :-(

            regards, tom lane



Re: [PATCH] minor doc fix for create-role

From
Tara Anne
Date:
Hi Tom,

I agree mostly. It actually does have the words “SQL identifier” in the patch. But you are right it doesn’t link to
whata SQL identifier is, but it does provide a practical solution of quoting. That was the part I cared about as a
user,I just wanted to solve my problem of an email address as a role name (yes I know that’s sort of dumb as email
addresseschange). This also addresses the question, why just here, because this was a pain point in the docs for me
yesterday:) 

 I also agree your ideal solution is definitely better than what I pushed. But I’m not ready to take that on. If
someoneelse is, I welcome their patch over mine. 

-Tara

  —
“Rivers know this: there is no hurry. We shall get there some day.”

> On Aug 18, 2019, at 9:41 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> tara@anne.cat writes:
>> Attached is a minor patch to fix the name param documentation for create role, just adding a direct quote from
user-manag.sgmltalking about what the role name is allowed to be.  I was searching for this information and figured the
referencepage should have it as well. 
>
> Hm, I guess my reaction to this proposal is "why just here?".  We have
> an awful lot of different CREATE commands, and none of them say more
> about the target name than this one does.  (Not to mention ALTER, DROP,
> etc.)  Perhaps it's worth adding some boilerplate text to all those
> places, but I'm dubious.
>
> Also, the specific text proposed for addition doesn't seem that helpful,
> since it doesn't define which characters are "special characters".
> I'd rather see something like "The name must be a valid SQL identifier
> as defined in <link to section 4.1.1>."  But, while that would work fine
> in HTML output, it would not be particularly short or useful in man-page
> output.
>
> Perhaps the ideal solution would be further markup on the synopsis
> sections that somehow identifies each term as an "identifier" or
> other appropriate syntactic category, and provides a hyperlink to
> a definition (in output formats that are friendly to that).  Seems
> like a lot of work though :-(
>
>            regards, tom lane