Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector

Поиск
Список
Период
Сортировка
От Karl O. Pinc
Тема Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Дата
Msg-id 20231003181520.02f104df@slate.karlpinc.com
обсуждение исходный текст
Ответ на Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector  ("David G. Johnston" <david.g.johnston@gmail.com>)
Ответы Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Список pgsql-hackers
On Tue, 3 Oct 2023 14:51:31 -0700
"David G. Johnston" <david.g.johnston@gmail.com> wrote:

> 0001 - I would just call the section:
> Capturing Command Results into Variables

I like your wording a lot.  Let's use it!

> I would add commentary in there that it is only possible for
> variables to take on single value at any given time and so in order
> to handle multiple row results you need to instantiate a loop as per
> 43.6.6

That sounds reasonable.  Let me see what I can come up with.
I'll do it in a separate commit.

> 0002 - {Inferred | Indirect} Types ?
> We are already in the Declarations section so the fact we are
> declaring new variables is already covered.

I agree.  But the problem is that the section title needs
to stand on it's own when the table of contents is scanned.
By that I don't mean that getting context from a "Declaration"
higher level section is somehow out-of-bounds, but that
neither "inferred" nor "indirect" has a recognizable meaning
unless the section body itself is read.

I thought about: Referencing Existing Types
But the problem with that is that the reader does not know
that this has to do with the types of existing objects
rather than working with user-defined types (or something else).

Also, I kind of like "re-using".  Shorter, simpler, word.

There is: Re-Using the Type of Objects

"Objects" is not very clear.  Variables are very different from
columns.  It seemed best to just write it out.

> "Instead of literally writing a type name you can write variable%TYPE
> and the system will indirectly apply the then-current type of the
> named variable to the newly declared variable." 

I've no objection to the section leading with a summary sentence like
this.  The trouble is coming up with something good.  I'd go with
"You can reference the data type of an existing column or variable
with the %TYPE notation.  The syntax is:"  Shorter and simpler.

Along with this change, it might be nice to move the "By using %TYPE
..." paragraph to after the first example (before the first paragraph).

But this is really feature creep for this commit.  :)

> (using "copy the
> then-current" reads pretty well and makes the existing title
> usable...)

I disagree.  The title needs to be understandable without reading
the section body.

> 
> 0003 - The noun "Exception" here means "deviating from the normal
> flow of the code", not, "A subclass of error".  I don't see this
> title change being particularly beneficial.

Isn't the entire section about "deviating from the normal flow of the
code"?  That's what makes me want "Exception" in the section title.

> 0004 - Agreed, but "You can raise an error explicitly as described in
> "Errors and Messages".  I would not use the phrase "raise an
> exception", it doesn't fit.

I like the brevity of your sentence.  And you're right that
the sentence does not flow as written.  How about:

You can raise an exception to throw an error as described in
"Errors and Messages".

?  I remain (overly?) focused on the word "exception", since that's
whats in the brain of the user that's writing RAISE EXCEPTION.
It matters if exceptions and errors are different.  If they're
not, then it also matters since it's exceptions that the user's
code raises.

> 0005 - This tone of voice is used throughout the introductory
> documentation sections, this single change doesn't seem warranted.

Ok.  I'll drop it unless somebody else chimes in.

> 0006 - Useful.  The view itself provides all configuration parameters
> known to the system, the "or selected" isn't true of the view itself,
> and it seems to go without saying that the user can add a where
> clause to any query they write using that view.  At most I'd make one
> of the examples include a where clause.

Good points.

I'll get rid of the ", or selected,".  May as well leave the
examples as short as possible.  Less is more.  :)

> 0007 - I'm leaning toward this area being due for some improvement,
> especially given the v16 changes bring new attention to it, but this
> patch doesn't scream "improvement" to me.

Let's see how it looks with 0012, which uses the vocabulary.

I do like the "Roles therefore control who has what access to which
objects." sentence.  I was shooting for shorter sentences, but then
when I broke the existing sentences into pieces I couldn't resist
adding text.

> 0008 - Same as 0007.  INHERIT is no longer an attribute though, it is
> a property of membership.

(!) I'm going to have to pay more attention.

>  This seems more acceptable on its own but
> I'd need more time to take in the big picture myself.

It's the big picture that I'm trying to draw. 0007, 0008, and 0012
all kind of go together.  It may be worth forking the email thread,
or something.

Have you any thoughts on the "permissions", "privleges" and 
"attributes" vocabulary/concepts used in this area?


Thanks very much for the review.  I'm going to wait a bit
before incorporating your suggestions and sending in another
patch set in case Daniel chimes in.  (I'm slightly
nervous about the renumbering making the thread hard to follow.)

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Jeff Davis
Дата:
Сообщение: Re: Improve WALRead() to suck data directly from WAL buffers when possible
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag