Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
On Thu, 28 Mar 2024 08:28:16 -0400 Robert Treat wrote: > I spoke with Karl briefly on this and he is working on getting an > updated patch together. The work now involves incorporating feedback > and some rebasing, but hopefully we will see something in the next few > days. Well, Friday has come and gone and I've not gotten to this. I'll see if I can spend time tomorrow. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Hello, Thank you all for your help. I won't be able to submit new patches based on reviews for 2 weeks. On Thu, 30 Nov 2023 16:02:28 +0530 Shubham Khanna wrote: > -v7-0012-Explain-role-management.patch > > + The managment of most database objects is by way of granting some > role > > Here 'managment' should be replaced with 'management'. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
On Tue, 3 Oct 2023 14:51:31 -0700
"David G. Johnston" 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
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
On Mon, 2 Oct 2023 15:18:32 -0500
"Karl O. Pinc" wrote:
Version 7
Added:
v7-0016-Predicate-locks-are-held-per-cluster-not-per-data.patch
> > > On Mon, 25 Sep 2023 23:37:44 -0500
> > > "Karl O. Pinc" wrote:
> > >
> > > > > On Mon, 25 Sep 2023 17:55:59 -0500
> > > > > "Karl O. Pinc" wrote:
> > > > >
> > > > > > On Mon, 25 Sep 2023 14:14:37 +0200
> > > > > > Daniel Gustafsson wrote:
> > > > >
> > > > > > > Once done you can do "git format-patch origin/master -v 1"
> > > > > > > which will generate a set of n patches named v1-0001
> > > > > > > through v1-000n.
> > >
> > > > > > I am not particularly confident in the top-line commit
> > > > > > descriptions.
> > > > >
> > > > > > The bulk of the commit descriptions are very wordy
> > > > >
> > > > > > Listing all the attachments here for future discussion:
v7-0001-Change-section-heading-to-better-reflect-saving-a.patch
v7-0002-Change-section-heading-to-better-describe-referen.patch
v7-0003-Better-section-heading-for-plpgsql-exception-trap.patch
v7-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch
v7-0005-Improve-sentences-in-overview-of-system-configura.patch
v7-0006-Provide-examples-of-listing-all-settings.patch
v7-0007-Cleanup-summary-of-role-powers.patch
v7-0008-Explain-the-difference-between-role-attributes-an.patch
v7-0009-Document-the-oidvector-type.patch
v7-0010-Improve-sentences-about-the-significance-of-the-s.patch
v7-0011-Add-a-sub-section-to-describe-schema-resolution.patch
v7-0012-Explain-role-management.patch
v7-0013-Hyperlink-from-CREATE-FUNCTION-reference-page-to-.patch
v7-0014-Add-index-entries-for-parallel-safety.patch
v7-0015-Trigger-authors-need-not-worry-about-parallelism.patch
v7-0016-Predicate-locks-are-held-per-cluster-not-per-data.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
>From 122665c4155698abe88e2bd17639a991791b94e3 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:49:30 -0500
Subject: [PATCH v7 01/16] Change section heading to better reflect saving a
result in variable(s)
The current section title of "Executing a Command with a Single-Row
Result" does not reflect what the section is really about. Other
sections make clear how to _execute_ commands, single-row result or not.
What this section is about is how to _save_ a single row of results into
variable(s).
It would be nice to talk about saving results into variables in the
section heading but I couldn't come up with anything pithy. "Saving a
Single-Row of a Command's Result" seems good enough, especially since
there's few other places to save results other than in variables.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f55e901c7e..8747e84245 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1126,7 +1126,7 @@ PERFORM create_mv('cs_session_page_requests_mv', my_query);
-Executing a Command with a Single-Row Result
+Saving a Single-Row of a Command's Result
SELECT INTO
--
2.30.2
>From 4de4a31d41124dfa793cc5cce0516673811ea414 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:52:21 -0500
Subject: [PATCH v7 02/16] Change section heading to better describe reference
of existing types
The section heading of "Copying Types" does not reflect what the
section is about. It is not about making copies of data types but
about using the data type of existing columns (or rows) in new type
declarations without having to know what the existing type is.
"Re-Using the Type of Columns and Variables" seems adequate. Getting
something in there about declartions seems too wordy. I thought
perhaps "Referencing" instead of "Re-Using", but "referencing" isn't
perfect and "re-using" is generic enough, shorter, and simpler to read.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 8747e84245..874578265e 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -672,7 +672,7 @@ DECLARE
- Copying Types
+ Re-Using the Type of Columns and Variables
variable%TYPE
--
2.30.2
>From 80c2b8ef7ad6e610f5c7bdc61b827983a87110e2 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 16:03:29 -0500
Subject:
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
On Sun, 1 Oct 2023 18:18:07 -0500
"Karl O. Pinc" wrote:
Version 6
Added:
v6-0015-Trigger-authors-need-not-worry-about-parallelism.patch
Can't say if this is an awesome idea or not. (Might have saved me time.)
Read the commit message for a justification.
> > On Mon, 25 Sep 2023 23:37:44 -0500
> > "Karl O. Pinc" wrote:
> >
> > > > On Mon, 25 Sep 2023 17:55:59 -0500
> > > > "Karl O. Pinc" wrote:
> > > >
> > > > > On Mon, 25 Sep 2023 14:14:37 +0200
> > > > > Daniel Gustafsson wrote:
> > > >
> > > > > > Once done you can do "git format-patch origin/master -v 1"
> > > > > > which will generate a set of n patches named v1-0001 through
> > > > > > v1-000n.
> >
> > > > > I am not particularly confident in the top-line commit
> > > > > descriptions.
> > > >
> > > > > The bulk of the commit descriptions are very wordy
> > > >
> > > > > Listing all the attachments here for future discussion:
v6-0001-Change-section-heading-to-better-reflect-saving-a.patch
v6-0002-Change-section-heading-to-better-describe-referen.patch
v6-0003-Better-section-heading-for-plpgsql-exception-trap.patch
v6-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch
v6-0005-Improve-sentences-in-overview-of-system-configura.patch
v6-0006-Provide-examples-of-listing-all-settings.patch
v6-0007-Cleanup-summary-of-role-powers.patch
v6-0008-Explain-the-difference-between-role-attributes-an.patch
v6-0009-Document-the-oidvector-type.patch
v6-0010-Improve-sentences-about-the-significance-of-the-s.patch
v6-0011-Add-a-sub-section-to-describe-schema-resolution.patch
v6-0012-Explain-role-management.patch
v6-0013-Hyperlink-from-CREATE-FUNCTION-reference-page-to-.patch
v6-0014-Add-index-entries-for-parallel-safety.patch
v6-0015-Trigger-authors-need-not-worry-about-parallelism.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
>From 122665c4155698abe88e2bd17639a991791b94e3 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:49:30 -0500
Subject: [PATCH v6 01/15] Change section heading to better reflect saving a
result in variable(s)
The current section title of "Executing a Command with a Single-Row
Result" does not reflect what the section is really about. Other
sections make clear how to _execute_ commands, single-row result or not.
What this section is about is how to _save_ a single row of results into
variable(s).
It would be nice to talk about saving results into variables in the
section heading but I couldn't come up with anything pithy. "Saving a
Single-Row of a Command's Result" seems good enough, especially since
there's few other places to save results other than in variables.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f55e901c7e..8747e84245 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1126,7 +1126,7 @@ PERFORM create_mv('cs_session_page_requests_mv', my_query);
-Executing a Command with a Single-Row Result
+Saving a Single-Row of a Command's Result
SELECT INTO
--
2.30.2
>From 4de4a31d41124dfa793cc5cce0516673811ea414 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:52:21 -0500
Subject: [PATCH v6 02/15] Change section heading to better describe reference
of existing types
The section heading of "Copying Types" does not reflect what the
section is about. It is not about making copies of data types but
about using the data type of existing columns (or rows) in new type
declarations without having to know what the existing type is.
"Re-Using the Type of Columns and Variables" seems adequate. Getting
something in there about declartions seems too wordy. I thought
perhaps "Referencing" instead of "Re-Using", but "referencing" isn't
perfect and "re-using" is generic enough, shorter, and simpler to read.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 8747e84245..874578265e 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -672,7 +672,7 @@ DECLARE
- Copying Types
+ Re-Using the Type of Columns and Variables
variable%TYPE
--
2.30.2
>From 80c2b8ef7ad6e610f5c7bdc61b827983a87110e2 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 16:03:29 -0500
Subject: [PATCH v6 03/15] Better section heading for plpgsql excep
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Version 5, this time with attachments.
Changed word order in a sentence:
v5-0012-Explain-role-management.patch
Added a hyperlink:
v5-0013-Hyperlink-from-CREATE-FUNCTION-reference-page-to-.patch
Added 3 index entries:
v5-0014-Add-index-entries-for-parallel-safety.patch
> On Mon, 25 Sep 2023 23:37:44 -0500
> "Karl O. Pinc" wrote:
>
> > > On Mon, 25 Sep 2023 17:55:59 -0500
> > > "Karl O. Pinc" wrote:
> > >
> > > > On Mon, 25 Sep 2023 14:14:37 +0200
> > > > Daniel Gustafsson wrote:
> > >
> > > > > Once done you can do "git format-patch origin/master -v 1"
> > > > > which will generate a set of n patches named v1-0001 through
> > > > > v1-000n.
>
> > > > I am not particularly confident in the top-line commit
> > > > descriptions.
> > >
> > > > The bulk of the commit descriptions are very wordy
> > >
> > > > Listing all the attachments here for future discussion:
v5-0001-Change-section-heading-to-better-reflect-saving-a.patch
v5-0002-Change-section-heading-to-better-describe-referen.patch
v5-0003-Better-section-heading-for-plpgsql-exception-trap.patch
v5-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch
v5-0005-Improve-sentences-in-overview-of-system-configura.patch
v5-0006-Provide-examples-of-listing-all-settings.patch
v5-0007-Cleanup-summary-of-role-powers.patch
v5-0008-Explain-the-difference-between-role-attributes-an.patch
v5-0009-Document-the-oidvector-type.patch
v5-0010-Improve-sentences-about-the-significance-of-the-s.patch
v5-0011-Add-a-sub-section-to-describe-schema-resolution.patch
v5-0012-Explain-role-management.patch
v5-0013-Hyperlink-from-CREATE-FUNCTION-reference-page-to-.patch
v5-0014-Add-index-entries-for-parallel-safety.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
>From 122665c4155698abe88e2bd17639a991791b94e3 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:49:30 -0500
Subject: [PATCH v5 01/14] Change section heading to better reflect saving a
result in variable(s)
The current section title of "Executing a Command with a Single-Row
Result" does not reflect what the section is really about. Other
sections make clear how to _execute_ commands, single-row result or not.
What this section is about is how to _save_ a single row of results into
variable(s).
It would be nice to talk about saving results into variables in the
section heading but I couldn't come up with anything pithy. "Saving a
Single-Row of a Command's Result" seems good enough, especially since
there's few other places to save results other than in variables.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f55e901c7e..8747e84245 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1126,7 +1126,7 @@ PERFORM create_mv('cs_session_page_requests_mv', my_query);
-Executing a Command with a Single-Row Result
+Saving a Single-Row of a Command's Result
SELECT INTO
--
2.30.2
>From 4de4a31d41124dfa793cc5cce0516673811ea414 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:52:21 -0500
Subject: [PATCH v5 02/14] Change section heading to better describe reference
of existing types
The section heading of "Copying Types" does not reflect what the
section is about. It is not about making copies of data types but
about using the data type of existing columns (or rows) in new type
declarations without having to know what the existing type is.
"Re-Using the Type of Columns and Variables" seems adequate. Getting
something in there about declartions seems too wordy. I thought
perhaps "Referencing" instead of "Re-Using", but "referencing" isn't
perfect and "re-using" is generic enough, shorter, and simpler to read.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 8747e84245..874578265e 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -672,7 +672,7 @@ DECLARE
- Copying Types
+ Re-Using the Type of Columns and Variables
variable%TYPE
--
2.30.2
>From 80c2b8ef7ad6e610f5c7bdc61b827983a87110e2 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 16:03:29 -0500
Subject: [PATCH v5 03/14] Better section heading for plpgsql exception
trapping
The docs seem to use "error" and "exception" interchangeably, perhaps
50% each. But they never say that the are the same thing, and in
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Version 5 Changed word order in a sentence: v5-0012-Explain-role-management.patch Added a hyperlink: v5-0013-Hyperlink-from-CREATE-FUNCTION-reference-page-to-.patch Added 3 index entries: v5-0014-Add-index-entries-for-parallel-safety.patch > On Mon, 25 Sep 2023 23:37:44 -0500 > "Karl O. Pinc" wrote: > > > > On Mon, 25 Sep 2023 17:55:59 -0500 > > > "Karl O. Pinc" wrote: > > > > > > > On Mon, 25 Sep 2023 14:14:37 +0200 > > > > Daniel Gustafsson wrote: > > > > > > > > Once done you can do "git format-patch origin/master -v 1" > > > > > which will generate a set of n patches named v1-0001 through > > > > > v1-000n. > > > > > I am not particularly confident in the top-line commit > > > > descriptions. > > > > > > > The bulk of the commit descriptions are very wordy > > > > > > > Listing all the attachments here for future discussion: v5-0001-Change-section-heading-to-better-reflect-saving-a.patch v5-0002-Change-section-heading-to-better-describe-referen.patch v5-0003-Better-section-heading-for-plpgsql-exception-trap.patch v5-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch v5-0005-Improve-sentences-in-overview-of-system-configura.patch v5-0006-Provide-examples-of-listing-all-settings.patch v5-0007-Cleanup-summary-of-role-powers.patch v5-0008-Explain-the-difference-between-role-attributes-an.patch v5-0009-Document-the-oidvector-type.patch v5-0010-Improve-sentences-about-the-significance-of-the-s.patch v5-0011-Add-a-sub-section-to-describe-schema-resolution.patch v5-0012-Explain-role-management.patch v5-0013-Hyperlink-from-CREATE-FUNCTION-reference-page-to-.patch v5-0014-Add-index-entries-for-parallel-safety.patch Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Version 4.
Added: v4-0012-Explain-role-management.patch
On Mon, 25 Sep 2023 23:37:44 -0500
"Karl O. Pinc" wrote:
> > On Mon, 25 Sep 2023 17:55:59 -0500
> > "Karl O. Pinc" wrote:
> >
> > > On Mon, 25 Sep 2023 14:14:37 +0200
> > > Daniel Gustafsson wrote:
> >
> > > > Once done you can do "git format-patch origin/master -v 1" which
> > > > will generate a set of n patches named v1-0001 through v1-000n.
> > > I am not particularly confident in the top-line commit
> > > descriptions.
> >
> > > The bulk of the commit descriptions are very wordy
> >
> > > Listing all the attachments here for future discussion:
v4-0001-Change-section-heading-to-better-reflect-saving-a.patch
v4-0002-Change-section-heading-to-better-describe-referen.patch
v4-0003-Better-section-heading-for-plpgsql-exception-trap.patch
v4-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch
v4-0005-Improve-sentences-in-overview-of-system-configura.patch
v4-0006-Provide-examples-of-listing-all-settings.patch
v4-0007-Cleanup-summary-of-role-powers.patch
v4-0008-Explain-the-difference-between-role-attributes-an.patch
v4-0009-Document-the-oidvector-type.patch
v4-0010-Improve-sentences-about-the-significance-of-the-s.patch
v4-0011-Add-a-sub-section-to-describe-schema-resolution.patch
v4-0012-Explain-role-management.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
>From 122665c4155698abe88e2bd17639a991791b94e3 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:49:30 -0500
Subject: [PATCH v4 01/12] Change section heading to better reflect saving a
result in variable(s)
The current section title of "Executing a Command with a Single-Row
Result" does not reflect what the section is really about. Other
sections make clear how to _execute_ commands, single-row result or not.
What this section is about is how to _save_ a single row of results into
variable(s).
It would be nice to talk about saving results into variables in the
section heading but I couldn't come up with anything pithy. "Saving a
Single-Row of a Command's Result" seems good enough, especially since
there's few other places to save results other than in variables.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f55e901c7e..8747e84245 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1126,7 +1126,7 @@ PERFORM create_mv('cs_session_page_requests_mv', my_query);
-Executing a Command with a Single-Row Result
+Saving a Single-Row of a Command's Result
SELECT INTO
--
2.30.2
>From 4de4a31d41124dfa793cc5cce0516673811ea414 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:52:21 -0500
Subject: [PATCH v4 02/12] Change section heading to better describe reference
of existing types
The section heading of "Copying Types" does not reflect what the
section is about. It is not about making copies of data types but
about using the data type of existing columns (or rows) in new type
declarations without having to know what the existing type is.
"Re-Using the Type of Columns and Variables" seems adequate. Getting
something in there about declartions seems too wordy. I thought
perhaps "Referencing" instead of "Re-Using", but "referencing" isn't
perfect and "re-using" is generic enough, shorter, and simpler to read.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 8747e84245..874578265e 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -672,7 +672,7 @@ DECLARE
- Copying Types
+ Re-Using the Type of Columns and Variables
variable%TYPE
--
2.30.2
>From 80c2b8ef7ad6e610f5c7bdc61b827983a87110e2 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 16:03:29 -0500
Subject: [PATCH v4 03/12] Better section heading for plpgsql exception
trapping
The docs seem to use "error" and "exception" interchangeably, perhaps
50% each. But they never say that the are the same thing, and in the
larger world they are not. Errors tend to be something that drop on
the floor and usually halt execution whereas exceptions can be trapped
and give the programmer more control over the flow of the program.
(Although, to be fair, exceptions are a subset of errors.) "Trapping
Errors" is not a good section title for these reasons, and because
when it comes to programmatically raising errors in Pl/PgSQL you
don't, you raise exceptions. The current s
Re: [PGdocs] fix description for handling pf non-ASCII characters
On Thu, 28 Sep 2023 12:54:33 +0900 Michael Paquier wrote: > I was looking at this thread overall, the three v3 flavors of the doc > changes and v4. > > -application_name value. Other characters > will be > -replaced with question marks (?). > +application_name value. > +Other characters are replaced with +linkend="sql-syntax-strings-escape">C-style hexadecimal > escapes. > > The simplicity of the change in v4 seems like the best approach to me, > so +1 for that (including the mention to "C-style"). I agree with Tom that it's not worth spending anyone's attention on bytes v.s. characters. So I'm marking the patch ready for committer. (I have not tried the version that patches against PGv16.) Thank you everyone, especially Hayato, for spending time and making this better. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: [PGdocs] fix description for handling pf non-ASCII characters
On Thu, 28 Sep 2023 09:49:03 +1000 Peter Smith wrote: > On Wed, Sep 27, 2023 at 11:59 PM Karl O. Pinc > wrote: > > > > On Wed, 27 Sep 2023 12:58:54 + > > "Hayato Kuroda (Fujitsu)" wrote: > > > > > > Should the committer be interested, your patch applies cleanly > > > > and the docs build as expected. > > > > > > Yeah, but cfbot accepted previous version. Did you have anything > > > in your mind? > > > > No. I'm letting the committer know everything I've checked > > so that they can decide what they want to check. > > > > > Hmm, what you said looked right. But as Peter pointed out [1], the > > > fix seems too much. So I attached three version of patches. How do > > > you think? For me, type C is best. > > > > > > A. A patch which completely follows your comments. The name is > > > "v3-0001-...patch". Cfbot tests it. > > > B. A patch which completely follows Peter's comments [1]. The > > > name is "Peter_v3-txt". > > > C. A patch which follows both comments. Based on > > > b, but some comments (Don't use the future tense, "Other > > > characters"->"The bytes of other characters"...) were picked. The > > > name is "Both_v3-txt". > > > > I also like C. Fewer words is better. So long > > as nothing is left unsaid fewer words make for clarity. > > > > However, in the last hunk, "of other than" does not read well. > > Instead of writing > > "and the bytes of other than printable ASCII characters" > > you want "and the bytes that are not printable ASCII characters". > > That would be my suggestion. > > > > I also prefer Option C, but... > > ~~~ > > +application_name value. > +The bytes of other characters are replaced with > +C-style escaped > hexadecimal > +byte values. > > V > > +cluster_name value. > +The bytes of other characters are replaced with > +C-style escaped > hexadecimal > +byte values. > > V > > + NAMEDATALEN characters and the bytes of other > than > + printable ASCII characters are replaced with + linkend="sql-syntax-strings-escape">C-style escaped > hexadecimal byte > + values. > > > IIUC all of these 3 places can have exactly the same wording change > (e.g. like Karl's last suggestion [1]). > > SUGGESTION > Any bytes that are not printable ASCII characters are replaced with > C-style escaped hexadecimal > byte values. I don't see the utility in having exactly the same phrase everywhere, especially since the last hunk is modifying the end of a long sentence. (Apologies if I'm mis-reading what Peter wrote above.) I like short sentences. So I prefer "The bytes of other characters" rather than "Any bytes that are not printable ASCII characters" for the first 2 hunks. In context I don't see the need to repeat the whole "printable ASCII characters" part that appears in the preceding sentence of both hunks. "Other" is clear, IMHO. But because I like short sentences I now think that it's a good idea to break the long sentence of the last hunk into two. Add a period and use the Peter's SUGGESTION above as the text for the second sentence. Is this desireable? Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein P.S. Hayato, it is good practice to cc everybody who has replied to a thread. At least I think that's what I see, it's not just people being lazy with reply-all. So I'm adding Tom Lane back to the thread. He can tell us otherwise if I'm wrong to add him back.
Re: [PGdocs] fix description for handling pf non-ASCII characters
On Wed, 27 Sep 2023 12:58:54 + "Hayato Kuroda (Fujitsu)" wrote: > > Should the committer be interested, your patch applies cleanly > > and the docs build as expected. > > Yeah, but cfbot accepted previous version. Did you have anything in > your mind? No. I'm letting the committer know everything I've checked so that they can decide what they want to check. > Hmm, what you said looked right. But as Peter pointed out [1], the > fix seems too much. So I attached three version of patches. How do > you think? For me, type C is best. > > A. A patch which completely follows your comments. The name is > "v3-0001-...patch". Cfbot tests it. > B. A patch which completely follows Peter's comments [1]. The name is > "Peter_v3-txt". > C. A patch which follows both comments. Based on > b, but some comments (Don't use the future tense, "Other > characters"->"The bytes of other characters"...) were picked. The > name is "Both_v3-txt". I also like C. Fewer words is better. So long as nothing is left unsaid fewer words make for clarity. However, in the last hunk, "of other than" does not read well. Instead of writing "and the bytes of other than printable ASCII characters" you want "and the bytes that are not printable ASCII characters". That would be my suggestion. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: [PGdocs] fix description for handling pf non-ASCII characters
Sep 26, 2023 1:10:55 PM Tom Lane : > "Karl O. Pinc" writes: >> For the last hunk you'd change around "anything". Write: >> "... it will be truncated to less than NAMEDATALEN characters and >> the bytes of the string which are not printable ASCII characters ...". > >> Notice that I have also changed "that" to "which" just above. >> I _think_ this is better English. > > No, I'm pretty sure you're mistaken. It's been a long time since > high school English, but the way I think this works is that "that" > introduces a restrictive clause, which narrows the scope of what > you are saying. That is, you say "that" when you want to talk > about only the bytes of the string that aren't ASCII. But "which" > introduces a non-restrictive clause that adds information or > commentary. If you say "bytes of the string which are not ASCII", > you are effectively making a side assertion that no byte of the > string is ASCII. Which is not the meaning you want here. Makes sense to me. "That" it is. Thanks for the help. I never would have figured that out.
Re: [PGdocs] fix description for handling pf non-ASCII characters
On Tue, 26 Sep 2023 13:40:26 + "Hayato Kuroda (Fujitsu)" wrote: > Your effort is quite helpful for me. You're welcome. > Before replying your comments, I thought I should show the difference > between versions. Regarding old versions (here PG15 was used), > non-ASCIIs (like Japanese) are replaced with '?'. > > ``` > psql (15.4) > Type "help" for help. > > postgres=# SET application_name TO 'あああ'; > SET > postgres=# SHOW application_name ; > application_name > -- > ? > (1 row) > ``` > > As for the HEAD, as my patch said, non-ASCIIs are replaced > with hexadecimal representations. (Were my terminologies correct?). > > ``` > psql (17devel) > Type "help" for help. > > postgres=# SET application_name TO 'あああ'; > SET > postgres=# SHOW application_name ; >application_name > -- > \xe3\x81\x82\xe3\x81\x82\xe3\x81\x82 > (1 row) > ``` I think you're terminology is correct, but see my suggestion at bottom. Never hurts to have output to look at. I noticed here and when reading the patch that changed the output -- the patch is operating on bytes, not characters per-se. > > First, it is now best practice in the PG docs to > > put a line break at the end of each sentence. > > At least for the sentences on the lines you change. > > (No need to update the whole document!) Please > > do this in the next version of your patch. I don't > > know if this is a requirement for acceptance by > > a committer, but it won't hurt. > > I didn't know that. Could you please tell me if you have a source? I thought I could find an email but the search is taking forever. If I find something I'll let you know. I could even be mis-remembering, but it's a nice practice regardless. There are a number of undocumented conventions. Another is the use of gender neutral language. The place to discuss doc conventions/styles would be the pgsql-docs list. (If you felt like asking there.) You could try submitting another patch to add various doc conventions to the official docs at https://www.postgresql.org/docs/current/docguide-style.html :-) > Anyway, I put a line break for each sentences for now. Thanks. A related thing that's nice to have is to limit the line length of the documentation source to 80 characters or less. 79 is probably best. Since the source text around your patch conforms to this convention you should also. > IIUC the word " Postgres" cannot be used in the doc. I think you're right. > Based on your all comments, I changed as below. How do you think? > > ``` > Characters that are not printable ASCII, like > \x03, are replaced with the > PostgreSQL linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte > value. ``` > > > Since the both of you have looked and confirmed that the > > actual behavior matches the new documentation I have not > > done this. > > I showed the result again, please see. Should the committer be interested, your patch applies cleanly and the docs build as expected. Also, based on the comments in the patch which changed the system's behavior, I believe that your patch updates all the relevant places in the documentation. > > But, have either of you checked that we're really talking about > > replacing everything outside the 7-bit ASCII encodings? > > My reading of the commit referenced in the first email of this > > thread says that it's everything outside of the _printable_ > > ASCII encodings, ASCII values outside of the range 32 to 127, > > inclusive. > > > > Please check. The docs should probably say "printable ASCII", > > or "non-printable ASCII", depending. I think the meaning > > of "printable ASCII" is widely enough known to be 32-127. > > So "printable" is good enough, right? > > For me, "non-printable ASCII" sounds like control characters. So that > I used the sentence "Characters that are not printable ASCII ... are > replaced with...". Please tell me if you have better explanation? Your explanation sounds good to me. I now think that you should consider another change to your wording. Instead of starting with "Characters that are not printable ASCII ..." consider writing "The bytes of the string which are not printable ASCII ...". Notice above that characters (e.g. あ) generate output for each non-ASCII byte (e.g. \xe3\x81\x82). So my thought is that the docs should be talking about bytes. For the last hunk you'd change around "anything". Write: "... it will be truncated to less than NAMEDATALEN characters and the bytes of the string which are not printable ASCII characters ...". Notice that I have also changed "that" to "which" just above. I _think_ this is better English. See sense 3 of: https://en.wiktionary.org/wiki/which See also the first paragraph of: https://en.wikipedia.org/wiki/Relative_pronoun If the comments above move you, send another patch. Seems to me we're close to sending this on to a committer. Regards, Karl Free
Re: [PGdocs] fix description for handling pf non-ASCII characters
Hello Hayato and Jian, On Tue, 4 Jul 2023 01:30:56 + "Hayato Kuroda (Fujitsu)" wrote: > Dear Jian, > > looks fine. Do you need to add to commitfest? > > Thank you for your confirmation. ! I registered to following: > > https://commitfest.postgresql.org/44/4437/ The way the Postgres commitfest process works is that someone has to update the page to mark "reviewed" and the reviewer has to use the commitfest website to pass the patches to a committer. I see a few problems with the English and style of the patches and am commenting below and have signed up as a reviewer. At commitfest.postgresql.org I have marked the thread as needing author attention. Hayato, you will need to mark the thread as needing review when you have replied to this message. Jian, you might want to sign on as a reviewer as well. It can be nice to have that record of your participation. Now, to reviewing the patch: First, it is now best practice in the PG docs to put a line break at the end of each sentence. At least for the sentences on the lines you change. (No need to update the whole document!) Please do this in the next version of your patch. I don't know if this is a requirement for acceptance by a committer, but it won't hurt. diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e700782d3c..a4ce99ba4d 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7040,9 +7040,8 @@ local0.*/var/log/postgresql The name will be displayed in the pg_stat_activity view and included in CSV log entries. It can also be included in regular log entries via the parameter. -Only printable ASCII characters may be used in the -application_name value. Other characters will be -replaced with question marks (?). +Non-ASCII characters used in the application_name +will be replaced with hexadecimal strings. Don't use the future tense to describe the system's behavior. Instead of "will be" write "are". (Yes, this change would be an improvement on the original text. We should fix it while we're working on it and our attention is focused.) It is more accurate, if I understand the issue, to say that characters are replaced with hexadecimal _representations_ of the input bytes. Finally, it would be good to know what representation we're talking about. Perhaps just give the \xhh example and say: the Postgres C-style escaped hexadecimal byte value. And hyperlink to https://www.postgresql.org/docs/16/sql-syntax-lexical.html#SQL-SYNTAX-STRINGS-ESCAPE (The docbook would be, depending on text you want to link: C-style escaped hexadecimal byte value. I think. You link to id="someidvalue" attribute values.) @@ -8037,10 +8036,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; The name can be any string of less than NAMEDATALEN characters (64 characters in a standard -build). Only printable ASCII characters may be used in the -cluster_name value. Other characters will be -replaced with question marks (?). No name is shown -if this parameter is set to the empty string '' (which is +build). Non-ASCII characters used in the cluster_name +will be replaced with hexadecimal strings. No name is shown if this +parameter is set to the empty string '' (which is the default). This parameter can only be set at server start. Same review as for the first patch hunk. diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 5062d712e7..98785e87ea 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -1067,9 +1067,8 @@ postgres=# SELECT postgres_fdw_disconnect_all(); of any length and contain even non-ASCII characters. However when it's passed to and used as application_name in a foreign server, note that it will be truncated to less than - NAMEDATALEN characters and anything other than - printable ASCII characters will be replaced with question - marks (?). + NAMEDATALEN characters and non-ASCII characters will be + replaced with hexadecimal strings. See for details. Same review as for the first patch hunk. Since the both of you have looked and confirmed that the actual behavior matches the new documentation I have not done this. But, have either of you checked that we're really talking about replacing everything outside the 7-bit ASCII encodings? My reading of the commit referenced in the first email of this thread says that it's everything outside of the _printable_ ASCII encodings, ASCII values outside of the range 32 to 127, inclusive. Please check. The docs should probably say "printable ASCII", or "non-printable ASCII", depending. I think the meaning of "printable ASCII" is widely enough known to be 32-127. So "printable" is good enough, right? Regards, Karl Free Software: "You don't pay back, you pay
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Forgot to attach. Sorry.
On Mon, 25 Sep 2023 23:30:38 -0500
"Karl O. Pinc" wrote:
> Version 3.
>
> Re-do title, which is all of patch v3-003.
>
> On Mon, 25 Sep 2023 17:55:59 -0500
> "Karl O. Pinc" wrote:
>
> > On Mon, 25 Sep 2023 14:14:37 +0200
> > Daniel Gustafsson wrote:
>
> > > Once done you can do "git format-patch origin/master -v 1" which
> > > will generate a set of n patches named v1-0001 through v1-000n.
>
> > Done. 11 patches attached. Thanks for the help.
>
> > I am not particularly confident in the top-line commit
> > descriptions.
>
> > The bulk of the commit descriptions are very wordy
>
> > Listing all the attachments here for future discussion:
>
> v3-0001-Change-section-heading-to-better-reflect-saving-a.patch
> v3-0002-Change-section-heading-to-better-describe-referen.patch
> v3-0003-Better-section-heading-for-plpgsql-exception-trap.patch
> v3-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch
> v3-0005-Improve-sentences-in-overview-of-system-configura.patch
> v3-0006-Provide-examples-of-listing-all-settings.patch
> v3-0007-Cleanup-summary-of-role-powers.patch
> v3-0008-Explain-the-difference-between-role-attributes-an.patch
> v3-0009-Document-the-oidvector-type.patch
> v3-0010-Improve-sentences-about-the-significance-of-the-s.patch
> v3-0011-Add-a-sub-section-to-describe-schema-resolution.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
>From 122665c4155698abe88e2bd17639a991791b94e3 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:49:30 -0500
Subject: [PATCH v3 01/11] Change section heading to better reflect saving a
result in variable(s)
The current section title of "Executing a Command with a Single-Row
Result" does not reflect what the section is really about. Other
sections make clear how to _execute_ commands, single-row result or not.
What this section is about is how to _save_ a single row of results into
variable(s).
It would be nice to talk about saving results into variables in the
section heading but I couldn't come up with anything pithy. "Saving a
Single-Row of a Command's Result" seems good enough, especially since
there's few other places to save results other than in variables.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f55e901c7e..8747e84245 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1126,7 +1126,7 @@ PERFORM create_mv('cs_session_page_requests_mv', my_query);
-Executing a Command with a Single-Row Result
+Saving a Single-Row of a Command's Result
SELECT INTO
--
2.30.2
>From 4de4a31d41124dfa793cc5cce0516673811ea414 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:52:21 -0500
Subject: [PATCH v3 02/11] Change section heading to better describe reference
of existing types
The section heading of "Copying Types" does not reflect what the
section is about. It is not about making copies of data types but
about using the data type of existing columns (or rows) in new type
declarations without having to know what the existing type is.
"Re-Using the Type of Columns and Variables" seems adequate. Getting
something in there about declartions seems too wordy. I thought
perhaps "Referencing" instead of "Re-Using", but "referencing" isn't
perfect and "re-using" is generic enough, shorter, and simpler to read.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 8747e84245..874578265e 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -672,7 +672,7 @@ DECLARE
- Copying Types
+ Re-Using the Type of Columns and Variables
variable%TYPE
--
2.30.2
>From 80c2b8ef7ad6e610f5c7bdc61b827983a87110e2 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 16:03:29 -0500
Subject: [PATCH v3 03/11] Better section heading for plpgsql exception
trapping
The docs seem to use "error" and "exception" interchangeably, perhaps
50% each. But they never say that the are the same thing, and in the
larger world they are not. Errors tend to be something that drop on
the floor and usually halt execution whereas exceptions can be trapped
and give the programmer more control over the flow of the program.
(Although, to be fair, exceptions are a subset of errors.) "Trapping
Errors" is not a good section title for these reasons, and because
when it comes to programmatically raising errors i
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
Version 3. Re-do title, which is all of patch v3-003. On Mon, 25 Sep 2023 17:55:59 -0500 "Karl O. Pinc" wrote: > On Mon, 25 Sep 2023 14:14:37 +0200 > Daniel Gustafsson wrote: > > Once done you can do "git format-patch origin/master -v 1" which > > will generate a set of n patches named v1-0001 through v1-000n. > Done. 11 patches attached. Thanks for the help. > I am not particularly confident in the top-line commit > descriptions. > The bulk of the commit descriptions are very wordy > Listing all the attachments here for future discussion: v3-0001-Change-section-heading-to-better-reflect-saving-a.patch v3-0002-Change-section-heading-to-better-describe-referen.patch v3-0003-Better-section-heading-for-plpgsql-exception-trap.patch v3-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch v3-0005-Improve-sentences-in-overview-of-system-configura.patch v3-0006-Provide-examples-of-listing-all-settings.patch v3-0007-Cleanup-summary-of-role-powers.patch v3-0008-Explain-the-difference-between-role-attributes-an.patch v3-0009-Document-the-oidvector-type.patch v3-0010-Improve-sentences-about-the-significance-of-the-s.patch v3-0011-Add-a-sub-section-to-describe-schema-resolution.patch Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
On Mon, 25 Sep 2023 14:14:37 +0200
Daniel Gustafsson wrote:
> > On 25 Sep 2023, at 14:00, Karl O. Pinc wrote:
>
> > Is there a preferred data format or should I send
> > each patch in a separate attachment with description?
> Once done you can do "git format-patch origin/master -v 1" which will
> generate a set of n patches named v1-0001 through v1-000n. You can
> then attache those to the thread.
Done. 11 patches attached. Thanks for the help.
(This is v2, since I made some changes upon review.)
I am not particularly confident in the top-line commit
descriptions. Some seem kind of long and not a whole
lot of thought went into them. But the commit descriptions
are for the committer to decide anyway.
The bulk of the commit descriptions are very wordy
and will surely need at least some editing.
Listing all the attachments here for future discussion:
v2-0001-Change-section-heading-to-better-reflect-saving-a.patch
v2-0002-Change-section-heading-to-better-describe-referen.patch
v2-0003-Better-section-heading-for-plpgsql-exception-trap.patch
v2-0004-Describe-how-to-raise-an-exception-in-the-excepti.patch
v2-0005-Improve-sentences-in-overview-of-system-configura.patch
v2-0006-Provide-examples-of-listing-all-settings.patch
v2-0007-Cleanup-summary-of-role-powers.patch
v2-0008-Explain-the-difference-between-role-attributes-an.patch
v2-0009-Document-the-oidvector-type.patch
v2-0010-Improve-sentences-about-the-significance-of-the-s.patch
v2-0011-Add-a-sub-section-to-describe-schema-resolution.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
>From 122665c4155698abe88e2bd17639a991791b94e3 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:49:30 -0500
Subject: [PATCH v2 01/11] Change section heading to better reflect saving a
result in variable(s)
The current section title of "Executing a Command with a Single-Row
Result" does not reflect what the section is really about. Other
sections make clear how to _execute_ commands, single-row result or not.
What this section is about is how to _save_ a single row of results into
variable(s).
It would be nice to talk about saving results into variables in the
section heading but I couldn't come up with anything pithy. "Saving a
Single-Row of a Command's Result" seems good enough, especially since
there's few other places to save results other than in variables.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f55e901c7e..8747e84245 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1126,7 +1126,7 @@ PERFORM create_mv('cs_session_page_requests_mv', my_query);
-Executing a Command with a Single-Row Result
+Saving a Single-Row of a Command's Result
SELECT INTO
--
2.30.2
>From 4de4a31d41124dfa793cc5cce0516673811ea414 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 15:52:21 -0500
Subject: [PATCH v2 02/11] Change section heading to better describe reference
of existing types
The section heading of "Copying Types" does not reflect what the
section is about. It is not about making copies of data types but
about using the data type of existing columns (or rows) in new type
declarations without having to know what the existing type is.
"Re-Using the Type of Columns and Variables" seems adequate. Getting
something in there about declartions seems too wordy. I thought
perhaps "Referencing" instead of "Re-Using", but "referencing" isn't
perfect and "re-using" is generic enough, shorter, and simpler to read.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 8747e84245..874578265e 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -672,7 +672,7 @@ DECLARE
- Copying Types
+ Re-Using the Type of Columns and Variables
variable%TYPE
--
2.30.2
>From 0252dd434bb9ab2487cd37a93912d19ca1ef5149 Mon Sep 17 00:00:00 2001
From: "Karl O. Pinc"
Date: Sun, 24 Sep 2023 16:03:29 -0500
Subject: [PATCH v2 03/11] Better section heading for plpgsql exception
trapping
The docs seem to use "error" and "exception" interchangeably, perhaps
50% each. But they never say that the are the same thing, and in the
larger world they are not. Errors tend to be something that drop on
the floor and usually halt execution whereas exceptions can be trapped
and give the programmer more control over the flow of the program.
"Trapping Errors" is not a good section title for these reasons, and
because when it comes to programmatically raising errors in Pl/PgSQL
you do
Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector
On Mon, 25 Sep 2023 09:26:38 +0200 Daniel Gustafsson wrote: > > On 25 Sep 2023, at 00:57, Karl O. Pinc wrote: > > > I have made various, mostly unrelated to each other, > > small improvements to the documentation. > While I agree it's subjective, I don't think adding a new section or > paragraph qualifies as short or small. I would prefer if each > (related) change is in a single commit with a commit message which > describes the motivation for the change. A reviewer can second-guess > the rationale for the changes, but they shouldn't have to. Will do. Is there a preferred data format or should I send each patch in a separate attachment with description? > The resulting patchset can all be in the same thread though. Thanks. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Fix bug in VACUUM and ANALYZE docs
On Wed, 20 Sep 2023 13:39:02 +0900 Michael Paquier wrote: > On Wed, Sep 20, 2023 at 09:43:15AM +0900, Shinya Kato wrote: > > You're right. It looks good to me. > > Right, it feels like there has been a lot of copy-paste in this area > of the docs. Applied down to 16. I signed up to review, but I think that perhaps commitfest https://commitfest.postgresql.org/45/4574/ needs marking as applied and done? Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Various small doc improvements; plpgsql, schemas, permissions, oidvector
Hi, I have made various, mostly unrelated to each other, small improvements to the documentation. These are usually in the areas of plpgsql, schemas, and permissions. Most change 1 lines, but some supply short overviews. "Short" is subjective, so if these need to be broken into different threads or different commitfest entries let me know. I'm starting simple and submitting a single patch. Attached: various_doc_patches_v1.patch Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6bc1b215db..ea70dd3597 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -10,7 +10,27 @@ There are many configuration parameters that affect the behavior of - the database system. In the first section of this chapter we + the database system. + A single master list of all parameters and their characteristics is not + provided in the documentation. + The pg_settings view into the system catalogs can provide such summaries, + e.g: + + + +-- Describe all configuration parameters. +SELECT name, short_desc FROM pg_settings ORDER BY name; + +-- Show the current configuration of the connected database and session. +SELECT name, setting, unit FROM pg_settings ORDER BY name; + +-- Show the means available to change the setting; whether the setting is +-- per-cluster, per-database, per-session, etc. +SELECT name, context FROM pg_settings ORDER BY name; + + + + In the first section of this chapter we describe how to interact with configuration parameters. The subsequent sections discuss each parameter in detail. diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index 8d32a8c9c5..91cc1e5cb0 100644 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -4754,6 +4754,17 @@ INSERT INTO mytable VALUES(-1); -- fails signed-versus-unsigned confusion if you do this.) + +oidvector + + + + The legacy oidvector type can be cast to an array of OIDs, + and vice versa, for examination and manipulation. + The resultant array of OIDs, unlike a typical array, is indexed + zero-relative. + + The OID alias types have no operations of their own except for specialized input and output routines. These routines are able diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 075ff32991..df8a373652 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -3106,12 +3106,12 @@ SHOW search_path; The first schema in the search path that exists is the default location for creating new objects. That is the reason that by -default objects are created in the public schema. When objects -are referenced in any other context without schema qualification -(table modification, data modification, or query commands) the -search path is traversed until a matching object is found. -Therefore, in the default configuration, any unqualified access -again can only refer to the public schema. +default objects are created in the public schema. +When objects are referenced in a context without schema qualification +(table modification, data modification, or query commands) the search path +is traversed until a matching object is found. +Therefore, again, in the default configuration, any unqualified access +refers only to objects in the public schema. @@ -3162,6 +3162,53 @@ SELECT 3 OPERATOR(pg_catalog.+) 4; + +Schema Resolution + + + schema + resolution + + + + Schema resolution happens when SQL is run. + + Exactly what this means is usually obvious; using an unqualified table + name when creating a table creates the table in the first schema of the + search path in effect, the current search path is used to find + unqualified table names when executing a SELECT, and so forth. + But there are less obvious implications when it comes to statements + that manipulate database objects: + tables, triggers, functions and the like. + + + + Most SQL expressions appearing within the statements that manipulate + database objects are resolved at the time of manipulation. + Consider the creation of tables and triggers. + The schemas of foreign key table references, the functions and operators + used in table and column constraint expressions, table partition + expressions, and so forth are resolved at the time of table creation. + So is the schema of the function named when a trigger is created. + These already-resolved tables, functions, operators, + etc. need not be in the search path when the + constraints or triggers are executed, when the content of the table is + modified and the data validation occurs. + This is just as if all the objects were fully schema qualified in
Re: doc: add missing "id" attributes to extension packaging page
On Thu, 13 Apr 2023 10:53:31 -0500 "Karl O. Pinc" wrote: > On Thu, 13 Apr 2023 16:01:35 +0200 > Brar Piening wrote: > > > On 13.04.2023 at 10:31, Peter Eisentraut wrote: > > > Side project: I noticed that these new hover links don't appear in > > > the single-page HTML output (make postgres.html), even though the > > > generated HTML source code looks correct. Maybe someone has an > > > idea there. > > I feel responsible for the feature to work for all use cases where > > it makes sense. I'll investigate this and post back. > > Looks to me like the ">" in the CSS was transformed into the > HTML entity when the stylesheet was included into the single-file > HTML. The XSLT 1.0 spec says that characters in
Re: doc: add missing "id" attributes to extension packaging page
On Thu, 13 Apr 2023 16:01:35 +0200 Brar Piening wrote: > On 13.04.2023 at 10:31, Peter Eisentraut wrote: > > The first patch has been committed. > > Yay - thank you! > > > The second patch should be sent to pgsql-www for integrating into > > the web site. > Done via [1]. Thanks for the hint. > > > Side project: I noticed that these new hover links don't appear in > > the single-page HTML output (make postgres.html), even though the > > generated HTML source code looks correct. Maybe someone has an idea > > there. > I feel responsible for the feature to work for all use cases where it > makes sense. I'll investigate this and post back. Looks to me like the ">" in the CSS was transformed into the HTML entity when the stylesheet was included into the single-file HTML. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Thu, 13 Apr 2023 15:58:03 +0100 Dagfinn Ilmari Mannsåker wrote: > Peter Eisentraut writes: > > The first patch has been committed. > Another side note: I notice the links don't appear on > elements (e.g. > https://www.postgresql.org/docs/devel/sql-select.html#SQL-WITH), only > . This we know. Working with elements is a different dive into the XSLT which was deliberately put off for future work. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Thu, 6 Apr 2023 16:19:30 +0200 Brar Piening wrote: > Reviewer is Karl O. Pink "Karl O. Pinc" actually, with a "c". Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Tue, 4 Apr 2023 21:52:31 +0200 Brar Piening wrote: > On 04.04.2023 at 16:54, Peter Eisentraut wrote: > > The XSLT implementation looks sound to me. It would be a touch > > better if it had some comments about which parts of the templates > > were copied from upstream stylesheets and which were changed. I like this idea. A lot. (Including which stylesheets were copied from.) > > However, I wonder if this is the right way to approach this. I > > don't think we should put these link markers directly into the > > HTML. It feels like this is the wrong layer. For example, if you > > have CSS turned off, then all these # marks show up by default. > > I'd consider this a feature rather than a problem but this is > certainly debatable. I too would consider this a feature. If you don't style your html presentation, you see everything. The "#" links to content are, well, content. > > It seems to me that the correct way to do this is to hook in some > > JavaScript that does this transformation directly on the DOM. Then > > we don't need to carry this presentation detail in the HTML. I would argue the opposite. The HTML/CSS is delivered to the browser which is then free to present the content to the user in the form preferred by the user. This puts control of presentation in the hands of the end-user, where, IMO, it belongs. Using JavaScript to manipulate the DOM is all well and good when using AJAX to interact with the server to produce dynamic content. But otherwise manipulating the DOM with JavaScript seems overly heavy-handed, even though popular. It seems like JavaScript is used more because CSS is difficult and an "extra technology" when instead JavaScript can "do everything". So CSS is put aside. I may be biased, not being a JavaScript fan. (I tend to be one of those cranky individuals who keep JavaScript turned off.) I'd rather not have code executing when such overhead/complication can be avoided. (Insert here exciting argument about "what is code and what is data".) Glancing at the documentation source, I don't see JavaScript used at all. Introducing it would be adding something else to the mix. Not that this would be bad if it provides value. In the end, I don't _really_ care. And don't do web development all day either so my fundamentals could be just wrong. But this is my take. > > Moreover, it would avoid tight coupling between the website and the > > documentation sources. I don't really understand this statement. Are you saying that the documentation's source CSS needn't/shouldn't be the CSS used on the website? That seems counter-intuitive. But then I don't understand why the default CSS used when developing the documentation is not the CSS used on the website. I can imagine administrative arguments around server maintenance for wanting to keep the website decoupled from the PG source code. (I think.) But I can't speak to any of that. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Wed, 29 Mar 2023 21:32:05 -0700 Noah Misch wrote: > On Sun, Jan 22, 2023 at 02:42:46PM -0600, Karl O. Pinc wrote: > > v10-0001-List-trusted-and-obsolete-extensions.patch > > > + > > + These modules and extensions are obsolete: > > + > > + > > + > > + > > + > > + > > Commit a013738 incorporated this change. Since xml2 is the only > in-tree way to use XSLT from SQL, I think xml2 is not obsolete. Some > individual functions, e.g. xml_valid(), are obsolete. (There are > years-old threats to render the module obsolete, but this has never > happened.) Your point seems valid but this is above my station. I have no idea as to how to best resolve this, or even how to make the resolution happen now that the change has been committed. Someone who knows more than me about the situation is needed to change the phrasing, or re-categorize, or rework the xml2 module docs, or come up with new categories of obsolescence-like states, or provide access to libxslt from PG, or something. I am invested in the patch and appreciate being cc-ed. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
Hi Brar, An observation: The # that shows up when hovering over section-level headings is styled as the section-level heading is. But the # that shows up when hovering over varlistentrys has the default text style. This works for me. It's nice to have the "section #"s look like the section heading. But the varlistentry's terms are smaller than the normal font, and their line width is less heavy than normal. I'm not really invested one way or the other, but I find it kind of nice that the varlistentry's #s are easier to click on and more noticable because they're slightly larger than might be expected. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
This is for the committer, as an FYI. I cut out the portion of the docbook XSLT and diffed it with the code for the same template in the patch. The diff looks like: -- /tmp/sections.xsl2023-03-22 13:00:33.432968357 -0500 +++ /tmp/make_html_ids_discoverable_v3.patch2023-03-22 13:03:39.776930603 -0500 @@ -52,5 +52,8 @@ + + + (So, this output would start with line 52 of the template, not from the top of the stock sections.xsl file.) However, I am not really familiar with exactly what flavor of docbook, version, namespace-d or not, etc., that PG uses. So I could be diffing with the wrong thing. Hope this helps and is not just noise. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
Hi Brar, It occurs to me that I had not actually tested the way the anchor is put only after the last term in a varlistentry. (The code looked obviously right and should work if any varlistentry terms have anchors, which they do, but) Have you tested this? If not, catalog.sgml, the DEPENDENCY_PARTITION_SEC term is a 2nd term and usable for at test case. But, at the moment there are no ids for any of these varlistentries so you'd have to hack them in. Apologies for missing this. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Thu, 23 Mar 2023 10:35:55 +0100 Alvaro Herrera wrote: > As with the patch, we'll need to patch the CSS used in > the website for the docs too, as that's the most important place > where docs are visited. See this commit for an example: > https://git.postgresql.org/gitweb/?p=pgweb.git;a=commitdiff;h=0b89ea0fff28d29ed177c82a274144453e3c7f82 > > In order to test locally that your patched stylesheet works correctly, > you'd have to compile the docs with "make html STYLE=website" in the > doc subdir, and tweak one of the CSS files there (I think it's > docs-complete.css) so that it references your local copy instead of > fetching it from the website. I should have known to put the css in a separate patch. > I'm not clear on what exactly becomes visible when one hovers over > what. Can you please share a screenshot? Attached are 2 screenshots. I don't know why, but for some reason you can't see the mouse pointer. "#' shows up when the mouse is anywhere over the html heading element. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Thu, 23 Mar 2023 08:24:48 +0100 Brar Piening wrote: > On 23.03.2023 at 04:09, Karl O. Pinc wrote: > > Sorry for the extra work I've put you through. > > No problem. As always I've learnt something which may help me in the > future. I don't know about you, but sadly, my brain eventually leaks. ;-) > > I'm attaching 2 patches > > with only minor changes. > > 001-add-needed-ids_v1.patch > > > > This separates out the addition of ids from the XSLT changes, just > > to keep things tidy. > > 002-make_html_ids_discoverable_v4.patch > > > > I changed the linked text, the #, so that the leading space > > is not linked. This is arguable, as the extra space makes > > it easier to put the mouse on the region. > I tend to prefer a slightly bigger mouseover-region but I don't > really mind. I'm leaving it for the committer to review. > I've changed the format of > > the error message. What do you think? (Try it out by _not_ > > applying 001-add-needed-ids_v1.patch.) > > > > Also, the error message now has leading and trailing newlines to > > make it stand out. Including the error message/make output here, so everyone can see easily. -- /usr/bin/xsltproc --nonet --path . --stringparam pg.version '16devel' stylesheet.xsl postgres-full.xml Ids are required in order to provide the public HTML documentation with stable URLs for element content; id missing at: /book[@id = 'postgres']/part[@id = 'appendixes']/appendix[@id = 'contrib']/sect1[@id = 'pgwalinspect']/sect2[@id = 'pgwalinspect-funcs']/variablelist no result for postgres-full.xml make: *** [Makefile:146: html-stamp] Error 10 -- > I like it and think it's ready for commiter. I've marked it ready for the committer in the commitfest. > > Outstanding questions (for committer?): > > > > The 002-make_html_ids_discoverable_v4.patch generates xhtml , > > , etc. attributes using a XSLT element with a > > "namespace" attribute. > > I'm not sure I follow. I cannot see any namespacing weirdness in my > output. There's nothing weird in the output, it's all about how you're generating it in the xslt with
Re: doc: add missing "id" attributes to extension packaging page
On Tue, 21 Mar 2023 23:16:25 +0100
Brar Piening wrote:
> On 17.01.2023 at 23:43, Karl O. Pinc wrote:
> > It's good you asked. After poking about the XSLT 1.0 spec to
> > refresh my memory I abandoned the "mode" approach entirely. The
> > real "right way" is to use the import mechanism.
> After quite some time trying to figure out why things don't work as
> intended, I ended up reading the XSLT 1.0 spec.
>
> As the name already suggests, is closely related
> to with the difference that it *applies* a
> *template rule* from an imported style sheet instead of applying a
> template rule from the current style sheet
> (https://www.w3.org/TR/1999/REC-xslt-19991116#apply-imports). What it
> does not do is *calling* a *named template*
> (https://www.w3.org/TR/1999/REC-xslt-19991116#named-templates).
>
> What this basically means is that in XSLT 1.0 you can use
> to override template rules ( match="this-pattern-inside-match-makes-it-a-template-rule">) but you
> cannot use it to override named templates ( name="this-id-inside-name-makes-it-a-named-template">). If you want to
> override named templates you basically have to duplicate and change
> them.
>
> While there are mechanisms to call overriden named templates in XSLT
> 3, this is out of scope here, since we're bound to XSLT 1.0
(It was reassuring to see you take the steps above; I once did exactly
the same with and had the same excitements and disappointments. I
feel validated. ;-)
(One of my disappointments is that xsltproc supports only XSLT 1.0,
and nothing later. IIRC, apparently one big reason is not the amount
work needed to develop the program but the work required to develop a
test suite to validate conformance.)
> As a consequence, there was little I could change in the initial patch
> to avoid the code duplication and all attempts to do so, ultimately
> led to even longer and more complex code without really reducing the
> amount of duplication.
You're quite right. I clearly didn't have my XSLT turned on. Importing
only works when templates are matched, not called by name.
Sorry for the extra work I've put you through.
> The approach actually does work in the
> varlistentry case, although this doesn't really change a lot
> regarding the length of the patch (it's a bit nicer though since in
> this case it really avoids duplication).
You've put in a lot of good work. I'm attaching 2 patches
with only minor changes.
001-add-needed-ids_v1.patch
This separates out the addition of ids from the XSLT changes, just
to keep things tidy. Content is from your patch.
002-make_html_ids_discoverable_v4.patch
I changed the linked text, the #, so that the leading space
is not linked. This is arguable, as the extra space makes
it easier to put the mouse on the region. But it seems
tidy.
I've tided up so the lines are no longer than 80 chars.
> I've also taken the advice
> to terminate the build and print the xpath if a required id is
> missing.
This looks awesome. I love the xpath! I've changed the format of the
error message. What do you think? (Try it out by _not_ applying
001-add-needed-ids_v1.patch.)
Also, the error message now has leading and trailing newlines to make
it stand out. I'm normally against this sort of thing but thought I'd
add it anyway for others to review.
I'm ready to send these on to a committer but if you don't
like what I did please send more patches for me to review.
Outstanding questions (for committer?):
The 002-make_html_ids_discoverable_v4.patch generates xhtml ,
, etc. attributes using a XSLT element with a
"namespace" attribute. I'm unclear on the relationship PG has with
xhtml and namespaces. Looks right to me, since the generated html has
the same namespace name appearing in the xmlns attribute of the html
tag, but somebody who knows more than me might want to review this.
Using the namespace attribute does not seem to have affected the
generated html, as far as my random sampling of output can tell.
What character should be used to represent a link anchor?
I've left your choice of "#" in the patch.
If we go to unicode, My preference is the text paperclip ︎
Here's a table of all the choices I came up with, there may be others
that are suitable. (The hex representations are Python 3 string
literals. So print("\U0001f4ce\ufe0e") prints the text paperclip.)
Hash mark # (ASCII, used in the patch, \u0023)
Anchor ⚓ \u2693
Place of interest ⌘ \u2318
Bullseye ◎ \u25ce
Link (emoji variant) \U0001f517
Link (text variant)︎ \U0001f517\ufe0e
Paperclip (emoji variant) \U0001f4ce
Paperclip (text variant) ︎ \U0001f4ce\ufe0e
Regards,
Karl
Free Software: "You don't pay back, you pay forward.&quo
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
Hi Alvaro, On Thu, 9 Mar 2023 10:22:49 +0100 Alvaro Herrera wrote: > On 2023-Jan-22, Karl O. Pinc wrote: > > > Actually, this CSS, added to doc/src/sgml/stylesheet.css, > > makes the column spacing look pretty good: > Okay, this looks good to me too. However, for it to actually work, we > need to patch the corresponding CSS file in the pgweb repository too. > I'll follow up in the other mailing list. Do you also like the page breaking in the PDF for each contributed package, per the v10-0002-Page-break-before-sect1-in-contrib-appendix-when-pdf.patch of https://www.postgresql.org/message-id/20230122144246.0ff87372%40slate.karlpinc.com ? No need to reply if I don't need to do anything. (I didn't want the patch to get lost.) Thanks for the review. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Tue, 14 Feb 2023 12:13:18 -0800
Andres Freund wrote:
> A small note: As-is this fails on CI, because we don't allow network
> access during the docs build anymore (since it always fails these
> days):
>
> https://cirrus-ci.com/task/5474029402849280?logs=docs_build#L297
>
> [17:02:03.114] time make -s -j${BUILD_JOBS} -C doc
> [17:02:04.092] I/O error : Attempt to load network entity
> http://cdn.docbook.org/release/xsl/current/html/sections.xsl
> [17:02:04.092] warning: failed to load external entity
> "http://cdn.docbook.org/release/xsl/current/html/sections.xsl;
> [17:02:04.092] compilation error: file stylesheet-html-common.xsl
> line 17 element import [17:02:04.092] xsl:import : unable to load
> http://cdn.docbook.org/release/xsl/current/html/sections.xsl
This makes me think that it would be useful to add --nonet to the
xsltproc invocations. That would catch this error before it goes to
CI.
> I think this is just due to the common URL in docbook packages being
> http://docbook.sourceforge.net/release/xsl/current/*
> Because of that the docbook catalog matching logic won't work for
> that file:
>
> E.g. I have the following in /etc/xml/docbook-xsl.xml, on debian
> unstable: uriStartString="http://docbook.sourceforge.net/release/xsl/;
> catalog="file:///usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml"/>
>
> As all our other references use the sourceforge address, this should
> too.
Agreed.
I'm also noticing that the existing xsl:import-s all import entire
docbook stylesheets. It does not hurt to do this; the output is
unaffected, although I can't say what it means for build performance.
It does keep it simple. Only one import is needed no matter which
templates we use the import mechanism to extend. And by importing
"everything" there's no concern about any (unlikely) changes to
the the "internals" of the catalog.
Should we import only what we need or all of docbook? I don't know.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Re: drop postmaster symlink
On Wed, 25 Jan 2023 18:03:25 -0600 "Karl O. Pinc" Buried in > https://www.postgresql.org/message-id/20230107165942.748ccf4e%40slate.karlpinc.com > is the one change I see that should be made. > > > In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY > > defined which references the deleted postmaster.sgml file. > > This line needs to be removed and the > 0002-Don-t-install-postmaster-symlink-anymore.patch > updated. (Unless there's some magic going on > with the various allfiles.sgml files of which I am > not aware.) > > If this is fixed I see no other problems. Buried in the same email, and I apologize for not mentioning this, is one other bit of documentation text that might or might not need attention. > I see a possible problem at line 1,412 of runtime.sgml This says: in the postmaster's startup script just before invoking the postmaster. Depending on how this is read, it could be interpreted to mean that a "postmaster" binary is invoked. It might be more clear to write: ... just before invoking postgres. Or it might not be worth bothering about; at this point, probably not, but I thought you might want the heads-up anyway. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: drop postmaster symlink
Hello, Somehow I missed the email changing the status of this back to "needs review". Buried in https://www.postgresql.org/message-id/20230107165942.748ccf4e%40slate.karlpinc.com is the one change I see that should be made. > In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY > defined which references the deleted postmaster.sgml file. This line needs to be removed and the 0002-Don-t-install-postmaster-symlink-anymore.patch updated. (Unless there's some magic going on with the various allfiles.sgml files of which I am not aware.) If this is fixed I see no other problems. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Sun, 22 Jan 2023 14:42:46 -0600 "Karl O. Pinc" wrote: > Attached are 2 patches: > > v10-0001-List-trusted-and-obsolete-extensions.patch > > List trusted extenions in 4 columns, with the CSS altered > to put spacing between vertical columns. In theory, a number of other simplelist presentations could benefit from this. For example, in the Data Types Boolean Type section the true truth values are presently listed vertically, like so: true yes on 1 Instead they could still be listed 'type="vert"' (the default), but with 'columns="4"', to produce something like: trueyeson1 This stands out just as much, but takes less space on the page. Likewise, perhaps some tables are tables instead of simplelists just because putting simplelists into columns was so ugly. I'll leave such modifications to others, at least for now. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Sun, 22 Jan 2023 08:09:03 -0600
"Karl O. Pinc" wrote:
> On Sat, 21 Jan 2023 08:11:43 -0600
> "Karl O. Pinc" wrote:
>
> > Attached are 2 v9 patch versions. I don't think I like them.
> > I think the v8 versions are better. But I thought it
> > wouldn't hurt to show them to you.
> >
> > On Fri, 20 Jan 2023 14:22:25 -0600
> > "Karl O. Pinc" wrote:
> >
> > > Attached are 2 alternatives:
> > > (They touch separate files so the ordering is meaningless.)
> > >
> > >
> > > v8-0001-List-trusted-and-obsolete-extensions.patch
> > >
> > > Instead of putting [trusted] and [obsolete] in the titles
> > > of the modules, like v7 does, add a list of them into the text.
> > >
> >
> > v9 puts the list in vertical format, 5 columns.
> >
> > But the column spacing in HTML is ugly, and I don't
> > see a parameter to set to change it. I suppose we could
> > do more work on the stylesheets, but this seems excessive.
>
> Come to think of it, this should be fixed by using CSS
> with a
>
> table.simplelist
Actually, this CSS, added to doc/src/sgml/stylesheet.css,
makes the column spacing look pretty good:
/* Adequate spacing between columns in a simplelist non-inline table */
.simplelist td { padding-left: 2em; padding-right: 2em; }
(No point in specifying table, since td only shows up in tables.)
Note that the default simplelist type value is "vert", causing a 1
column vertical display. There are a number of these in the
documenation. I kind of like what the above css does to these
layouts. An example would be the layout in
doc/src/sgml/html/datatype-boolean.html, which is the "Data Types"
section "Boolean Type" sub-section.
For other places affected see: grep -l doc/src/sgml/*.sgml simplelist
Attached are 2 patches:
v10-0001-List-trusted-and-obsolete-extensions.patch
List trusted extenions in 4 columns, with the CSS altered
to put spacing between vertical columns. I changed this
from the 5 columns of v9 because with 5 columns there
was a little bit of overflow into the right hand margin
of a US-letter PDF. The PDF still has an ugly page
break right before the table. To avoid that use the v8
version, which presents the list inline.
v10-0002-Page-break-before-sect1-in-contrib-appendix-when-pdf.patch
This is exactly like the v8 version. See my comments earlier
about v8 v.s. v9.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index 12c79b798b..b9f3268cad 100644
--- a/doc/src/sgml/contrib.sgml
+++ b/doc/src/sgml/contrib.sgml
@@ -84,6 +84,32 @@ CREATE EXTENSION extension_name;
provide access to outside-the-database functionality.
+ These are the trusted extensions:
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Many extensions allow you to install their objects in a schema of your
choice. To do that, add SCHEMA
@@ -100,6 +126,15 @@ CREATE EXTENSION extension_name;
component for details.
+
+ These modules and extensions are obsolete:
+
+
+
+
+
+
+
diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css
index 6410a47958..61d8a6537d 100644
--- a/doc/src/sgml/stylesheet.css
+++ b/doc/src/sgml/stylesheet.css
@@ -163,3 +163,6 @@ acronym { font-style: inherit; }
width: 75%;
}
}
+
+/* Adequate spacing between columns in a simplelist non-inline table */
+table.simplelist td { padding-left: 2em; padding-right: 2em; }
diff --git a/doc/src/sgml/stylesheet-fo.xsl b/doc/src/sgml/stylesheet-fo.xsl
index 0c4dff92c4..68a46f9e24 100644
--- a/doc/src/sgml/stylesheet-fo.xsl
+++ b/doc/src/sgml/stylesheet-fo.xsl
@@ -132,4 +132,12 @@
+
+
+
+
+
+
+
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Sat, 21 Jan 2023 08:11:43 -0600 "Karl O. Pinc" wrote: > Attached are 2 v9 patch versions. I don't think I like them. > I think the v8 versions are better. But I thought it > wouldn't hurt to show them to you. > > On Fri, 20 Jan 2023 14:22:25 -0600 > "Karl O. Pinc" wrote: > > > Attached are 2 alternatives: > > (They touch separate files so the ordering is meaningless.) > > > > > > v8-0001-List-trusted-and-obsolete-extensions.patch > > > > Instead of putting [trusted] and [obsolete] in the titles > > of the modules, like v7 does, add a list of them into the text. > > v9 puts the list in vertical format, 5 columns. > > But the column spacing in HTML is ugly, and I don't > see a parameter to set to change it. I suppose we could > do more work on the stylesheets, but this seems excessive. Come to think of it, this should be fixed by using CSS with a table.simplelist selector. Or something along those lines. But I don't have a serious interest in proceeding further. A inline list seems good enough, even if it does not stand out in a visual scan of the page. There is a certain amount of visual-standout due to all the hyperlinks next to each other in the inline presentation. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
Attached are 2 v9 patch versions. I don't think I like them. I think the v8 versions are better. But I thought it wouldn't hurt to show them to you. On Fri, 20 Jan 2023 14:22:25 -0600 "Karl O. Pinc" wrote: > Attached are 2 alternatives: > (They touch separate files so the ordering is meaningless.) > > > v8-0001-List-trusted-and-obsolete-extensions.patch > > Instead of putting [trusted] and [obsolete] in the titles > of the modules, like v7 does, add a list of them into the text. v9 puts the list in vertical format, 5 columns. But the column spacing in HTML is ugly, and I don't see a parameter to set to change it. I suppose we could do more work on the stylesheets, but this seems excessive. It looks good in PDF, but the page break in the middle of the paragraph is ugly. (US-Letter) Again (without forcing a hard page break by frobbing the stylesheet and adding a processing instruction), I don't see a a good way to fix the page break. (sagehill.net says that soft page breaks don't work. I didn't try it.) > v8-0002-Page-break-before-sect1-in-contrib-appendix-when-pdf.patch > > This frobs the PDF style sheet so that when sect1 is used > in the appendix for the contrib directory, there is a page > break before every sect1. This puts each module/extension > onto a separate page, but only for the contrib appendix. > > Aside from hardcoding the "contrib" id, which I suppose isn't > too bad since it's publicly exposed as a HTML anchor (or URL > component?) and unlikely to change, this also means that the > contrib documentation can't use instead of . v9 supports using instead of just . But I don't know that it's worth it -- the appendix is committed to sect* entities. Once you start with sect* the stylesheet does not allow "section" use to be interspersed. All the sect*s would have to be changed to "section" throughout the appendix and I don't see that happening. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/stylesheet-fo.xsl b/doc/src/sgml/stylesheet-fo.xsl index 0c4dff92c4..9ce36c7279 100644 --- a/doc/src/sgml/stylesheet-fo.xsl +++ b/doc/src/sgml/stylesheet-fo.xsl @@ -132,4 +132,12 @@ + + + + + + + diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml index 12c79b798b..077184d90d 100644 --- a/doc/src/sgml/contrib.sgml +++ b/doc/src/sgml/contrib.sgml @@ -84,6 +84,31 @@ CREATE EXTENSION extension_name; provide access to outside-the-database functionality. + These are the trusted extensions: + + + + + + + + + + + + + + + + + + + + + + + + Many extensions allow you to install their objects in a schema of your choice. To do that, add SCHEMA @@ -100,6 +125,15 @@ CREATE EXTENSION extension_name; component for details. + + These modules and extensions are obsolete: + + + + + + +
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Fri, 20 Jan 2023 14:22:25 -0600 "Karl O. Pinc" wrote: > v8-0001-List-trusted-and-obsolete-extensions.patch > > Instead of putting [trusted] and [obsolete] in the titles > of the modules, like v7 does, add a list of them into the text. The list is inline. It might be worthwhile experimenting with a tabular list, like that produced by: But only for the list of trusted extensions. There's not enough obsolete extensions to do anything but inline. (IMO) Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Fri, 20 Jan 2023 20:12:38 +0100 Alvaro Herrera wrote: > Ah, I wanted to attach the two remaining patches and forgot. Attached are 2 alternatives: (They touch separate files so the ordering is meaningless.) v8-0001-List-trusted-and-obsolete-extensions.patch Instead of putting [trusted] and [obsolete] in the titles of the modules, like v7 does, add a list of them into the text. v8-0002-Page-break-before-sect1-in-contrib-appendix-when-pdf.patch This frobs the PDF style sheet so that when sect1 is used in the appendix for the contrib directory, there is a page break before every sect1. This puts each module/extension onto a separate page, but only for the contrib appendix. Aside from hardcoding the "contrib" id, which I suppose isn't too bad since it's publicly exposed as a HTML anchor (or URL component?) and unlikely to change, this also means that the contrib documentation can't use instead of . Sometimes I think I only know enough XSLT to get into trouble. While v8 is "right", I can't say if it is a good idea/good practice. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml index 8816e06337..87833915e9 100644 --- a/doc/src/sgml/contrib.sgml +++ b/doc/src/sgml/contrib.sgml @@ -84,6 +84,31 @@ CREATE EXTENSION extension_name; provide access to outside-the-database functionality. + These are the trusted extensions: + + + + + + + + + + + + + + + + + + + + + + + + Many extensions allow you to install their objects in a schema of your choice. To do that, add SCHEMA @@ -100,6 +125,15 @@ CREATE EXTENSION extension_name; component for details. + + These modules and extensions are obsolete: + + + + + + + diff --git a/doc/src/sgml/stylesheet-fo.xsl b/doc/src/sgml/stylesheet-fo.xsl index 0c4dff92c4..68a46f9e24 100644 --- a/doc/src/sgml/stylesheet-fo.xsl +++ b/doc/src/sgml/stylesheet-fo.xsl @@ -132,4 +132,12 @@ + + + + + + +
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Fri, 20 Jan 2023 20:12:03 +0100 Alvaro Herrera wrote: > On 2023-Jan-20, Karl O. Pinc wrote: > > > On Fri, 20 Jan 2023 12:42:31 +0100 > > Alvaro Herrera wrote: > > > > Hmm, I didn't know that. I guess I can put it back. My own > > > instinct is to put the most important stuff first, not last, but > > > if research says to do otherwise, fine, let's do that. > > > > A quick google on the subject tells me that I can't figure out a > > good quick google. I believe it's from the book at bottom. > > Memorability goes "end", "beginning", "middle". IIRC. > > Ah well. I just put it back the way you had it. > > > > I hope somebody with more docbook-fu can comment: maybe > > > there's a way to fix it more generally somehow? > > > > What would the general solution be? > > I don't know, I was thinking that perhaps at the start of the appendix > we could have some kind of marker that says "in this chapter, the > s all get a page break", then a marker to stop that at the end > of the appendix. Or a tweak to the stylesheet, "when inside an > appendix, all s get a pagebreak", in a way that doesn't affect > the other chapters. > > The solution looks really ugly to me (in the source > code I mean), but I suppose if we discover no other way to do it, we > could do it like that. I can do a forced page break for sect1-s in the pdf stylesheet just for the contrib appendix (appendix F) by looking for a parent with an id of "contrib". That would work, but seems like a kludge. (Otherwise, you look for a parent of "appendix" and force the page break in all appendixes.) I'll send a patch. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Fri, 20 Jan 2023 12:42:31 +0100 Alvaro Herrera wrote: > On 2023-Jan-19, Karl O. Pinc wrote: > > > On Thu, 19 Jan 2023 11:03:53 -0600 > > "Karl O. Pinc" wrote: > > > > > Attached are 2 patches, a regular and a delta from your v4 review: > > > > > > contrib_v5-delta.patch.txt > > > contrib_v5.patch.txt > > > > I left your appendix title unchanged: "Additional Supplied > > Extensions and Modules". > > > > I had put "Extensions" after > > "Modules", because, apparently, things that come last in the > > sentence are most remembered by the reader. My impression is that > > more people are looking for extensions than modules. > > Hmm, I didn't know that. I guess I can put it back. My own instinct > is to put the most important stuff first, not last, but if research > says to do otherwise, fine, let's do that. A quick google on the subject tells me that I can't figure out a good quick google. I believe it's from the book at bottom. Memorability goes "end", "beginning", "middle". IIRC. > I went over all the titles again. There were a couple of mistakes > and inconsistencies, which I've fixed to the best of my knowledge. > I'm happy with 0001 now and will push shortly unless there are > complaints. > > I'm still unsure of the [trusted]/[obsolete] marker, so I split that > out to commit 0002. I would like to see more support for that before > pushing that one. > > I also put the page-split bits to another page, because it seems a bit > too clumsy. All the above sounds good to me. > I hope somebody with more docbook-fu can comment: maybe > there's a way to fix it more generally somehow? What would the general solution be? There could be a forced page break at the beginning of _every_ sect1. For PDFs. That seems a bit much, but maybe not. The only other thing I can think of that's "general" would be to force a page break for sect1-s that are in an appendix. Is any of this wanted? (Or technically "better"?) Thanks for the help. Writing for Readers By George R. Bramer, Dorothy Sedley · 1981 About this edition ISBN:9780675080453, 0675080452 Page count:532 Published:1981 Format:Hardcover Publisher:C.E. Merrill Publishing Company Original from:Pennsylvania State University Digitized:July 15, 2009 Language:English Author:George R. Bramer, Dorothy Sedley It's part of a wave of reaction against Strunk & White, where they started basing writing on research into reading. (If it's the right book.) Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Thu, 19 Jan 2023 11:03:53 -0600 "Karl O. Pinc" wrote: > Attached are 2 patches, a regular and a delta from your v4 review: > > contrib_v5-delta.patch.txt > contrib_v5.patch.txt I left your appendix title unchanged: "Additional Supplied Extensions and Modules". I had put "Extensions" after "Modules", because, apparently, things that come last in the sentence are most remembered by the reader. My impression is that more people are looking for extensions than modules. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Thu, 19 Jan 2023 13:35:17 +0100 Alvaro Herrera wrote: > On 2023-Jan-18, Karl O. Pinc wrote: > > > Oops. Already sent a revised patch that includes starting each > > module on a new page, for PDF output. I'll wait to rip that > > out after review and start a new thread if necessary. (I have not removed the PDF page breaks from the latest patch.) > Here's my review in the form of a delta patch. Love it. > I didn't find that a thing called "ISN" actually exists. Is there a > reference to that? Maybe. I came across it somewhere and it seemed useful. It's an initialism for International Standard Number. https://en.wikipedia.org/wiki/International_Standard_Number It's the same ISN as in the file name, "isn.sgml". I've frobbed the ISN related text in my response patch. (And added a line break to btree-gin.) Attached are 2 patches, a regular and a delta from your v4 review: contrib_v5-delta.patch.txt contrib_v5.patch.txt Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/btree-gin.sgml b/doc/src/sgml/btree-gin.sgml index c9efe1cccf..f0cda0e32a 100644 --- a/doc/src/sgml/btree-gin.sgml +++ b/doc/src/sgml/btree-gin.sgml @@ -1,7 +1,8 @@ - btree_gin GIN B-tree equivalent operator classes [trusted] + btree_gin + GIN B-tree equivalent operator classes [trusted] btree_gin diff --git a/doc/src/sgml/isn.sgml b/doc/src/sgml/isn.sgml index 27abdc8c66..8008c84fe4 100644 --- a/doc/src/sgml/isn.sgml +++ b/doc/src/sgml/isn.sgml @@ -1,8 +1,8 @@ - isn - data types for item numbers (ISBN, EAN, UPC, etc.) [trusted] + isn data types for international standard numbers + (ISBN, EAN, UPC, etc.) [trusted] isn diff --git a/doc/src/sgml/adminpack.sgml b/doc/src/sgml/adminpack.sgml index 184e96d7a0..04f3b52379 100644 --- a/doc/src/sgml/adminpack.sgml +++ b/doc/src/sgml/adminpack.sgml @@ -1,7 +1,7 @@ - adminpack + adminpack pgAdmin support toolpack adminpack diff --git a/doc/src/sgml/amcheck.sgml b/doc/src/sgml/amcheck.sgml index 923cbde9dd..4006c75cdf 100644 --- a/doc/src/sgml/amcheck.sgml +++ b/doc/src/sgml/amcheck.sgml @@ -1,7 +1,7 @@ - amcheck + amcheck tools to verify index consistency amcheck diff --git a/doc/src/sgml/auth-delay.sgml b/doc/src/sgml/auth-delay.sgml index 40629311b1..0571f2a99d 100644 --- a/doc/src/sgml/auth-delay.sgml +++ b/doc/src/sgml/auth-delay.sgml @@ -1,7 +1,7 @@ - auth_delay + auth_delay pause on authentication failure auth_delay diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index bb7342b120..0c4656ee30 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -1,7 +1,7 @@ - auto_explain + auto_explain log execution plans of slow queries auto_explain diff --git a/doc/src/sgml/basebackup-to-shell.sgml b/doc/src/sgml/basebackup-to-shell.sgml index 491368eb8f..b6a3b39541 100644 --- a/doc/src/sgml/basebackup-to-shell.sgml +++ b/doc/src/sgml/basebackup-to-shell.sgml @@ -1,7 +1,7 @@ - basebackup_to_shell + basebackup_to_shell example "shell" pg_basebackup module basebackup_to_shell diff --git a/doc/src/sgml/basic-archive.sgml b/doc/src/sgml/basic-archive.sgml index 60f23d2855..b4d43ced20 100644 --- a/doc/src/sgml/basic-archive.sgml +++ b/doc/src/sgml/basic-archive.sgml @@ -1,7 +1,7 @@ - basic_archive + basic_archive an example WAL archive module basic_archive diff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgml/bloom.sgml index 98d0316175..19f2b172cc 100644 --- a/doc/src/sgml/bloom.sgml +++ b/doc/src/sgml/bloom.sgml @@ -1,7 +1,7 @@ - bloom + bloom bloom filter index access method bloom diff --git a/doc/src/sgml/btree-gin.sgml b/doc/src/sgml/btree-gin.sgml index 870c25559e..f0cda0e32a 100644 --- a/doc/src/sgml/btree-gin.sgml +++ b/doc/src/sgml/btree-gin.sgml @@ -1,14 +1,15 @@ - btree_gin + btree_gin + GIN B-tree equivalent operator classes [trusted] btree_gin - btree_gin provides sample GIN operator classes that + btree_gin provides GIN operator classes that implement B-tree equivalent behavior for the data types int2, int4, int8, float4, float8, timestamp with time zone, diff --git a/doc/src/sgml/btree-gist.sgml b/doc/src/sgml/btree-gist.sgml index 92aa8e277e..c08d1fdfc3 100644 --- a/doc/src/sgml/btree-gist.sgml +++ b/doc/src/sgml/btree-gist.sgml @@ -1,7 +1,8 @@ - btree_gist + btree_gist + B-tree equivalent GiST index operators [trusted] btree_gist diff --git a/doc/src/sgml/citext.sgml b/doc/src/sgml/citext.sgml index 3df2825592..710fbdddf2 100644 --- a/doc/src/sgml/citext.sgml +++ b/doc/src/sgml/citext.sgml @@ -1,7 +1,8 @@ - citext + citext + a case-insensitive character string type [trusted] citext diff --git a/doc/src/sgml/contrib-spi.sgml b/doc/src/sgml/contrib-spi.sgml inde
Re: doc: add missing "id" attributes to extension packaging page
On Tue, 17 Jan 2023 16:43:13 -0600 "Karl O. Pinc" wrote: > It might be useful to add --nonet to the xsltproc invocation(s) > in the Makefile(s). Just in case; to keep from retrieving > stylesheets from the net. (If the option is not already there. > I didn't look.) > > If this is the first time that PG uses the XSLT import mechanism > I imagine that "things could go wrong" depending on what sort > of system is being used to build the docs. I'm not worried, > but it is something to note for the committer. Looks like doc/src/sgml/stylesheet-fo.xsl already uses xsl:import, although it is unclear to me whether the import is applied. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Wed, 18 Jan 2023 18:34:47 +0100 Alvaro Herrera wrote: > On 2023-Jan-18, Karl O. Pinc wrote: > > > On Wed, 18 Jan 2023 13:30:45 +0100 > > Alvaro Herrera wrote: > > > > > Not related to this patch: it's very annoying that in the PDF > > > output, each section in the appendix doesn't start on a blank > > > page -- which means that the doc page for many modules starts in > > > the middle of a page were the previous one ends. > > > > > I wonder if we can tweak something in the stylesheet to include a > > > page break. > > > > Would this be something to be included in this patch? > > (If I can figure it out.) > > No, I think we should do that change separately. I just didn't think > a parenthical complain was worth a separate thread for it; but if you > do create a patch, please do create a new thread (unless the current > patch in this one is committed already.) Oops. Already sent a revised patch that includes starting each module on a new page, for PDF output. I'll wait to rip that out after review and start a new thread if necessary. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Wed, 18 Jan 2023 13:25:57 +0100 Alvaro Herrera wrote: > On 2023-Jan-02, Karl O. Pinc wrote: > > Attached is a patch: contrib_v1.patch > > > > It modifies Appendix F, the contrib directory. > > > > It adds brief text into the titles shown in the > > table of contents so it's easier to tell what > > each module does. It also suffixes [trusted] or [obsolete] > > on the relevant titles. > > I'm not 100% sold on having the > "trusted" or "obsolete" marker on the titles themselves, though. Not > sure what alternative do we have, though, other than leave them out > completely. The alternative would be to have a separate table with modules for rows and "trusted" and "obsolete" columns. It seems like more of a maintenance hassle than having the markers in the titles. Let me know if you want a table. I do like having a place to look to over all the modules to see what is "trusted" or "obsolete". I suppose there could just be a table, with module names, descriptions, and trusted and obsolete flags. Instead of a table of contents for the modules the module names in the table could be links. But that'd involve suppressing the table of contents showing all the module names. And has the problem of possible mis-match between the modules listed in the table and the modules that exist. > There's a typo "equalivent" in two places. Fixed. > In passwordcheck, I would say just "check for weak passwords" or maybe > "verify password strength". I used "verify password strength". > > pg_buffercache is missing. Maybe "-- inspect state of the Postgres > buffer cache". I used "inspect Postgres buffer cache state" > For pg_stat_statements I suggest "track statistics of planning and > execution of SQL queries" I had written "track SQL query planning and execution statistics". Changed to: "track statistics of SQL planning and execution" I don't really care. If you want your version I'll submit another patch. > For sepgsql, as I understand it is strictly SELinux based, not just > "-like". So this needs rewording: "label-based, SELinux-like, > mandatory access control". Maybe "SELinux-based implementation of > mandatory access control for row-level security". Changed to: "SELinux-based row-level security mandatory access control" > xml -- typo "qeurying" Fixed. I have also made the patch put each module on a separate page when producing PDF documents. This did produce one warning, which seems unrelated to me. The pdf seems right. I also tried just "make", to be sure I didn't break anything unrelated. Seemed to work. So..., works for me. New patch attached: contrib_v2.patch Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/adminpack.sgml b/doc/src/sgml/adminpack.sgml index 184e96d7a0..04f3b52379 100644 --- a/doc/src/sgml/adminpack.sgml +++ b/doc/src/sgml/adminpack.sgml @@ -1,7 +1,7 @@ - adminpack + adminpack pgAdmin support toolpack adminpack diff --git a/doc/src/sgml/amcheck.sgml b/doc/src/sgml/amcheck.sgml index 923cbde9dd..4006c75cdf 100644 --- a/doc/src/sgml/amcheck.sgml +++ b/doc/src/sgml/amcheck.sgml @@ -1,7 +1,7 @@ - amcheck + amcheck tools to verify index consistency amcheck diff --git a/doc/src/sgml/auth-delay.sgml b/doc/src/sgml/auth-delay.sgml index 40629311b1..0571f2a99d 100644 --- a/doc/src/sgml/auth-delay.sgml +++ b/doc/src/sgml/auth-delay.sgml @@ -1,7 +1,7 @@ - auth_delay + auth_delay pause on authentication failure auth_delay diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index bb7342b120..0c4656ee30 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -1,7 +1,7 @@ - auto_explain + auto_explain log execution plans of slow queries auto_explain diff --git a/doc/src/sgml/basebackup-to-shell.sgml b/doc/src/sgml/basebackup-to-shell.sgml index 491368eb8f..b6a3b39541 100644 --- a/doc/src/sgml/basebackup-to-shell.sgml +++ b/doc/src/sgml/basebackup-to-shell.sgml @@ -1,7 +1,7 @@ - basebackup_to_shell + basebackup_to_shell example "shell" pg_basebackup module basebackup_to_shell diff --git a/doc/src/sgml/basic-archive.sgml b/doc/src/sgml/basic-archive.sgml index 60f23d2855..b4d43ced20 100644 --- a/doc/src/sgml/basic-archive.sgml +++ b/doc/src/sgml/basic-archive.sgml @@ -1,7 +1,7 @@ - basic_archive + basic_archive an example WAL archive module basic_archive diff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgml/bloom.sgml index 98d0316175..672ac2ed19 100644 --- a/doc/src/sgml/bloom.sgml +++ b/doc/src/sgml/bloom.sgml @@ -1,7
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Wed, 18 Jan 2023 13:30:45 +0100 Alvaro Herrera wrote: > Not related to this patch: it's very annoying that in the PDF output, > each section in the appendix doesn't start on a blank page -- which > means that the doc page for many modules starts in the middle of a > page were the previous one ends. > I wonder if we can tweak something in the stylesheet to include a page > break. Would this be something to be included in this patch? (If I can figure it out.) Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Tue, 17 Jan 2023 19:13:38 +0100
Brar Piening wrote:
> On 17.01.2023 at 14:12, Karl O. Pinc wrote:
> > If you're not willing to try I am willing to see if I can
> > produce an example to work from. My XSLT is starting to
> > come back.
>
> I'm certainly willing to try but I'd appreciate an example in any
> case.
It's good you asked. After poking about the XSLT 1.0 spec to refresh
my memory I abandoned the "mode" approach entirely. The real "right
way" is to use the import mechanism.
I've attached a patch that "wraps" the section.heading template
and adds extra stuff to the HTML output generated by the stock
template. (example_section_heading_override.patch)
There's a bug. All that goes into the html is a comment, not
a hoverable link. But the technique is clear.
On my system (Debian 11, bullseye) I found the URI to import
by looking at:
/usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml
(Which is probably the right place to look.)
Ultimately, that file is findable via: /etc/xml/catalog
The "best way" on
Debian is: /usr/share/doc/docbook-xsl/README.gz
In other words, the README that comes with the style sheets.
Supposedly, the href=URLs are really URIs and will be good
no matter what/when. The XSLT processor should know to look
at the system catalogs and read the imported style sheet
from the local file system.
It might be useful to add --nonet to the xsltproc invocation(s)
in the Makefile(s). Just in case; to keep from retrieving
stylesheets from the net. (If the option is not already there.
I didn't look.)
If this is the first time that PG uses the XSLT import mechanism
I imagine that "things could go wrong" depending on what sort
of system is being used to build the docs. I'm not worried,
but it is something to note for the committer.
> My XSLT skills are mostly learning by doing combined with trial and
> error.
I think of XSLT as a functional programming language. Recursion is
a big deal, and data directed programming can be a powerful technique
because XSLT is so good with data structures.
(https://mitp-content-server.mit.edu/books/content/sectbyfn/books_pres_0/6515/sicp.zip/full-text/book/book-Z-H-17.html#%_sec_2.4.3)
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl
index 9df2782ce4..88e084e190 100644
--- a/doc/src/sgml/stylesheet-html-common.xsl
+++ b/doc/src/sgml/stylesheet-html-common.xsl
@@ -12,6 +12,10 @@
all HTML output variants (chunked and single-page).
-->
+
+http://cdn.docbook.org/release/xsl/current/html/sections.xsl"/>
+
@@ -301,4 +305,53 @@ set toc,title
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ #
+
+
+
+
+
+ id_link
+
+ #
+
+
+
+
+
+
+
+ element without id. Please add an id to make it usable
+ as stable anchor in the public HTML documentation.
+
+
+ Please add an id to the
+
+ element in the sgml source code.
+
+
+
+
+
+
diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css
index 6410a47958..e4174e0613 100644
--- a/doc/src/sgml/stylesheet.css
+++ b/doc/src/sgml/stylesheet.css
@@ -163,3 +163,13 @@ acronym { font-style: inherit; }
width: 75%;
}
}
+
+/* Links to ids of headers and definition terms */
+a.id_link {
+color: inherit;
+visibility: hidden;
+}
+
+*:hover > a.id_link {
+visibility: visible;
+}
Re: doc: add missing "id" attributes to extension packaging page
On Tue, 17 Jan 2023 06:57:23 +0100 Brar Piening wrote: > On 17.01.2023 at 02:05, Karl O. Pinc wrote: > > Or maybe the right way is to set a mode at the very top, > > the first apply-templates call, and not mess with the > > built-in templates at all. (You'd write your own > > "postgres-mode" templates the same way, to "wrap" > > and call the default templates.) > > > > Think of the mode as an implicit argument that's preserved and > > passed down through each template invocation without having to > > be explicitly specified by the calling code. > > I think the document you're missing is [1]. > > There are multiple ways to customize DocBook XSL output and it sounds > like you want me to write a customization layer which I didn't do > because there is precedent that the typical "way to do it" (TM) in the > PostgreSQL project is [2]. > > Regards, > > Brar > > [1] http://www.sagehill.net/docbookxsl/CustomizingPart.html > [2] http://www.sagehill.net/docbookxsl/ReplaceTemplate.html > Sagehill is normally vary good. But in this case [2] does not apply. Or rather it applies but it is overkill because you do not want to replace what a template is producing. You want to add to what a template is producing. So you want to wrap the template, with your new code adding output before/ after what the original produces. [1] does not contain this technique. If you're not willing to try I am willing to see if I can produce an example to work from. My XSLT is starting to come back. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Mon, 16 Jan 2023 11:14:35 -0600 "Karl O. Pinc" wrote: > On Sun, 15 Jan 2023 18:01:50 -0600 > "Karl O. Pinc" wrote: > > > Regards XSLT: > > > > I believe the XSLT needs work. > In XSLT 1.0 there is no xml:default-mode. So I _think_ what you do > then is modify the built-in template rules so that the (default) > template (mode='') is invoked when there is no 'postgres-mode' > version of the template, but otherwise the 'postgres-mode' version of > the template is invoked. Your 'postgres-mode' templates will > xsl:call-template the default template, adding whatever they want to > the output produced by the default template. Or maybe the right way is to set a mode at the very top, the first apply-templates call, and not mess with the built-in templates at all. (You'd write your own "postgres-mode" templates the same way, to "wrap" and call the default templates.) Think of the mode as an implicit argument that's preserved and passed down through each template invocation without having to be explicitly specified by the calling code. Regards Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Sun, 15 Jan 2023 18:01:50 -0600 "Karl O. Pinc" wrote: > Regards XSLT: > > I believe the XSLT needs work. > I believe that overriding the XSLT by copying the original and making > modifications is the "wrong way" (TM). I think that the right way is > to declare a xsl:default-mode somewhere in the stylesheets. There > does not seem to be one, so the default mode for PG doc processing > could be something like "postgres-mode". And I'd expect it to be > declared somewhere right at the top of the xml hierarchy. (I forget > what that is, probably something like "document" or "book" or > something.) Then, you'd write your for the match="varlistentry"> and with > a mode of "postgres-mode", and have your templates call the > "traditional default", "modeless", templates. That way your not > copying and patching upstream code, but are instead, in effect, > calling it as a subroutine. > > This should work especially well since, I think, you're just adding > new output to what the upstream templates do. You're not trying to > insert new output into the middle of the stock output or otherwise > modify the stock output. > > You're adding only about 6 lines of XSLT to the upstream templates, > and copying 100+ lines. There must be a better way. > > See: https://www.w3.org/TR/xslt-30/#modes > > I've never tried this, although I do recall doing something or another > with modes in the past. And I've not gone so far as to figure out > (again?) how to call a "modeless template", so you can invoke the > original, upstream, templates. And setting a default mode seems like > something of a "big hammer", so should probably be checked over by > someone who's more steeped in Postgres docs than myself. (Like a > committer. :) But I believe it is the way to go. To make it work > you'll need to figure out the XSLT mode selection process and make > sure that it first selects the "postgres-mode", and then the modeless > templates, and also still works right when a template calls another > and explicitly sets a mode. Drat. I forgot. We're using xsltproc which is XSLT 1.0. So this is the relevant spec: https://www.w3.org/TR/1999/REC-xslt-19991116#modes In XSLT 1.0 there is no xml:default-mode. So I _think_ what you do then is modify the built-in template rules so that the (default) template (mode='') is invoked when there is no 'postgres-mode' version of the template, but otherwise the 'postgres-mode' version of the template is invoked. Your 'postgres-mode' templates will xsl:call-template the default template, adding whatever they want to the output produced by the default template. See: https://www.w3.org/TR/1999/REC-xslt-19991116#built-in-rule Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Sun, 15 Jan 2023 18:01:50 -0600 "Karl O. Pinc" wrote: > Regards XSLT: > > I believe the XSLT needs work. I also think that the XSLT should error and halt when there's no id (in the expected places). Instead of just giving a warning and keeping going. Otherwise they'll constantly be ignored warnings and periodically there will have to be patches to supply missing ids. To solve the "which id is missing where so I can fix it" problem, I propose the error text show the chapter title, all the enclosing sub-section titles, and any previous existing varlistentry ids occurring before the tag with the missing attribute. At least for varlistentry-s. For sections you could do chapter and enclosing sub-section titles and the title of the section with the problem. That should be enough for an author to find the place in the source sgml that needs fixing. Maybe, possibly, you can see how this is done by looking at whatever XSLT there is that automatically generates ids for sections without ids, so that the table of contents have something to link to. In any case, XSLT is really good at "looking at" parent/enclosing XML, so producing a useful error message shouldn't be _that_ hard. I've definitely done this sort of thing before so I can tell you it's readily doable. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
Hi Brar,
Here's my first review of the make_html_ids_discoverable.patch.
Overall:
To start with, I'd like to say I like the goal and how everything
works when the patch is applied. I was a bit skeptical, thinking that
the whole thing was going to be distracting when reading the docs or
otherwise odd, but it really works.
The thought comes to mind to take accessibility into account. I am
unclear on what that would mean in this case.
Regards CSS:
The CSS looks good. Also, it passes the W3C CSS validator (
https://jigsaw.w3.org/css-validator/) for CSS level 3.
Regards XSLT:
I believe the XSLT needs work.
Without really knowing the style of PG's XSLT, I'm going to suggest
putting the comment
above the new XSLT. Precede the comment with 2 blank lines and follow
it by one blank line. The idea is to make the code you're adding an
identifiable "separate section".
I have a question, it is the docbook html stylesheets for XML that are
being overridden? It looks like it, and what else would it be, but
(Sorry, I'm a bit stale on this stuff.)
I believe that overriding the XSLT by copying the original and making
modifications is the "wrong way" (TM). I think that the right way is
to declare a xsl:default-mode somewhere in the stylesheets. There
does not seem to be one, so the default mode for PG doc processing
could be something like "postgres-mode". And I'd expect it to be
declared somewhere right at the top of the xml hierarchy. (I forget
what that is, probably something like "document" or "book" or
something.) Then, you'd write your for the and with a
mode of "postgres-mode", and have your templates call the "traditional
default", "modeless", templates. That way your not copying and
patching upstream code, but are instead, in effect, calling it as a
subroutine.
This should work especially well since, I think, you're just adding
new output to what the upstream templates do. You're not trying to
insert new output into the middle of the stock output or otherwise
modify the stock output.
You're adding only about 6 lines of XSLT to the upstream templates,
and copying 100+ lines. There must be a better way.
See: https://www.w3.org/TR/xslt-30/#modes
I've never tried this, although I do recall doing something or another
with modes in the past. And I've not gone so far as to figure out
(again?) how to call a "modeless template", so you can invoke the
original, upstream, templates. And setting a default mode seems like
something of a "big hammer", so should probably be checked over by
someone who's more steeped in Postgres docs than myself. (Like a
committer. :) But I believe it is the way to go. To make it work
you'll need to figure out the XSLT mode selection process and make
sure that it first selects the "postgres-mode", and then the modeless
templates, and also still works right when a template calls another
and explicitly sets a mode.
Regards visual presentation:
Here's the fun question, what to have "appear" when a section or
varlistentry with an id is hovered over?
I kind of like your choice of the "#" character as the screen element
that becomes visible when you hover over a section or varlistentry
with the mouse, which then shows you the URL of the thing over which
you are hovering. That's what's in the patch now. But I wonder why
you chose it. Is there some sort of standard? I've seen the anchor
Unicode character before, (⚓) \u2693. I don't find it particularly
helpful. The "place of interest" symbol,(⌘) \u2318, might be nice.
There is (◎), \u25ce, the "bullseye" symbol. There is the link symbol
(), \U0001f517. Like the anchor, it has generally been confusing
when I come across it. The link symbol with the "text variant form",
(︎) \U0001f517\ufe0e, looks more link an actual chain and is somewhat
better. (The opposite of "text variant" is "emoji variant".) There
is also the paperclip, () \U0001f4ce. And the paperclip text
variant, (︎) \U0001f4ce\ufe0e.
Of all the Unicode choices above, I think I prefer the last. The text
paperclip. ︎
(The hex representations above are Python 3 string literals. So
print("\U0001f4ce\ufe0e") prints the text paperclip.)
The actual representation of the various Unicode characters is going
to depend on the font. (Things change for me when looked at in an
email v.s. in a browser.)
The question I have is should we use "Unicode icons" at all or is
plain old UTF-8 good enough for us (because it was good enough for our
ancestors ;) ? Of course an image is also possible For now I'm
not going to advocate for a change from the "#" already used in the
patch.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On Sun, 15 Jan 2023 07:11:30 +0100 Brar Piening wrote: > On 03.01.2023 at 01:00, Karl O. Pinc wrote: > > Attached is a patch: contrib_v1.patch > > > > It modifies Appendix F, the contrib directory. > > Review: > It adds a brief explanatory part to the headers of all contrib modules > The explanatory parts added make sense to me, althogh I'm not an > expert in all the different contrib modules. Neither am I. I read the beginning of each module's docs and made a best-effort. There may sometimes be a better summary phrase to describe a module/extension. Thanks for the review. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Mon, 09 Jan 2023 15:18:18 -0500 Tom Lane wrote: > Brar Piening writes: > > On 09.01.2023 at 03:38, vignesh C wrote: > >> There are couple of commitfest entries for this: > >> https://commitfest.postgresql.org/41/4041/ > >> https://commitfest.postgresql.org/41/4042/ Can one of them be > >> closed? > > > I've split the initial patch into two parts upon Álvaro's request > > in [1] so that we can discuss them separately > I pushed the ID-addition patch, with a few fixes: > It's probably going to be necessary to have follow-on patches, > because I'm sure there is stuff in the pipeline that adds more > ID-less tags. Or do we have a way to create warnings about that? I am unclear on how to make warnings with xslt. You can make a listing of problems, but who would read it if the build completed successfully? You can make errors and abort. But my xslt and docbook and pg-docs-fu are a bit stale. I think there's more to comment on regards the xslt in the other patch, but I'll wait for the new thread for that patch. That might be where there should be warnings raised, etc. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Mon, 09 Jan 2023 15:18:18 -0500 Tom Lane wrote: > I pushed the ID-addition patch, with a few fixes: > > * AFAIK our practice is to use "-" never "_" in XML ID attributes. > You weren't very consistent about that even within this patch, and > the overall effect would have been to have no standard about that > at all, which doesn't seem great. I changed them all to "-". Apologies for not catching this. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Mon, 9 Jan 2023 08:09:02 +0100 Brar Piening wrote: > On 09.01.2023 at 03:31, vignesh C wrote: > > The patch does not apply on top of HEAD as in [1], please post a > > rebased patch: > This one applies on top of 3c569049b7b502bb4952483d19ce622ff0af5fd6 > and the documentation build succeeds. Beyond rebasing I've added a > few more ids (to make the other patch > (make_html_ids_discoverable.patch) build without warnings again) but > nothing that would justify another review. Agreed. I believe that as long as your system has xmllint installed and the documentation builds there's not a lot that can go wrong. This patch only adds lots-of-id attributes. > We probably have to move quickly with this patch since it touches > pretty much any file in the documentation and will be outdated in a > minute. +1 Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: drop postmaster symlink
On Sat, 7 Jan 2023 22:29:35 -0600 "Karl O. Pinc" wrote: > The only way I could think of to review a patch > that removes something is to report all the places > I looked where a reference to the symlink might be. I forgot to report that I also tried a `make install` and 'make uninstall`, with no problems. FWIW, I suspect the include/backend/postmaster/ directory would today be named include/backend/postgres/. Now would be the time to change the name, but I don't see it being worth the work. And while such a change wouldn't break pg, that kind of change would break something for somebody. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: drop postmaster symlink
On Sat, 7 Jan 2023 19:56:08 -0600 "Karl O. Pinc" wrote: > On Sat, 07 Jan 2023 18:38:25 -0500 > Tom Lane wrote: > > > "Karl O. Pinc" writes: > > > This is a review of Peter's 2 patches. I see only 1 small > > > problem. ... > > Hmm ... I thought this patch was about getting rid of the > > admittedly-obsolete installed symlink. ... > No. It's about getting rid of the symlink. The only way I could think of to review a patch that removes something is to report all the places I looked where a reference to the symlink might be. Then report what I found each place I looked and report if there's a problem, or might be. That way the committer knows where I didn't look if there's more that needs removing. Apologies that this was not clear. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: drop postmaster symlink
On Sat, 7 Jan 2023 19:33:38 -0500 Joe Conway wrote: > On 1/7/23 18:38, Tom Lane wrote: > > "Karl O. Pinc" writes: > >> This is a review of Peter's 2 patches. I see only 1 small > >> problem. The small problem is a reference to a deleted file. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: drop postmaster symlink
On Sat, 07 Jan 2023 18:38:25 -0500 Tom Lane wrote: > "Karl O. Pinc" writes: > > This is a review of Peter's 2 patches. I see only 1 small problem. > > > > > Looking at the documentation, a "postmaster" in the glossary is > > defined as the controlling process. This works; it needs to be > > called something. There is still a postmaster.pid (etc.) in the > > data directory. > > > The word "postmaster" (case insensitive) shows up 84 times in the > > documentation. I looked at all of these. > > Hmm ... I thought this patch was about getting rid of the > admittedly-obsolete installed symlink. If it's trying to get rid of > the "postmaster" terminology for our parent process, I'm very strongly > against that, either as regards to code or documentation. No. It's about getting rid of the symlink. I was grepping around to look for use of the symlink, started with the glossary just to be sure, and saw that "postmaster" is consistently (I think) used to refer to the controlling, parent, process. And wrote down what I was doing and what I found as I went along. And then sent out my notes. Sorry for the confusion. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: drop postmaster symlink
Hello,
This is a review of Peter's 2 patches. I see only 1 small problem.
+++
Looking at the documentation, a "postmaster" in the glossary is
defined as the controlling process. This works; it needs to be called
something. There is still a postmaster.pid (etc.) in the data
directory.
The word "postmaster" (case insensitive) shows up 84 times in the
documentation. I looked at all of these.
I see a possible problem at line 1,412 of runtime.sgml, the "Linux
Memory Overcommit" section. It talks about the postmaster's startup
script invoking the postmaster. It might, possibly, be better to say
that "postgres" is invoked, that being the name of the invoked program.
There's a similar usage at line 1,416. On the other hand, the existing
text makes it quite clear what is going on and there's a lot to be said
for consistently using the word "postmaster". I mention this only in
case somebody deems it significant. Perhaps there's a way to use
markup, identifying "postgres" as a program with a name in the file
system, to make things more clear. Most likely, nobody will care.
In doc/src/sgml/ref/allfiles.sgml at line 222 there is an ENTITY
defined which references the deleted postmaster.sgml file. Since I
did a maintainer-clean, and the patch deletes the postmaster.sgml
file, and I don't see any references to the entity in the docs, I
believe that this line should be removed. (In other words, I don't
think this file is automatically maintained.)
After applying the patches the documentation seems to build just fine.
I have not run contrib/start-scripts/{freebsd,linux}, but the patch
looks fine and the result of applying the patch looks fine, and the
patch is a one-line simple replacement of "postmaster" with "postgres"
so I expect no problems.
The modification to src/port/path.c is in a comment, so it will surely
not affect anything at runtime. And the changed comment looks right
to me.
There's thousands of lines of comments in the code where "postmaster"
(case insensitive) shows up after applying the patches. I've not
reviewed any of these. Or looked for odd variable names, or
references in the code to the postmaster binary - by name, etc. I'm
not sure where it would make sense to look for such problems.
Aside from the "allfiles.sgml" problem, I see no reason why these 2
patches should not be applied. As mentioned by others, I don't have
strong feelings about getting rid of the postmaster symlink. But I
did pick this patch to review because I remember, once upon a time,
being slightly confused by a "postmaster" in the process list.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Re: doc: add missing "id" attributes to extension packaging page
On Tue, 3 Jan 2023 21:35:09 +0100 Brar Piening wrote: > On 02.01.2023 at 21:53, Karl O. Pinc wrote: > > If the author will look over my version of the patch I believe it > > can be approved and sent on to the committers. > > LGTM. Approved for committer! Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Doc: Rework contrib appendix -- informative titles, tweaked sentences
Hi, Attached is a patch: contrib_v1.patch It modifies Appendix F, the contrib directory. It adds brief text into the titles shown in the table of contents so it's easier to tell what each module does. It also suffixes [trusted] or [obsolete] on the relevant titles. I added the word "extension" into the appendix title because I always have problems scanning through the appendix and finding the one to do with extensions. The sentences describing what the modules are and how to build them have been reworked. Some split in 2, some words removed or replaced, etc. I introduced the word "component" because the appendix has build instructions for command line programs as well as extensions and libraries loaded with shared_preload_libraries(). This involved removing most occurrences of the word "module", although it is left in the section title. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/adminpack.sgml b/doc/src/sgml/adminpack.sgml index 184e96d7a0..04f3b52379 100644 --- a/doc/src/sgml/adminpack.sgml +++ b/doc/src/sgml/adminpack.sgml @@ -1,7 +1,7 @@ - adminpack + adminpack pgAdmin support toolpack adminpack diff --git a/doc/src/sgml/amcheck.sgml b/doc/src/sgml/amcheck.sgml index 5d61a33936..48d72a24a3 100644 --- a/doc/src/sgml/amcheck.sgml +++ b/doc/src/sgml/amcheck.sgml @@ -1,7 +1,7 @@ - amcheck + amcheck tools to verify index consistency amcheck diff --git a/doc/src/sgml/auth-delay.sgml b/doc/src/sgml/auth-delay.sgml index 3bc9cfb207..690774f86f 100644 --- a/doc/src/sgml/auth-delay.sgml +++ b/doc/src/sgml/auth-delay.sgml @@ -1,7 +1,7 @@ - auth_delay + auth_delay pause on authentication failure auth_delay diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index 394fec94e8..a80910dab5 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -1,7 +1,7 @@ - auto_explain + auto_explain log execution plans of slow queries auto_explain diff --git a/doc/src/sgml/basebackup-to-shell.sgml b/doc/src/sgml/basebackup-to-shell.sgml index b2ecc373eb..fd082ceb0b 100644 --- a/doc/src/sgml/basebackup-to-shell.sgml +++ b/doc/src/sgml/basebackup-to-shell.sgml @@ -1,7 +1,7 @@ - basebackup_to_shell + basebackup_to_shell example "shell" pg_basebackup module basebackup_to_shell diff --git a/doc/src/sgml/basic-archive.sgml b/doc/src/sgml/basic-archive.sgml index 0b650f17a8..c412590dd6 100644 --- a/doc/src/sgml/basic-archive.sgml +++ b/doc/src/sgml/basic-archive.sgml @@ -1,7 +1,7 @@ - basic_archive + basic_archive an example WAL archive module basic_archive diff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgml/bloom.sgml index a3f51cfdc4..4a188ad5f1 100644 --- a/doc/src/sgml/bloom.sgml +++ b/doc/src/sgml/bloom.sgml @@ -1,7 +1,7 @@ - bloom + bloom bloom filter index access bloom diff --git a/doc/src/sgml/btree-gin.sgml b/doc/src/sgml/btree-gin.sgml index 5bc5a054e8..5aafe856b5 100644 --- a/doc/src/sgml/btree-gin.sgml +++ b/doc/src/sgml/btree-gin.sgml @@ -1,7 +1,8 @@ - btree_gin + btree_gin + sample GIN B-tree equalivent operator classes [trusted] btree_gin diff --git a/doc/src/sgml/btree-gist.sgml b/doc/src/sgml/btree-gist.sgml index b67f20a00f..67ff13c77f 100644 --- a/doc/src/sgml/btree-gist.sgml +++ b/doc/src/sgml/btree-gist.sgml @@ -1,7 +1,8 @@ - btree_gist + btree_gist + B-tree equalivent GiST index operators [trusted] btree_gist diff --git a/doc/src/sgml/citext.sgml b/doc/src/sgml/citext.sgml index 5986601327..bf08e9e6a2 100644 --- a/doc/src/sgml/citext.sgml +++ b/doc/src/sgml/citext.sgml @@ -1,7 +1,8 @@ - citext + citext + a case-insensitive character string type [trusted] citext diff --git a/doc/src/sgml/contrib-spi.sgml b/doc/src/sgml/contrib-spi.sgml index fed6f24932..edeb5b6346 100644 --- a/doc/src/sgml/contrib-spi.sgml +++ b/doc/src/sgml/contrib-spi.sgml @@ -1,7 +1,7 @@ - spi + spi Server Programming Interface features/examples SPI diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml index 4e7b87a42f..fd2e40980d 100644 --- a/doc/src/sgml/contrib.sgml +++ b/doc/src/sgml/contrib.sgml @@ -1,27 +1,31 @@ - Additional Supplied Modules + Additional Supplied Modules and Extensions - This appendix and the next one contain information regarding the modules that - can be found in the contrib directory of the + This appendix and the next one contain information on the + optional components + found in the contrib directory of the PostgreSQL distribution. These include porting tools, analysis utilities, - and plug-in features that are not part of the core PostgreSQL system, - mainly because they address a limited audience or are too experimental + and plug-in features that are not part of the core PostgreSQL system. + They are separate mainly + because they address a limited audience
Re: Patch to document base64 encoding
On Sat, 18 Jan 2020 18:05:49 -0500 Tom Lane wrote: > And there were some errors; notably, the patch added descriptions > for shaNNN(text), which are functions we do not have AFAICS. Apologies for that, my mistake. Thank you to Fabien and everybody who helped. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
On Thu, 16 Jan 2020 15:44:44 -0300 Alvaro Herrera wrote: > Because of how the new tables look in the PDF docs, I thought it might > be a good time to research how to make each function-entry occupy two > rows: one for prototype, return type and description, and the other > for the example and its result. Another approach might be to fix/change the software that generates PDFs. Or whatever turns it into latex if that's the intermediate and really where the problem lies. (FWIW, I've had luck with dblatex.) (Maybe best to take this thread to the pgsql-docs mailing list?) Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
On Thu, 16 Jan 2020 14:41:33 +0100 (CET)
Fabien COELHO wrote:
> Some comments about v13:
>
> The note about get_byte reads:
>
>get_byte and set_byte number the first byte of a binary string as
> byte 0. get_bit and set_bit number bits from the right within each
> byte; for example bit 0 is the least significant bit of the first
> byte, and bit 15 is the most significant bit of the second byte.
>
> The two sentences starts with a lower case letter, which looks
> strange to me. I'd suggest to put "Functions" at the beginning of the
> sentences:
>
>Functions get_byte and set_byte number the first byte of a binary
> string as byte 0. Functions get_bit and set_bit number bits from the
> right within each byte; for example bit 0 is the least significant
> bit of the first byte, and bit 15 is the most significant bit of the
> second byte.
Excellent suggestion, done.
> The note about hash provides an example for getting the hex
> representation out of sha*. I'd add an exemple to get the bytea
> representation from md5, eg "DECODE(MD5('hello world'), 'hex')"…
Ok. Done.
> Maybe the encode/decode in the note could be linked to the function
> description? Well, they are just after, maybe it is not very useful.
Can't hurt? Done.
Patch attached: doc_base64_v14.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 72072e7545..c075872364 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1459,6 +1459,13 @@
natively for the bit-string types.
+
+ Functions which convert, both to and from, strings and
+ the bytea type
+ are documented
+ separately.
+
+
SQL defines some string functions that use
key words, rather than commas, to separate
@@ -1820,101 +1827,6 @@
abcde,2,22
-
-
-
- convert
-
-convert(string bytea,
-src_encoding name,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding. The
-original encoding is specified by
-src_encoding. The
-string must be valid in this encoding.
-Conversions can be defined by CREATE CONVERSION.
-Also there are some predefined conversions. See for available conversions.
-
- convert('text_in_utf8', 'UTF8', 'LATIN1')
- text_in_utf8 represented in Latin-1
- encoding (ISO 8859-1)
-
-
-
-
-
- convert_from
-
-convert_from(string bytea,
-src_encoding name)
-
- text
-
-Convert string to the database encoding. The original encoding
-is specified by src_encoding. The
-string must be valid in this encoding.
-
- convert_from('text_in_utf8', 'UTF8')
- text_in_utf8 represented in the current database encoding
-
-
-
-
-
- convert_to
-
-convert_to(string text,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding.
-
- convert_to('some text', 'UTF8')
- some text represented in the UTF8 encoding
-
-
-
-
-
- decode
-
-decode(string text,
-format text)
-
- bytea
-
-Decode binary data from textual representation in string.
-Options for format are same as in encode.
-
- decode('MTIzAAE=', 'base64')
- \x3132330001
-
-
-
-
-
- encode
-
-encode(data bytea,
-format text)
-
- text
-
-Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
-escape converts zero bytes and high-bit-set bytes to
-octal sequences (\nnn) and
-doubles backslashes.
-
- encode('123\000\001', 'base64')
- MTIzAAE=
-
-
@@ -1982,19 +1894,6 @@
4
-
- length(string bytea,
-encoding name )
- int
-
-Number of characters in string in the given
-encoding. The string
-must be valid in this encoding.
-
- length('jose', 'UTF8')
- 4
-
-
@@ -2044,8 +1943,8 @@
text
-Calculates the MD5 hash of string,
-returning the result in hexadecimal
+Calculates the MD5 hash
+of string, returning the result in hexadecimal
md5('abc')
900150983cd24fb0 d6963f7d28e17f72
@@ -2358,6 +2257,66 @@
test
+
+
+
+ sha224
+
+sha224(string)
+
+ bytea
+
+SHA-224 hash
+
+ sha224('abc')
+
Re: Patch to document base64 encoding
On Thu, 16 Jan 2020 14:41:33 +0100 (CET) Fabien COELHO wrote: > The "Binary String Functions and Operators" 9.5 section has only one > subsection, "9.5.1", which is about at two thirds of the page. This > structure looks weird. ISTM that a subsection is missing for the > beginning of the page, or that the subsection should just be dropped, > because it is somehow redundant with the table title. > > The "9.4" section has the same structural weirdness. Either remove > the subsection, or add some for the other parts? Hi Fabien, cc-ing the folks who did the work on format(), who added a sub-section 9.4.1. The whole thread for that is here: https://www.postgresql.org/message-id/flat/CAFj8pRBjMdAjybSZkczyez0x%3DFhC8WXvgR2wOYGuhrk1TUkraA%40mail.gmail.com I'm going to dis-agree with you on this. Yes, it's a little odd to have only a single sub-section but it does not really bother me. If there's a big/important enough chunk of information to present I like seeing something in the table of contents. That's the "big thing" to my mind. I don't see a good way to get rid of "9.4.1. format". Adding another sub-section heading above it just to have 2 seems pointless. I really want the "9.5.1 String to Binary and Binary to String Conversion" to show up in the table of contents. Because it is not at all obvious that "9.5. Binary String Functions and Operators" is the place to look for conversions between string and binary. Tom thought that merely having a separate table for string<->binary functions "could be overkill" so my impression right now is that having an entirely separate section for these would be rejected. (See: https://www.postgresql.org/message-id/22540.1564501...@sss.pgh.pa.us) Otherwise an entirely separate section might be the right approach. The following *.1 sections in the (devel version) documentation are "single sub-sections": (Er, this is too much but once I started I figured I'd finish.) 5.10. Inheritance 5.10.1. Caveats 9.4. String Functions and Operators 9.4.1. format 9.30. Statistics Information Functions 9.30.1. Inspecting MCV Lists 15.4. Parallel Safety 15.4.1. Parallel Labeling for Functions and Aggregates 17. Installation from Source Code on Windows 17.1. Building with Visual C++ or the Microsoft Windows SDK 18.10. Secure TCP/IP Connections with GSSAPI Encryption 18.10.1. Basic Setup 30.2. Subscription 30.2.1. Replication Slot Management 30.5. Architecture 30.5.1. Initial Snapshot 37.13. User-Defined Types 37.13.1. TOAST Considerations 41. Procedural Languages 41.1. Installing Procedural Languages 50.5. Planner/Optimizer 50.5.1. Generating Possible Plans 52.3. SASL Authentication 52.3.1. SCRAM-SHA-256 Authentication 57. Writing a Table Sampling Method 57.1. Sampling Method Support Functions 58.1. Creating Custom Scan Paths 58.1.1. Custom Scan Path Callbacks 58.2. Creating Custom Scan Plans 58.2.1. Custom Scan Plan Callbacks 58.3. Executing Custom Scans 58.3.1. Custom Scan Execution Callbacks 64.4. Implementation 64.4.1. GiST Buffering Build 67.1. Introduction 67.1.1. Index Maintenance 68.6. Database Page Layout 68.6.1. Table Row Layout G.2. Server Applications pg_standby — supports the creation of a PostgreSQL warm standby server I. The Source Code Repository I.1. Getting the Source via Git J.4. Documentation Authoring J.4.1. Emacs J.5. Style Guide J.5.1. Reference Pages I like that I can see these in the documentation. FYI, the format sub-section, 9.4.1, was first mentioned by Dean Rasheed in this email: https://www.postgresql.org/message-id/CAEZATCWLtRi-Vbh5k_2fYkOAPxas0wZh6a0brOohHtVOtHiddA%40mail.gmail.com "I'm thinking perhaps format() should now have its own separate sub-section in the manual, rather than trying to cram it's docs into a single table row." There was never really any further discussion or objection to having a separate sub-section. Attaching a new patch to my next email, leaving off the folks cc-ed regarding "9.4.1 format". Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
On Thu, 9 Jan 2020 08:27:28 +0100 (CET)
Fabien COELHO wrote:
> > Another option would be to use "bytes bytea".
>
> > (The current patch uses "string bytea".)
> > This would probably also require some re-wording throughout.
> I like it, but this is only my own limited opinion, and I'm not a
> native English speaker.
Per your request for consistency I made this change throughout
the entire binary string section.
New patch attached: doc_base64_v13.patch
This required surprisingly little re-wording.
Added word "binary" into the descriptions of convert(),
substring(), convert_from(), and convert_to().
I also added data types to the call syntax of set_bit()
and set_byte().
And this patch adds hyperlinks from the get_bit(), get_byte(),
set_bit(), and set_byte() descriptions to the note
that offsets are zero-based.
I also removed the hyperlinked asterisks about the hash
function results and instead hyperlinked the word "hash"
in the descriptions. (Links to the note about md5()
returning hex text and the others returning bytea and how
to convert between the two.)
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4b42f12862..3bd7232814 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1459,6 +1459,13 @@
natively for the bit-string types.
+
+ Functions which convert, both to and from, strings and
+ the bytea type
+ are documented
+ separately.
+
+
SQL defines some string functions that use
key words, rather than commas, to separate
@@ -1820,101 +1827,6 @@
abcde,2,22
-
-
-
- convert
-
-convert(string bytea,
-src_encoding name,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding. The
-original encoding is specified by
-src_encoding. The
-string must be valid in this encoding.
-Conversions can be defined by CREATE CONVERSION.
-Also there are some predefined conversions. See for available conversions.
-
- convert('text_in_utf8', 'UTF8', 'LATIN1')
- text_in_utf8 represented in Latin-1
- encoding (ISO 8859-1)
-
-
-
-
-
- convert_from
-
-convert_from(string bytea,
-src_encoding name)
-
- text
-
-Convert string to the database encoding. The original encoding
-is specified by src_encoding. The
-string must be valid in this encoding.
-
- convert_from('text_in_utf8', 'UTF8')
- text_in_utf8 represented in the current database encoding
-
-
-
-
-
- convert_to
-
-convert_to(string text,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding.
-
- convert_to('some text', 'UTF8')
- some text represented in the UTF8 encoding
-
-
-
-
-
- decode
-
-decode(string text,
-format text)
-
- bytea
-
-Decode binary data from textual representation in string.
-Options for format are same as in encode.
-
- decode('MTIzAAE=', 'base64')
- \x3132330001
-
-
-
-
-
- encode
-
-encode(data bytea,
-format text)
-
- text
-
-Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
-escape converts zero bytes and high-bit-set bytes to
-octal sequences (\nnn) and
-doubles backslashes.
-
- encode('123\000\001', 'base64')
- MTIzAAE=
-
-
@@ -1982,19 +1894,6 @@
4
-
- length(string bytea,
-encoding name )
- int
-
-Number of characters in string in the given
-encoding. The string
-must be valid in this encoding.
-
- length('jose', 'UTF8')
- 4
-
-
@@ -2044,8 +1943,8 @@
text
-Calculates the MD5 hash of string,
-returning the result in hexadecimal
+Calculates the MD5 hash
+of string, returning the result in hexadecimal
md5('abc')
900150983cd24fb0 d6963f7d28e17f72
@@ -2358,6 +2257,66 @@
test
+
+
+
+ sha224
+
+sha224(string)
+
+ bytea
+
+SHA-224 hash
+
+ sha224('abc')
+ \x23097d223405d8228642a477bda255b32aadbce4bda0b3f7e36c9da7
+
+
+
+
+
+ sha256
+
+sha256(string)
+
+ bytea
+
+SHA-256 hash
+
+ sha256('abc')
+
Re: Patch to document base64 encoding
On Mon, 6 Jan 2020 01:35:00 -0600 "Karl O. Pinc" wrote: > On Sun, 5 Jan 2020 12:48:59 +0100 (CET) > Fabien COELHO wrote: > > I'm not keen on calling the parameter the name of its type. I'd > > suggest to keep "string" as a name everywhere, which is not a type > > name in Pg. > > > > The functions descriptions are not homogeneous. Some have parameter > > name & type "btrim(string bytea, bytes bytea)" and others only type > > or parameter with tagged as a parameter "get_bit(bytea, > > offset)" (first param), "sha224(bytea)". > > > > I'd suggest to be consistent, eg use "string bytea" everywhere > > appropriate. > > Ok. Done. > If you're interested, another possibility would be the > consistent use of "data bytea" everywhere. > But then the word > "string" does not really fit in a lot of the descriptions. > So this choice would involve re-writing descriptions ... > The trouble with using "data bytea" is that there might > need to be adjustments to the word "string" elsewhere in > the section, not just in the descriptions. > > Let me know if you'd prefer "data bytea" to "string bytea" > and consequent frobbing of descriptions. That might be > out-of-scope for this patch. (Which is already > a poster-child for feature-creep.) Another option would be to use "bytes bytea". (The current patch uses "string bytea".) This would probably also require some re-wording throughout. Please let me know your preference. Thanks. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
Hello Fabien,
On Sun, 5 Jan 2020 12:48:59 +0100 (CET)
Fabien COELHO wrote:
> I'm in favor of moving and reorganizing these function descriptions,
> as they are somehow scattered with a unclear logic when you are
> looking for them.
I assume by this you mean you are happy with the organization done
by the patch.
For review (I think I've got this right) the organizational
changes are:
The changes suggested by Tom where 2
functions with the same name, one of which takes string
arguments and the other of which takes bytea arguments
now show up both in the doc section on string functions and
in the doc section on bytea functions.
I believe I also alphabetized the binary function ordering.
And this patch introduces a separate table/sub-section for
functions which convert between binary and string.)
There are much-expanded descriptions of encode()
and decode(). (Which is how this patch series
started, explaining base64 encoding/decoding.)
FYI. There is also an unusual usage of a hyperlinked
asterisk following the returned datatype of the hash
functions. The hyperlink leads to the historical
note on the datatype used for the md5() function v.s.
the other hash functions.
> + bytea
> ||
> +bytea
>bytea
>
>String concatenation
>
> Bytea concatenation?
Done. (Could just say "Concatenation" I suppose. But "Bytea
concatenation" does not hurt and would be nice if you
ever looked at the tables for string operators and bytea
operators side-by-side.)
> I'm not keen on calling the parameter the name of its type. I'd
> suggest to keep "string" as a name everywhere, which is not a type
> name in Pg.
>
> The functions descriptions are not homogeneous. Some have parameter
> name & type "btrim(string bytea, bytes bytea)" and others only type
> or parameter with tagged as a parameter "get_bit(bytea,
> offset)" (first param), "sha224(bytea)".
>
> I'd suggest to be consistent, eg use "string bytea" everywhere
> appropriate.
Ok. Done. Except that I've left the encode() function
as encode(data bytea, format text) because the whole
point is to convert _to_ a string/text datatype
from something that's _not_ a string. Calling
the input a string just seems wrong. This inconsistency seems ok
because encode() is in the table of string <-> bytea functions,
away from the other bytea functions.
If you're interested, another possibility would be the
consistent use of "data bytea" everywhere. I like this
choice because it works well to write
encode(data bytea,
formatstring bytea,
bytes bytea)
Remove the longest string containing only bytes appearing in
bytes from the start and end of
string
Would change to (say):
btrim(data bytea,
bytes bytea)
Remove the longest contiguous sequence of bytes containing only
those bytes appearing in bytes
from the start and end of data
The trouble with using "data bytea" is that there might
need to be adjustments to the word "string" elsewhere in
the section, not just in the descriptions.
Let me know if you'd prefer "data bytea" to "string bytea"
and consequent frobbing of descriptions. That might be
out-of-scope for this patch. (Which is already
a poster-child for feature-creep.)
Attached is doc_base64_v12.patch.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 57a1539506..2e45bf71c8 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1431,6 +1431,13 @@
natively for the bit-string types.
+
+ Functions which convert, both to and from, strings and
+ the bytea type
+ are documented
+ separately.
+
+
SQL defines some string functions that use
key words, rather than commas, to separate
@@ -1792,101 +1799,6 @@
abcde,2,22
-
-
-
- convert
-
-convert(string bytea,
-src_encoding name,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding. The
-original encoding is specified by
-src_encoding. The
-string must be valid in this encoding.
-Conversions can be defined by CREATE CONVERSION.
-Also there are some predefined conversions. See for available conversions.
-
- convert('text_in_utf8', 'UTF8', 'LATIN1')
- text_in_utf8 represented in Latin-1
- encoding (ISO 8859-1)
-
-
-
-
-
- convert_from
-
-convert_from(string bytea,
-src_encoding name)
-
- text
-
-Convert string to the database encoding. The original encoding
-is specified by src_encoding. The
-string must be valid in this encoding.
-
- convert_from('text_in_utf8', 'UTF8')
- text_in_utf8 represented in the current database encoding
-
Re: proposal: minscale, rtrim, btrim functions for numeric
On Tue, 10 Dec 2019 07:11:59 +0100 Pavel Stehule wrote: > út 10. 12. 2019 v 0:03 odesílatel Karl O. Pinc napsal: > > I also wonder whether all the trim_scale() tests > > are now necessary, but not enough to make any suggestions. > I don't think so tests should be minimalistic - there can be some > redundancy to coverage some less probable size effects of some future > changes. More - there is a small symmetry with min_scale tests - and > third argument - some times I use tests (result part) as > "documentation". Fine with me. Tests pass against HEAD. Docs build and look good. Patch looks good to me. I'm marking it ready for a committer. Thanks for the work. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: proposal: minscale, rtrim, btrim functions for numeric
On Mon, 9 Dec 2019 21:04:21 +0100 Pavel Stehule wrote: > I fixed almost all mentioned issues (that I understand) If you don't understand you might ask, or at least say. That way I know you've noticed my remarks and I don't have to repeat them. I have 2 remaining suggestions. 1) As previously suggested: Consider moving all the code you added to numeric.c to right after the scale() related code. This is equivalent to what was done in pg_proc.dat and regression tests where all the scale related stuff is in one place in the file. 2) Now that the function is called min_scale() it might be nice if your "minscale" variable in numeric.c was named "min_scale". I don't feel particularly strongly about either of the above but think them a slight improvement. I also wonder whether all the trim_scale() tests are now necessary, but not enough to make any suggestions. Especially because, well, tests are good. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: proposal: minscale, rtrim, btrim functions for numeric
On Mon, 9 Dec 2019 12:15:22 -0600 "Karl O. Pinc" wrote: > I've had some thoughts about the regression tests. > Having written > it out it seems like a lot of testing for such a simple function. FYI. I don't see trim_scale() needing such exhaustive testing because you'll have already tested a lot with the min_scale() tests. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: proposal: minscale, rtrim, btrim functions for numeric
Hi Pavel, I've had some thoughts about the regression tests. It wouldn't hurt to move them to right after the scale() tests in numeric.sql. I believe your tests are covering all the code paths but it is not clear just what test does what. I don't see a lot of comments in the tests so I don't know that it'd be appropriate to put them in to describe just what's tested. But in any case it could be nice to choose values where it is at least sort of apparent what part of the codebase is tested. FWIW, although the code paths are covered, the possible data permutations are not. E.g. I don't see a case where scale > 0 and the NDIGITS of the last digit is full. There are also some tests (the 0 and 0.00 tests) that duplicates the execution path. In the 0 case I don't see a problem but as a rule there's not a lot of point. Better test values would (mostly) eliminate these. So, my thoughts run along these lines: select minscale(numeric 'NaN') is NULL; -- should be true select minscale(NULL::numeric) is NULL; -- should be true select minscale(0); -- no digits select minscale(0.00); -- no digits again select minscale(1.0); -- no scale select minscale(1.1); -- scale 1 select minscale(1.12); -- scale 2 select minscale(1.123); -- scale 3 select minscale(1.1234);-- scale 4, filled digit select minscale(1.12345); -- scale 5, 2 NDIGITS select minscale(1.1000);-- 1 pos in NDIGITS select minscale(1.1200);-- 2 pos in NDIGITS select minscale(1.1230);-- 3 pos in NDIGITS select minscale(1.1234);-- all pos in NDIGITS select minscale(1.12345000);-- 2 NDIGITS select minscale(1.1234);-- strip() required/done select minscale(12345.123456789012345); -- "big" number select minscale(-12345.12345); -- negative number select minscale(1e100); -- very big number select minscale(1e100::numeric + 0.1); -- big number with scale I don't know why you chose some of your values so if there's something you were testing for that the above does not cover please include it. So, a combination of white and black box testing. Having written it out it seems like a lot of testing for such a simple function. On the other hand I don't see a lot of cost in having all these tests. Opinions welcome. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: proposal: minscale, rtrim, btrim functions for numeric
Hi Pavel,
Thanks for your changes. More inline below:
On Sun, 8 Dec 2019 08:38:38 +0100
Pavel Stehule wrote:
> ne 8. 12. 2019 v 2:23 odesílatel Karl O. Pinc napsal:
> > On Mon, 11 Nov 2019 15:47:37 +0100
> > Pavel Stehule wrote:
> > > I implemented two functions - first minscale, second trim_scale.
> > > The overhead of second is minimal - so I think it can be good to
> > > have it. I started design with the name "trim_scale", but the
> > > name can be any other.
> > I comment on various hunks in line below:
>
> > diff --git a/src/backend/utils/adt/numeric.c
> > b/src/backend/utils/adt/numeric.c index a00db3ce7a..35234aee4c
> > 100644 --- a/src/backend/utils/adt/numeric.c
> > +++ b/src/backend/utils/adt/numeric.c
> >
> >
> > I believe the hunks in this file should start at about line# 3181.
> > This is right after numeric_scale(). Seems like all the scale
> > related functions should be together.
> >
> > There's no hard standard but I don't see why lines (comment lines in
> > your case) should be longer than 78 characters without good reason.
> > Please reformat.
> >
I don't see any response from you regarding the above two suggestions.
>
> > + */
> > +static int
> > +get_min_scale(NumericVar *var)
> > +{
> > + int minscale = 0;
> > +
> > + if (var->ndigits > 0)
> > + {
> > + NumericDigit last_digit;
> > +
> > + /* maximal size of minscale, can be lower */
> > + minscale = (var->ndigits - var->weight - 1) *
> > DEC_DIGITS; +
> > + /*
> > +* When there are not digits after decimal point,
> > the previous expression
> >
> >
> > s/not/no/
> >
> >
> > +* can be negative. In this case, the minscale must
> > be zero.
> > +*/
> >
> >
> > s/can be/is/
> >
By the above, I intended the comment be changed (after line wrapping)
to:
/*
* When there are no digits after decimal point,
* the previous expression is negative. In this
* case the minscale must be zero.
*/
(Oh yes, on re-reading I think the comma is unnecessary so I removed it too.)
> >
> > + if (minscale > 0)
> > + {
> > + /* reduce minscale if trailing digits in
> > last numeric digits are zero */
And the above comment should either be wrapped (as requested above)
or eliminated. I like comments but I'm not sure this one contributes
anything.
> > --- a/src/include/catalog/pg_proc.dat
> > +++ b/src/include/catalog/pg_proc.dat
> > @@ -4288,6 +4288,12 @@
> >proname => 'width_bucket', prorettype => 'int4',
> >proargtypes => 'numeric numeric numeric int4',
> >prosrc => 'width_bucket_numeric' },
> > +{ oid => '3434', descr => 'returns minimal scale of numeric value',
> >
> >
> > How about a descr of?:
> >
> > minimal scale needed to store the supplied value without data loss
> >
> >
>
> done
>
> >
> > + proname => 'minscale', prorettype => 'int4', proargtypes =>
> > 'numeric',
> > + prosrc => 'numeric_minscale' },
> > +{ oid => '3435', descr => 'returns numeric value with minimal
> > scale',
> >
> >
> > And likewise a descr of?:
> >
> > numeric with minimal scale needed to represent the given value
> >
> >
> > + proname => 'trim_scale', prorettype => 'numeric', proargtypes =>
> > 'numeric',
> > + prosrc => 'numeric_trim_scale' },
> >
>
> done
Thanks for these changes. Looking at pg_proc.dat there seems to
be an effort made to keep the lines to a maximum of 78 or 80
characters. This means starting "descr => '..." on new lines
when the description is long. Please reformat, doing this or,
if you like, something even more clever to keep the lines short.
Looking good. We're making progress.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Re: proposal: minscale, rtrim, btrim functions for numeric
On Sun, 08 Dec 2019 13:57:00 -0500 Tom Lane wrote: > "Karl O. Pinc" writes: > > FWIW, I bumped around the Internet and looked at Oracle docs to see > > if there's any reason why minscale() might not be a good function > > name. I couldn't find any problems. > > > I also couldn't think of a better name than trim_scale() and don't > > have any problems with the name. > > I'd just comment that it seems weird that the same patch is > introducing two functions with inconsistently chosen names. Why does > one have an underscore separating the words and the other not? I > haven't got a large investment in either naming convention > specifically, but it'd be nice if we could at least pretend to be > considering consistency. Consistency would be lovely. I don't feel qualified to make the decision but here's what I came up with: I re-read the back-threads and don't see any discussion of the naming of minscale(). My thoughts run toward asking the "what is a word?" question, along with "what is the policy for separating a word?". Is "min" different from the prefix "sub"? "Trim" seems to clearly count as a word and trim_scale() seems mostly consistent with existing function names. (E.g. width_bucket(), convert_to(). But there's no true consistency. Plenty of functions don't separate words with "_". E.g. setseed().) As far as "min" goes, glancing through function names [1] does not help much. The index indicates that when PG puts "min" in a configuration parameter it separates it with "_". (Looking at "min" in the index.) Looking at the function names containing "min" [2] yields: aclitemin brin_minmax_add_value brin_minmax_consistent brin_minmax_opcinfo brin_minmax_union min numeric_uminus pg_terminate_backend range_minus txid_snapshot_xmin Not especially helpful. I'm inclined to want min_scale() instead of "minscale()" based on how config parameters are named and for consistency with trim_scale(). Pavel, if you agree then let's just change minscale() to min_scale() and let people object if they don't like it. Regards. Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein [1] select pg_proc.proname from pg_proc group by pg_proc.proname order by pg_proc.proname; [2] select pg_proc.proname from pg_proc where pg_proc.proname like '%min%' group by pg_proc.proname order by pg_proc.proname;
Re: proposal: minscale, rtrim, btrim functions for numeric
Hello Pavel,
On Mon, 11 Nov 2019 15:47:37 +0100
Pavel Stehule wrote:
> Here is a patch. It's based on Dean's suggestions.
>
> I implemented two functions - first minscale, second trim_scale. The
> overhead of second is minimal - so I think it can be good to have it.
> I started design with the name "trim_scale", but the name can be any
> other.
Here are my thoughts on your patch.
My one substantial criticism is that I believe that
trim_scale('NaN'::numeric) should return NaN.
So the test output should look like:
template1=# select trim_scale('nan'::numeric) = 'nan'::numeric;
?column?
--
t
(1 row)
FWIW, I bumped around the Internet and looked at Oracle docs to see if
there's any reason why minscale() might not be a good function name.
I couldn't find any problems.
I also couldn't think of a better name than trim_scale() and don't
have any problems with the name.
My other suggestions mostly have to do with documentation. Your code
looks pretty good to me, looks like the existing code, you name
variables and function names as in existing code, etc.
I comment on various hunks in line below:
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 28eb322f3f..6f142cd679 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -918,6 +918,19 @@
6.00
+
+
+
+ minscale
+
+
minscale(numeric)
+
+ integer
+ returns minimal scale of the argument (the number of
decimal digits in the fractional part)
+ scale(8.4100)
+ 2
+
+
*
Your description does not say what the minimal scale is. How about:
minimal scale (number of decimal digits in the fractional part) needed
to store the supplied value without data loss
*
@@ -1041,6 +1054,19 @@
1.4142135623731
+
+
+
+ trim_scale
+
+
trim_scale(numeric)
+
+ numeric
+ reduce scale of the argument (the number of decimal
digits in the fractional part)
+ scale(8.4100)
+ 8.41
+
+
How about:
reduce the scale (the number of decimal digits in the fractional part)
to the minimum needed to store the supplied value without data loss
*
diff --git a/src/backend/utils/adt/numeric.c
b/src/backend/utils/adt/numeric.c index a00db3ce7a..35234aee4c 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
I believe the hunks in this file should start at about line# 3181.
This is right after numeric_scale(). Seems like all the scale
related functions should be together.
There's no hard standard but I don't see why lines (comment lines in
your case) should be longer than 78 characters without good reason.
Please reformat.
@@ -5620,6 +5620,88 @@ int2int4_sum(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(Int64GetDatumFast(transdata->sum));
}
+/*
+ * Calculate minimal display scale. The var should be stripped already.
I think you can get rid of the word "display" in the comment.
+ */
+static int
+get_min_scale(NumericVar *var)
+{
+ int minscale = 0;
+
+ if (var->ndigits > 0)
+ {
+ NumericDigit last_digit;
+
+ /* maximal size of minscale, can be lower */
+ minscale = (var->ndigits - var->weight - 1) *
DEC_DIGITS; +
+ /*
+* When there are not digits after decimal point, the
previous expression
s/not/no/
+* can be negative. In this case, the minscale must be
zero.
+*/
s/can be/is/
+ if (minscale > 0)
+ {
+ /* reduce minscale if trailing digits in last
numeric digits are zero */
+ last_digit = var->digits[var->ndigits - 1];
+
+ while (last_digit % 10 == 0)
+ {
+ minscale--;
+ last_digit /= 10;
+ }
+ }
+ else
+ minscale = 0;
+ }
+
+ return minscale;
+}
+
+/*
+ * Returns minimal scale of numeric value when value is not changed
Improve comment, something like:
minimal scale required to represent supplied value without loss
+ */
+Datum
+numeric_minscale(PG_FUNCTION_ARGS)
+{
+ Numeric num = PG_GETARG_NUMERIC(0);
+ NumericVar arg;
+ int minscale;
+
+ if (NUMERIC_IS_NAN(num))
+ PG_RETURN_NULL();
+
+ init_var_from_num(num, );
+ strip_var();
+
+ minscale = get_min_scale();
+ free_var();
+
+ PG_RETURN_INT32(minscale);
+}
+
+/*
+ * Reduce scale of numeric value so value is not changed
Likewise, comment text could be improved
+ */
+Datum
+numeric_trim_scale(PG_FUNCTION_ARGS)
+{
+ Numeric num =
Re: Patch to document base64 encoding
Hi,
Attached is doc_base64_v11.patch
This addresses Tom's concerns. Functions
that operate on both strings and bytea
(e.g. length(text) and length(bytea))
are documented separately, one with
string functions and one with binary
string functions.
In this iteration I have also:
Added a sub-section for the functions
which convert between text and bytea.
Added some index entries.
Provided a link in the hash functions to
the text about why md5() returns text
not bytea.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 57a1539506..aac9065e16 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1432,6 +1432,13 @@
+ Functions which convert, both to and from, strings and
+ the bytea type
+ are documented
+ separately.
+
+
+
SQL defines some string functions that use
key words, rather than commas, to separate
arguments. Details are in
@@ -1793,101 +1800,6 @@
-
-
- convert
-
-convert(string bytea,
-src_encoding name,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding. The
-original encoding is specified by
-src_encoding. The
-string must be valid in this encoding.
-Conversions can be defined by CREATE CONVERSION.
-Also there are some predefined conversions. See for available conversions.
-
- convert('text_in_utf8', 'UTF8', 'LATIN1')
- text_in_utf8 represented in Latin-1
- encoding (ISO 8859-1)
-
-
-
-
-
- convert_from
-
-convert_from(string bytea,
-src_encoding name)
-
- text
-
-Convert string to the database encoding. The original encoding
-is specified by src_encoding. The
-string must be valid in this encoding.
-
- convert_from('text_in_utf8', 'UTF8')
- text_in_utf8 represented in the current database encoding
-
-
-
-
-
- convert_to
-
-convert_to(string text,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding.
-
- convert_to('some text', 'UTF8')
- some text represented in the UTF8 encoding
-
-
-
-
-
- decode
-
-decode(string text,
-format text)
-
- bytea
-
-Decode binary data from textual representation in string.
-Options for format are same as in encode.
-
- decode('MTIzAAE=', 'base64')
- \x3132330001
-
-
-
-
-
- encode
-
-encode(data bytea,
-format text)
-
- text
-
-Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
-escape converts zero bytes and high-bit-set bytes to
-octal sequences (\nnn) and
-doubles backslashes.
-
- encode('123\000\001', 'base64')
- MTIzAAE=
-
-
-
format
@@ -1955,19 +1867,6 @@
- length(string bytea,
-encoding name )
- int
-
-Number of characters in string in the given
-encoding. The string
-must be valid in this encoding.
-
- length('jose', 'UTF8')
- 4
-
-
-
lpad
@@ -2014,7 +1913,7 @@
md5(string)
- text
+ text*
Calculates the MD5 hash of string,
returning the result in hexadecimal
@@ -2333,6 +2232,66 @@
+ sha224
+
+sha224(string)
+
+ bytea*
+
+SHA-224 hash
+
+ sha224('abc')
+ \x23097d223405d8228642a477bda255b32aadbce4bda0b3f7e36c9da7
+
+
+
+
+
+ sha256
+
+sha256(string)
+
+ bytea*
+
+SHA-256 hash
+
+ sha256('abc')
+ \xba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad
+
+
+
+
+
+ sha384
+
+sha384(string)
+
+ bytea*
+
+SHA-384 hash
+
+ sha384('abc')
+ \xcb00753f45a35e8bb5a03d699ac65007272c32ab0eded1631a8b605a43ff5bed8086072ba1e7cc2358baeca134c825a7
+
+
+
+
+
+ sha512
+
+sha512(string)
+
+ bytea*
+
+SHA-512 hash
+
+ sha512('abc')
+ \xddaf35a193617abacc417349ae20413112e6fa4e89a97ea20a964b55d39a2192992a274fc1a836ba3c23a3feebbd454d4423643ce80e2a9ac94fa54ca49f
+
+
+
+
+
split_part
Re: Patch to document base64 encoding
On Fri, 02 Aug 2019 10:44:43 -0400 Tom Lane wrote: > I don't really have a problem with > repeating the entries for other functions that exist in both > text and bytea variants, either. Ok. Thanks. I'll repeat entries then. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
On Tue, 30 Jul 2019 23:44:49 +0200 (CEST) Fabien COELHO wrote: > Personnaly, I'd be ok with having a separate "conversion function" > table, and also with Tom suggestion to have string functions with > "only simple string" functions, and if any binary appears it is moved > into the binary section. I'll make a "conversion function" table and put it in the binary section. But I'm not happy with putting any function that works with bytea into the binary string section. This would mean moving, say, length() out of the regular string section. There's a lot of functions that work on both string and bytea inputs and most (not all, see below) are functions that people typically associate with string data. What I think I'd like to do is add a column to the table in the string section that says whether or not the function works with both string and bytea. The result would be: The hash functions (md5(), sha256(), etc.) would move to the string section, because they work on both strings and binary data. So the binary function table would considerably shorten. There would be a new table for conversions between bytea and string (both directions). This would be placed in the binary string section. So the binary string section would be "just simple bytea", plus conversion functions. Kind of the opposite of Tom's suggestion. Please let me know what you think. Thanks. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
On Tue, 30 Jul 2019 11:40:03 -0400 Tom Lane wrote: > "Karl O. Pinc" writes: > > On Mon, 15 Jul 2019 23:00:55 +0200 (CEST) > > Fabien COELHO wrote: > >> The patch clarifies the documentation about encode/decode and > >> other text/binary string conversion functions. > > > Other notable changes: > > Corrects categorization of functions as string or binary. > > Reorders functions alphabetically by function name. > > So I took a look at this, expecting that after so much discussion it > ought to just be committable ... It started simple, just changing the base64 function descriptions, but critique drew in additional issues. > but I am befuddled by your choices > about which functions to move where. The grouping is by the data type on which each function operates, the data type of the input. If there's to be 2 categories, as in the existing docs, it seems to me that you have to categorize either by the data type input or data type output. To categorize by input and output together would result 4 (or more?) categories, which would be even crazier. > It seems entirely crazy that > encode() and decode() are no longer in the same section, likewise that > convert_from() and convert_to() aren't documented together anymore. Awkward, yes. But findable if you know what the categories are. I suppose there could be 3 different categories: those that input and output strings, those that input and output binary, and those that convert -- inputting one data type and outputting another. I'm not sure that this would really address the issue of documenting, say encode() and decode() together. It pretty much makes sense to alphabetize the functions _within_ each category, because that's about the only easily defined way to do it. Going "by feel" and putting encode() and decode() together raises the question of where they should be together in the overall ordering within the category. > I'm not sure what is the right dividing line between string and binary > functions, but I don't think that anyone is going to find this > division helpful. Maybe there's a way to make more clear what the categories are? I could be explicit in the description of the section. > I do agree that documenting some functions twice is a bad plan, > so we need to clean this up somehow. > > After some thought, it seems like maybe a workable approach would be > to consider that all conversion functions going between text and > bytea belong in the binary-string-functions section. I think it's > reasonable to say that plain "string functions" just means stuff > dealing with text. Ok. Should the section title remain unchanged? "Binary String Functions and Operators" I think the summary description of the section will need a little clarification. > Possibly we could make a separate table in the binary-functions > section just for conversions, although that feels like it might be > overkill. I have no good answers. An advantage to a separate section for conversions is that you _might_ be able to pair the functions, so that encode() and decode() do show up right next to each other. I'm not sure exactly how to structure "pairing". I would have to play around and see what might look good. > While we're on the subject, Table 9.11 (conversion names) seems > entirely misplaced, and I don't just mean that it would need to > migrate to the binary-functions page. I don't think it belongs > in func.sgml at all. Isn't it pretty duplicative of Table 23.2 > (Client/Server Character Set Conversions)? I think we should > unify it with that table, or at least put it next to that one. > Perhaps that's material for a separate patch though. I don't know. But it does seem something that can be addressed in isolation and suitable for it's own patch. Thanks for the help. I will wait for a response to this before submitting another patch, just in case someone sees any ideas here to be followed up on. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
On Mon, 15 Jul 2019 23:00:55 +0200 (CEST) Fabien COELHO wrote: > The patch clarifies the documentation about encode/decode and other > text/binary string conversion functions. Other notable changes: Corrects categorization of functions as string or binary. Reorders functions alphabetically by function name. Thanks very much Fabien. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
Hi Fabien,
Attached is doc_base64_v10.patch
On Fri, 12 Jul 2019 15:58:21 +0200 (CEST)
Fabien COELHO wrote:
> >> I suggested "Function <>decode ...", which is the kind of thing
> >> we do in academic writing to improve precision, because I thought
> >> it could be better:-)
> >
> > "Function <>decode ..." just does not work in English.
>
> It really works in research papers: "Theorem X can be proven by
> applying Proposition Y. See Figure 2 for details. Algorithm Z
> describes whatever, which is listed in Table W..."
I've not thought about it before but I suppose the difference
is between declarative and descriptive, the latter being
more inviting and better allows for flow between sentences.
Otherwise you're writing in bullet points. So it is a
question of balance between specification and narration.
In regular prose you're always going to see the "the"
unless the sentence starts with the name. The trouble
is that we can't start sentences with function names
because of capitalization confusion.
> > I hope that 3 patches will make review easier.
>
> Not really. I'm reviewing the 3 patches put together rather than each
> one individually, which would require more work.
I figured with e.g. a separate patch for moving and alphabetizing
that it'd be easier to check that nothing got lost. Anyhow,
Just one patch this time.
> convert: I'd merge the 2 first sentences to state that if convert
> from X to Y. The doc does not say explicitely what happens if a
> character cannot be converted. After testing, an error is raised. The
> example comment could add ", if possible".
Done. Good idea. I reworked the whole paragraph to shorten and
clarify since I was altering it anyway. This does introduce
some inconsistency with wording that appears elsewhere but it seems
worth it because the listentry box was getting overfull.
> to_hex: add "." at the end of the sentence?
I left as-is, without a ".". The only consistent rule about
periods in the listentrys seems to be that if there's more than
one sentence then there's periods -- I think. In any case a
lot of them don't have periods and probably don't need
periods. I don't know what to do and since the original did
not have a period it seems better to leave well enough alone.
> Minor comment: you usually put two spaces between a "." and the first
> world of then next sentence, but not always.
I now always put 2 spaces after the end of a sentence. But
I've only done this where I've changed text, not when
moving pre-existing text around. Again, there seems
to be no consistency in the original. (I believe docbook
redoes all inter-sentence spacing anyway.)
Thanks for the help.
I'll be sure to sign up to review a patch (or patches) when life
permits.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a25c122ac8..131a63b36c 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1795,48 +1795,6 @@
- convert
-
-convert(string bytea,
-src_encoding name,
-dest_encoding name)
-
- bytea
-
-Convert string to dest_encoding. The
-original encoding is specified by
-src_encoding. The
-string must be valid in this encoding.
-Conversions can be defined by CREATE CONVERSION.
-Also there are some predefined conversions. See for available conversions.
-
- convert('text_in_utf8', 'UTF8', 'LATIN1')
- text_in_utf8 represented in Latin-1
- encoding (ISO 8859-1)
-
-
-
-
-
- convert_from
-
-convert_from(string bytea,
-src_encoding name)
-
- text
-
-Convert string to the database encoding. The original encoding
-is specified by src_encoding. The
-string must be valid in this encoding.
-
- convert_from('text_in_utf8', 'UTF8')
- text_in_utf8 represented in the current database encoding
-
-
-
-
-
convert_to
convert_to(string text,
@@ -1844,7 +1802,8 @@
bytea
-Convert string to dest_encoding.
+Convert string to dest_encoding. See for available conversions.
convert_to('some text', 'UTF8')
some text represented in the UTF8 encoding
@@ -1855,39 +1814,24 @@
decode
+
+ base64 encoding
+
decode(string text,
format text)
bytea
Decode binary data from textual representation in string.
-Options for format are same as in encode.
+Options
+for format are same as
+in encode.
decode('MTIzAAE=', 'base64')
\x3132330001
-
-
- encode
-
-
Re: Patch to document base64 encoding
On Thu, 9 May 2019 06:50:12 +0200 (CEST) Fabien COELHO wrote: > You might consider reviewing other people patches, that is expected > to make the overall process work. There are several documentation or > comment patches in the queue. Understood. I thought I had built up some reviewing credit, from some time ago. But perhaps that just made up for previous patches. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
Er, ping. Nobody has reviewed the latest patchs. They still apply to master... I am re-attaching the patches. See descriptions below. On Mon, 11 Mar 2019 15:32:14 -0500 "Karl O. Pinc" wrote: > On Sun, 10 Mar 2019 08:15:35 +0100 (CET) > Fabien COELHO What's causing problems here is that the encode and decode > functions are listed in both the string functions section > and the binary functions section. A related but not-relevant > problem is that there are functions listed in the string > function section which take binary input. > > I asked about this on IRC and the brief reply was > unflattering to the existing documentation. > > So I'm going to fix this also. 3 patches attached: > > doc_base64_part1_v9.patch > > This moves functions taking bytea and other non-string > input into the binary string section, and vice versa. > Eliminates duplicate encode() and decode() documentation. > > Affects: convert(bytea, name, name) >convert_from(bytea, name) >encode(bytea, text) >length(bytea, name) >quote_nullable(anytype) >to_hex(int or bigint) >decode(text, text) > > Only moves, eliminates duplicates, and adjusts indentation. > > > doc_base64_part2_v9.patch > > Cleanup wording after moving functions between sections. > > > doc_base64_part3_v9.patch > > Documents base64, hex, and escape encode() and decode() > formats. > > > >> "The string and the binary encode and decode functions..." > > >> sentence looks strange to me, especially with the English > > >> article that I do not really master, so maybe it is ok. I'd have > > >> written something more straightforward, eg: "Functions encode > > >> and decode support the following encodings:", > > > > > > It is an atypical construction because I want to draw attention > > > that this is documentation not only for the encode() and decode() > > > in section 9.4. String Functions and Operators but also for the > > > encode() and decode in section 9.5. Binary String Functions and > > > Operators. Although I can't think of a better approach it makes me > > > uncomfortable that documentation written in one section applies > > > equally to functions in a different section. > > > > People coming from the binary doc would have no reason to look at > > the string paragraph anyway. > > > > > Do you think it would be useful to hyperlink the word "binary" > > > to section 9.5? > > > > Hmmm... I think that the link is needed in the other direction. > > I'm not sure what you mean here or if it's still relevant. > > > I'd suggest (1) to use a simpler and direct sentence in the string > > section, (2) to simplify/shorten the in cell description in the > > binary section, and (3) to add an hyperlink from the binary section > > which would point to the expanded explanation in the string section. > > > > > The idiomatic phrasing would be "Both the string and the binary > > > encode and decode functions..." but the word "both" adds > > > no information. Shorter is better. > > > > Possibly, although "Both" would insist on the fact that it applies > > to the two variants, which was your intention. > > I think this is no longer relevant. Although I'm not sure what > you mean by 3. The format names already hyperlink back to the > string docs. > > > >> and also I'd use a direct "Function > > >> <...>decode ..." rather than "The > > >> decode function ..." (twice). > > > > > > The straightforward English would be "Decode accepts...". The > > > problem is that this begins the sentence with the name of a > > > function. This does not work very well when the function name is > > > all lower case, and can have other problems where clarity is lost > > > depending on documentation output formatting. > > > > Yep. > > > > > I don't see a better approach. > > > > I suggested "Function <>decode ...", which is the kind of thing > > we do in academic writing to improve precision, because I thought it > > could be better:-) > > "Function <>decode ..." just does not work in English. > > > >> Maybe I'd use the exact same grammatical structure for all 3 > > >> cases, starting with "The <>whatever encoding converts bla > >
Re: Patch to document base64 encoding
Hi Fabien, On Sun, 10 Mar 2019 08:15:35 +0100 (CET) Fabien COELHO I registered as a reviewer in the CF app. Thanks. What's causing problems here is that the encode and decode functions are listed in both the string functions section and the binary functions section. A related but not-relevant problem is that there are functions listed in the string function section which take binary input. I asked about this on IRC and the brief reply was unflattering to the existing documentation. So I'm going to fix this also. 3 patches attached: doc_base64_part1_v9.patch This moves functions taking bytea and other non-string input into the binary string section, and vice versa. Eliminates duplicate encode() and decode() documentation. Affects: convert(bytea, name, name) convert_from(bytea, name) encode(bytea, text) length(bytea, name) quote_nullable(anytype) to_hex(int or bigint) decode(text, text) Only moves, eliminates duplicates, and adjusts indentation. doc_base64_part2_v9.patch Cleanup wording after moving functions between sections. doc_base64_part3_v9.patch Documents base64, hex, and escape encode() and decode() formats. > >> "The string and the binary encode and decode functions..." sentence > >> looks strange to me, especially with the English article that I do > >> not really master, so maybe it is ok. I'd have written something > >> more straightforward, eg: "Functions encode and decode support the > >> following encodings:", > > > > It is an atypical construction because I want to draw attention > > that this is documentation not only for the encode() and decode() > > in section 9.4. String Functions and Operators but also for the > > encode() and decode in section 9.5. Binary String Functions and > > Operators. Although I can't think of a better approach it makes me > > uncomfortable that documentation written in one section applies > > equally to functions in a different section. > > People coming from the binary doc would have no reason to look at the > string paragraph anyway. > > > Do you think it would be useful to hyperlink the word "binary" > > to section 9.5? > > Hmmm... I think that the link is needed in the other direction. I'm not sure what you mean here or if it's still relevant. > I'd suggest (1) to use a simpler and direct sentence in the string > section, (2) to simplify/shorten the in cell description in the > binary section, and (3) to add an hyperlink from the binary section > which would point to the expanded explanation in the string section. > > > The idiomatic phrasing would be "Both the string and the binary > > encode and decode functions..." but the word "both" adds > > no information. Shorter is better. > > Possibly, although "Both" would insist on the fact that it applies to > the two variants, which was your intention. I think this is no longer relevant. Although I'm not sure what you mean by 3. The format names already hyperlink back to the string docs. > >> and also I'd use a direct "Function > >> <...>decode ..." rather than "The decode > >> function ..." (twice). > > > > The straightforward English would be "Decode accepts...". The > > problem is that this begins the sentence with the name of a > > function. This does not work very well when the function name is > > all lower case, and can have other problems where clarity is lost > > depending on documentation output formatting. > > Yep. > > > I don't see a better approach. > > I suggested "Function <>decode ...", which is the kind of thing we > do in academic writing to improve precision, because I thought it > could be better:-) "Function <>decode ..." just does not work in English. > >> Maybe I'd use the exact same grammatical structure for all 3 cases, > >> starting with "The <>whatever encoding converts bla bla bla" > >> instead of varying the sentences. > > > > Agreed. Good idea. The first paragraph of each term has to > > do with encoding and the second with decoding. > > > > Uniformity in starting the second paragraphs helps make > > this clear, even though the first paragraphs are not uniform. > > With this I am not concerned that the first paragraphs > > do not have a common phrasing that's very explicit about > > being about encoding. > > > > Adjusted. > > Cannot see it fully in the v8 patch: > > - The base64 encoding is > - hex represents > - escape converts I did only the decode paras. I guess no reason not to make the first paras uniform as well. Done. I also alphabetized by format name. I hope that 3 patches will make review easier. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 03859a78ea..3d748b660f 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -1692,48 +1692,6 @@ -
Re: Patch to document base64 encoding
Hi Fabien (and Michael), On Wed, 6 Mar 2019 16:37:05 -0600 "Karl O. Pinc" wrote: > I'm confident that the behavior I documented is how PG behaves > but you should know what I did in case you want further > validation. > > Attached: doc_base64_v8.patch FYI. To avoid a stall in the patch submission process. I notice that nobody has signed up as a reviewer for this patch. When the patch looks "ready" it needs to be marked as such at the PG commitfest website and a committer will consider committing. The commitfest URL is: https://commitfest.postgresql.org/23/ No rush. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
Re: Patch to document base64 encoding
On Wed, 6 Mar 2019 19:30:16 +0100 (CET)
Fabien COELHO wrote:
> "... section 6.8" -> "... Section 6.8" (capital S).
Fixed.
> "The string and the binary encode and decode functions..." sentence
> looks strange to me, especially with the English article that I do
> not really master, so maybe it is ok. I'd have written something more
> straightforward, eg: "Functions encode and decode support the
> following encodings:",
It is an atypical construction because I want to draw attention that
this is documentation not only for the encode() and decode() in
section 9.4. String Functions and Operators but also for
the encode() and decode in section 9.5. Binary String Functions
and Operators. Although I can't think of a better approach
it makes me uncomfortable that documentation written in
one section applies equally to functions in a different section.
Do you think it would be useful to hyperlink the word "binary"
to section 9.5?
The idiomatic phrasing would be "Both the string and the binary
encode and decode functions..." but the word "both" adds
no information. Shorter is better.
> and also I'd use a direct "Function
> <...>decode ..." rather than "The decode
> function ..." (twice).
The straightforward English would be "Decode accepts...". The problem
is that this begins the sentence with the name of a function.
This does not work very well when the function name is all lower case,
and can have other problems where clarity is lost depending
on documentation output formatting.
I don't see a better approach.
> Maybe I'd use the exact same grammatical structure for all 3 cases,
> starting with "The <>whatever encoding converts bla bla bla"
> instead of varying the sentences.
Agreed. Good idea. The first paragraph of each term has to
do with encoding and the second with decoding.
Uniformity in starting the second paragraphs helps make
this clear, even though the first paragraphs are not uniform.
With this I am not concerned that the first paragraphs
do not have a common phrasing that's very explicit about
being about encoding.
Adjusted.
> Otherwise, all explanations look both precise and useful to me.
When writing I was slightly concerned about being overly precise;
permanently committing to behavior that might (possibly) be an artifact
of implementation. E.g., that hex decoding accepts both
upper and lower case A-F characters, what input is ignored
and what raises an error, etc. But it seems best
to document existing behavior, all of which has existed so long
anyway that changing it would be disruptive. If anybody cares
they can object.
I wrote the docs by reading the code and did only a little
actual testing to be sure that what I wrote is correct.
I also did not check for regression tests which confirm
the behavior I'm documenting. (It wouldn't hurt to have
such regression tests, if they don't already exist.
But writing regression tests is more than I want to take on
with this patch. Feel free to come up with tests. :-)
I'm confident that the behavior I documented is how PG behaves
but you should know what I did in case you want further
validation.
Attached: doc_base64_v8.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..e756bf53ba 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64 encoding
+
decode(string text,
format text)
@@ -1769,16 +1772,25 @@
encode
+
+ base64 encoding
+
+
+ hex encoding
+
+
+ escape encoding
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
-escape converts zero bytes and high-bit-set bytes to
-octal sequences (\nnn) and
-doubles backslashes.
+formats are:
+base64,
+hex,
+escape.
encode('123\000\001', 'base64')
MTIzAAE=
@@ -2365,6 +2377,90 @@
format treats a NULL as a zero-element array.
+
+ encode
+
+
+ decode
+
+
+ base64 encoding
+
+
+hex encoding
+
+
+escape encoding
+
+
+
+ The string and the binary encode
+ and decode functions support the following
+ encodings:
+
+
+
+ base64
+
+
+ The base64 encoding is that
+ of https://tools.ietf.org/html/rfc2045#section-6.8;>RFC
+ 2045 Section 6.8. As per the RFC, encoded lines are
+ broken at 76 characters. However instead of the MIME CRLF
+ end-of-line marker, only a newline is used for end-of-line.
+
+
+
Re: Patch to document base64 encoding
On Wed, 6 Mar 2019 07:09:48 -0600
"Karl O. Pinc" wrote:
> On Tue, 5 Mar 2019 23:23:20 -0600
> "Karl O. Pinc" wrote:
>
> > Added documentation for hex and escape encodings,
> > including output formats and what are acceptable
> > inputs.
Attached: doc_base64_v7.patch
Improved escape decoding sentence.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..989344c98c 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64 encoding
+
decode(string text,
format text)
@@ -1769,16 +1772,25 @@
encode
+
+ base64 encoding
+
+
+ hex encoding
+
+
+ escape encoding
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
-escape converts zero bytes and high-bit-set bytes to
-octal sequences (\nnn) and
-doubles backslashes.
+formats are:
+base64,
+hex,
+escape.
encode('123\000\001', 'base64')
MTIzAAE=
@@ -2365,6 +2377,90 @@
format treats a NULL as a zero-element array.
+
+ encode
+
+
+ decode
+
+
+ base64 encoding
+
+
+hex encoding
+
+
+escape encoding
+
+
+
+ The string and the binary encode
+ and decode functions support the following
+ encodings:
+
+
+
+ base64
+
+
+ The base64 encoding is that
+ of https://tools.ietf.org/html/rfc2045#section-6.8;>RFC
+ 2045 section 6.8. As per the RFC, encoded lines are
+ broken at 76 characters. However instead of the MIME CRLF
+ end-of-line marker, only a newline is used for end-of-line.
+
+
+ The carriage-return, newline, space, and tab characters are
+ ignored by decode. Otherwise, an error is
+ raised when decode is supplied invalid
+ base64 data including when trailing padding is incorrect.
+
+
+
+
+
+ hex
+
+
+ hex represents each 4 bits of data as a single
+ hexadecimal digit, 0
+ through f. Encoding outputs
+ the a-f hex digits in lower
+ case. Because the smallest unit of data is 8 bits there are
+ always an even number of characters returned
+ by encode.
+
+
+ The decode function
+ accepts a-f characters in
+ either upper or lower case. An error is raised
+ when decode is supplied invalid hex data
+ including when given an odd number of characters.
+
+
+
+
+
+ escape
+
+
+ escape converts zero bytes and high-bit-set
+ bytes to octal sequences
+ (\nnn) and doubles
+ backslashes. Encoding always produces 4 characters for each
+ high-bit-set input byte.
+
+
+ The decode function accepts fewer than three
+ octal digits after a \ character. An error is
+ raised when decode is supplied a
+ single \ not followed by an octal digit.
+
+
+
+
+
+
See also the aggregate function string_agg in
.
@@ -3577,16 +3673,25 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
encode
+
+ base64 encoding
+
+
+ hex encoding
+
+
+ escape encoding
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
- formats are: base64, hex, escape.
- escape converts zero bytes and high-bit-set bytes to
- octal sequences (\nnn) and
- doubles backslashes.
+ formats are:
+ base64,
+ hex,
+ escape.
encode('123\000456'::bytea, 'escape')
123\000456
Re: Patch to document base64 encoding
On Tue, 5 Mar 2019 23:23:20 -0600
"Karl O. Pinc" wrote:
> Added documentation for hex and escape encodings,
> including output formats and what are acceptable
> inputs.
Attached: doc_base64_v6.patch
Added index entries for encode and decode functions
above the encoding documentation. As index entries
are currently generated this does not make any
difference. But this paragraph is very far
from the other encode/decode index targets on
the page.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..bf724d3dee 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64 encoding
+
decode(string text,
format text)
@@ -1769,16 +1772,25 @@
encode
+
+ base64 encoding
+
+
+ hex encoding
+
+
+ escape encoding
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
-escape converts zero bytes and high-bit-set bytes to
-octal sequences (\nnn) and
-doubles backslashes.
+formats are:
+base64,
+hex,
+escape.
encode('123\000\001', 'base64')
MTIzAAE=
@@ -2365,6 +2377,90 @@
format treats a NULL as a zero-element array.
+
+ encode
+
+
+ decode
+
+
+ base64 encoding
+
+
+hex encoding
+
+
+escape encoding
+
+
+
+ The string and the binary encode
+ and decode functions support the following
+ encodings:
+
+
+
+ base64
+
+
+ The base64 encoding is that
+ of https://tools.ietf.org/html/rfc2045#section-6.8;>RFC
+ 2045 section 6.8. As per the RFC, encoded lines are
+ broken at 76 characters. However instead of the MIME CRLF
+ end-of-line marker, only a newline is used for end-of-line.
+
+
+ The carriage-return, newline, space, and tab characters are
+ ignored by decode. Otherwise, an error is
+ raised when decode is supplied invalid
+ base64 data including when trailing padding is incorrect.
+
+
+
+
+
+ hex
+
+
+ hex represents each 4 bits of data as a single
+ hexadecimal digit, 0
+ through f. Encoding outputs
+ the a-f hex digits in lower
+ case. Because the smallest unit of data is 8 bits there are
+ always an even number of characters returned
+ by encode.
+
+
+ The decode function
+ accepts a-f characters in
+ either upper or lower case. An error is raised
+ when decode is supplied invalid hex data
+ including when given an odd number of characters.
+
+
+
+
+
+ escape
+
+
+ escape converts zero bytes and high-bit-set
+ bytes to octal sequences
+ (\nnn) and doubles
+ backslashes. Encoding always produces 4 characters for each
+ high-bit-set input byte.
+
+
+ The decode function accepts any number of
+ octal digits after a \ character. An error is
+ raised when decode is supplied a
+ single \ not followed by an octal digit.
+
+
+
+
+
+
See also the aggregate function string_agg in
.
@@ -3577,16 +3673,25 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
encode
+
+ base64 encoding
+
+
+ hex encoding
+
+
+ escape encoding
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
- formats are: base64, hex, escape.
- escape converts zero bytes and high-bit-set bytes to
- octal sequences (\nnn) and
- doubles backslashes.
+ formats are:
+ base64,
+ hex,
+ escape.
encode('123\000456'::bytea, 'escape')
123\000456
Re: Patch to document base64 encoding
On Wed, 6 Mar 2019 11:27:38 +0900
Michael Paquier wrote:
> On Tue, Mar 05, 2019 at 07:55:22PM -0600, Karl O. Pinc wrote:
> > Attached: doc_base64_v4.patch
>
> Details about the "escape" mode are already available within the
> description of function "encode". Wouldn't we want to consolidate a
> description for all the modes at the same place, including some words
> for hex? Your patch only includes the description of base64, which is
> a good addition, still not consistent with the rest. A paragraph
> after all the functions listed is fine I think as the description is
> long so it would bloat the table if included directly.
Makes sense. (As did hyperlinking to the RFC.)
(No matter how simple I think a patch is going to be it
always turns into a project. :)
Attached: doc_base64_v5.patch
Made index entries for hex and escape encodings.
Added word "encoding" to index entries.
Made entries with terms for
base64, hex, and escape encodings.
Added documentation for hex and escape encodings,
including output formats and what are acceptable
inputs.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..bd337fd530 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64 encoding
+
decode(string text,
format text)
@@ -1769,16 +1772,25 @@
encode
+
+ base64 encoding
+
+
+ hex encoding
+
+
+ escape encoding
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
-escape converts zero bytes and high-bit-set bytes to
-octal sequences (\nnn) and
-doubles backslashes.
+formats are:
+base64,
+hex,
+escape.
encode('123\000\001', 'base64')
MTIzAAE=
@@ -2365,6 +2377,84 @@
format treats a NULL as a zero-element array.
+
+ base64 encoding
+
+
+hex encoding
+
+
+escape encoding
+
+
+
+ The string and the binary encode
+ and decode functions support the following
+ encodings:
+
+
+
+ base64
+
+
+ The base64 encoding is that
+ of https://tools.ietf.org/html/rfc2045#section-6.8;>RFC
+ 2045 section 6.8. As per the RFC, encoded lines are
+ broken at 76 characters. However instead of the MIME CRLF
+ end-of-line marker, only a newline is used for end-of-line.
+
+
+ The carriage-return, newline, space, and tab characters are
+ ignored by decode. Otherwise, an error is
+ raised when decode is supplied invalid
+ base64 data including when trailing padding is incorrect.
+
+
+
+
+
+ hex
+
+
+ hex represents each 4 bits of data as a single
+ hexadecimal digit, 0
+ through f. Encoding outputs
+ the a-f hex digits in lower
+ case. Because the smallest unit of data is 8 bits there are
+ always an even number of characters returned
+ by encode.
+
+
+ The decode function
+ accepts a-f characters in
+ either upper or lower case. An error is raised
+ when decode is supplied invalid hex data
+ including when given an odd number of characters.
+
+
+
+
+
+ escape
+
+
+ escape converts zero bytes and high-bit-set
+ bytes to octal sequences
+ (\nnn) and doubles
+ backslashes. Encoding always produces 4 characters for each
+ high-bit-set input byte.
+
+
+ The decode function accepts any number of
+ octal digits after a \ character. An error is
+ raised when decode is supplied a
+ single \ not followed by an octal digit.
+
+
+
+
+
+
See also the aggregate function string_agg in
.
@@ -3577,16 +3667,25 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
encode
+
+ base64 encoding
+
+
+ hex encoding
+
+
+ escape encoding
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
- formats a
Re: Patch to document base64 encoding
Hi Fabien,
On Tue, 5 Mar 2019 23:02:26 +0100 (CET)
Fabien COELHO wrote:
> > Attached: doc_base64_v3.patch
>
> I'm ok with referencing the historical MIME RFC.
For the record, RFC 2045 is updated but not
yet obsolete. The updates don't invalidate
section 6.8.
> "RFC2045 section 6.8" -> "RFC 2045 Section 6.8"
>
> you can link to the RFC directly with:
>
> https://tools.ietf.org/html/rfc2045#section-6.8;>RFC 2045
> Section 6.8
Done.
Attached: doc_base64_v4.patch
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..06dfd84d22 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64
+
decode(string text,
format text)
@@ -1769,13 +1772,16 @@
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
+formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
@@ -2365,6 +2371,23 @@
format treats a NULL as a zero-element array.
+
+ base64
+
+
+
+ The base64 encoding of the encode
+ and decode functions is that
+ of https://tools.ietf.org/html/rfc2045#section-6.8;>RFC 2045
+ section 6.8. As per the RFC, encoded lines are broken at 76
+ characters. However instead of the MIME CRLF end-of-line marker, only a
+ newline is used for end-of-line. The carriage-return, newline, space,
+ and tab characters are ignored by decode.
+ Otherwise, an error is raised when decode is
+ supplied invalid base64 data including when trailing padding is
+ incorrect.
+
+
See also the aggregate function string_agg in
.
@@ -3577,13 +3600,16 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
- formats are: base64, hex, escape.
+ formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
Re: Patch to document base64 encoding
On Tue, 5 Mar 2019 07:26:17 -0600
"Karl O. Pinc" wrote:
> (I am not entirely pleased with the double dash
> but can't come up with anything better. And
> can't make an emdash entity work either.)
Attached: doc_base64_v3.patch
There is an mdash entity. This patch uses that.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..e5ae322faa 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64
+
decode(string text,
format text)
@@ -1769,13 +1772,16 @@
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
+formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
@@ -2365,6 +2371,21 @@
format treats a NULL as a zero-element array.
+
+ base64
+
+
+
+ The base64 encoding of the encode
+ and decode functions is that of RFC2045 section 6.8.
+ As per the RFC, encoded lines are broken at 76 characters. However
+ instead of the MIME CRLF end-of-line marker, only a newline is used for
+ end-of-line. The carriage-return, newline, space, and tab characters are
+ ignored by decode. Otherwise, an error is raised
+ when decode is supplied invalid base64 data
+ including when trailing padding is incorrect.
+
+
See also the aggregate function string_agg in
.
@@ -3577,13 +3598,16 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
- formats are: base64, hex, escape.
+ formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
Re: Patch to document base64 encoding
Hi Fabien,
On Tue, 5 Mar 2019 07:09:01 +0100 (CET)
Fabien COELHO wrote:
> > Doc patch, against master. Documents encode() and decode() base64
> > format.
>
> It is already documented. Enhance documentation, though.
Right. I was thinking that there are various implementations
of the base64 data format and so it needed more than
just to be named.
> > Attached: doc_base64_v1.patch
> >
> > References RFC2045 section 6.8 to define base64.
>
> Did you consider referencing RFC 4648 instead?
Not really. What drew me to document was the line
breaks every 76 characters. So I pretty much went
straight to the MIME RFC which says there should
be breaks at 76 characters.
I can see advantages and disadvantages either way.
More or less extraneous information either semi
or not base64 related in either RFC.
Which RFC do you think should be referenced?
Attached: doc_base64_v2.patch
This new version adds a phrase clarifying that
decode errors are raised when trailing padding
is wrong. Seemed like I may as well be explicit.
(I am not entirely pleased with the double dash
but can't come up with anything better. And
can't make an emdash entity work either.)
Thanks for taking a look.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..95b8f5cd5a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64
+
decode(string text,
format text)
@@ -1769,13 +1772,16 @@
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
+formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
@@ -2365,6 +2371,21 @@
format treats a NULL as a zero-element array.
+
+ base64
+
+
+
+ The base64 encoding of the encode
+ and decode functions is that of RFC2045 section 6.8.
+ As per the RFC, encoded lines are broken at 76 characters. However
+ instead of the MIME CRLF end-of-line marker, only a newline is used for
+ end-of-line. The carriage-return, newline, space, and tab characters are
+ ignored by decode. Otherwise, an error is raised
+ when decode is supplied invalid base64 data --
+ including when trailing padding is incorrect.
+
+
See also the aggregate function string_agg in
.
@@ -3577,13 +3598,16 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
- formats are: base64, hex, escape.
+ formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
Patch to document base64 encoding
Hi,
Doc patch, against master. Documents encode() and decode() base64
format.
Builds for me.
Attached: doc_base64_v1.patch
References RFC2045 section 6.8 to define base64.
Because encode() and decode() show up in both the string
functions section and the binary string functions section
I documented in only the string functions section and hyperlinked
"base64" in both sections to the new text.
Note that XML output can also generate base64 data. I suspect
this is done via the (different, src/common/base64.c)
pg_b64_encode() function which does not limit line length.
In any case this patch does not touch the XML documentation.
Regards,
Karl
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6765b0d584..f6b4b2c5a0 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1752,6 +1752,9 @@
decode
+
+ base64
+
decode(string text,
format text)
@@ -1769,13 +1772,16 @@
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
-formats are: base64, hex, escape.
+formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
@@ -2365,6 +2371,20 @@
format treats a NULL as a zero-element array.
+
+ base64
+
+
+
+ The base64 encoding of the encode
+ and decode functions is that of RFC2045 section 6.8.
+ As per the RFC, encoded lines are broken at 76 characters. However
+ instead of the MIME CRLF end-of-line marker, only a newline is used for
+ end-of-line. The carriage-return, newline, space, and tab characters are
+ ignored by decode. Otherwise, an error is raised
+ when decode is supplied invalid base64 data.
+
+
See also the aggregate function string_agg in
.
@@ -3577,13 +3597,16 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
encode
+
+ base64
+
encode(data bytea,
format text)
text
Encode binary data into a textual representation. Supported
- formats are: base64, hex, escape.
+ formats are: base64, hex, escape.
escape converts zero bytes and high-bit-set bytes to
octal sequences (\nnn) and
doubles backslashes.
Doc client_min_messages patch vis. INFO message severity
Hi, Attached is documentation patch: doc_client_min_messages_v1.patch Document that INFO severity messages are always sent to the client. This also adds hyperlinks to the table of severity levels where those levels are referenced in the docs. The patch was discussed on the #postgresql IRC channel with RhodiumToad. The patch is against master. It passes the xmllint, builds html, and generally appears to work. The motivation for this patch is that the client_min_messages documentation does not mention the INFO level. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e94b305add..a0098f93d1 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -5667,7 +5667,8 @@ local0.*/var/log/postgresql -Controls which message levels are written to the server log. +Controls which message +levels are written to the server log. Valid values are DEBUG5, DEBUG4, DEBUG3, DEBUG2, DEBUG1, INFO, NOTICE, WARNING, @@ -5693,7 +5694,9 @@ local0.*/var/log/postgresql Controls which SQL statements that cause an error condition are recorded in the server log. The current SQL statement is included in the log entry for any message of -the specified severity or higher. +the specified +severity +or higher. Valid values are DEBUG5, DEBUG4, DEBUG3, DEBUG2, DEBUG1, @@ -7025,7 +7028,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; -Controls which message levels are sent to the client. +Controls which +message levels +are sent to the client. Valid values are DEBUG5, DEBUG4, DEBUG3, DEBUG2, DEBUG1, LOG, NOTICE, @@ -7035,7 +7040,10 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; NOTICE. Note that LOG has a different rank here than in . - + +INFO level messages are always sent to the client. + +
