public inbox for [email protected]
help / color / mirror / Atom feedRe: [PGdocs] fix description for handling pf non-ASCII characters
22+ messages / 6 participants
[nested] [flat]
* Re: [PGdocs] fix description for handling pf non-ASCII characters
@ 2023-09-26 06:03 Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Karl O. Pinc @ 2023-09-26 06:03 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; 'jian he' <[email protected]>; +Cc: Postgres hackers <[email protected]>
Hello Hayato and Jian,
On Tue, 4 Jul 2023 01:30:56 +0000
"Hayato Kuroda (Fujitsu)" <[email protected]> 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
<structname>pg_stat_activity</structname> view and included in CSV log
entries. It can also be included in regular log entries via the <xref
linkend="guc-log-line-prefix"/> parameter.
- Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters
will be
- replaced with question marks (<literal>?</literal>).
+ Non-ASCII characters used in the
<varname>application_name</varname>
+ will be replaced with hexadecimal strings.
</para>
</listitem>
</varlistentry>
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:
<link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
byte value</link>.
I think. You link to id="someidvalue" attribute values.)
@@ -8037,10 +8036,9 @@ COPY postgres_log FROM
'/full/path/to/logfile.csv' WITH csv; <para>
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in
a standard
- build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name
is shown
- if this parameter is set to the empty string
<literal>''</literal> (which is
+ build). Non-ASCII characters used in the
<varname>cluster_name</varname>
+ will be replaced with hexadecimal strings. No name is shown if
this
+ parameter is set to the empty string <literal>''</literal>
(which is the default). This parameter can only be set at server start.
</para>
</listitem>
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 <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters and non-ASCII characters
will be
+ replaced with hexadecimal strings.
See <xref linkend="guc-application-name"/> for details.
</para>
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 <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
@ 2023-09-26 13:40 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 06:48 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
0 siblings, 2 replies; 22+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-09-26 13:40 UTC (permalink / raw)
To: 'Karl O. Pinc' <[email protected]>; 'jian he' <[email protected]>; +Cc: Postgres hackers <[email protected]>
Dear Karl,
Thank you for reviewing! PSA new version.
> I see a few problems with the English and style of the patches
> and am commenting below and have signed up as a reviewer.
Your effort is quite helpful for me.
> 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.
Sure. I will change the status after posting the patch.
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)
```
Note that non-printable ASCIIs are also replaced with the same rule.
```
psql (15.4)
Type "help" for help.
postgres=# SET application_name TO E'\x03';
SET
postgres=# SHOW application_name ;
application_name
------------------
?
(1 row)
psql (17devel)
Type "help" for help.
postgres=# SET application_name TO E'\x03';
SET
postgres=# SHOW application_name ;
application_name
------------------
\x03
(1 row)
```
> 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.
I didn't know that. Could you please tell me if you have a source? Anyway,
I put a line break for each sentences for now.
> 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
> <structname>pg_stat_activity</structname> view and included in CSV log
> entries. It can also be included in regular log entries via the <xref
> linkend="guc-log-line-prefix"/> parameter.
> - Only printable ASCII characters may be used in the
> - <varname>application_name</varname> value. Other characters
> will be
> - replaced with question marks (<literal>?</literal>).
> + Non-ASCII characters used in the
> <varname>application_name</varname>
> + will be replaced with hexadecimal strings.
> </para>
> </listitem>
> </varlistentry>
>
> 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-ST
> RINGS-ESCAPE
>
> (The docbook would be, depending on text you want to link:
>
> <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
> byte value</link>.
>
> I think. You link to id="someidvalue" attribute values.)
IIUC the word " Postgres" cannot be used in the doc.
Based on your all comments, I changed as below. How do you think?
```
Characters that are not printable ASCII, like <literal>\x03</literal>,
are replaced with the <productname>PostgreSQL</productname>
<link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte value</link>.
```
> @@ -8037,10 +8036,9 @@ COPY postgres_log FROM
> '/full/path/to/logfile.csv' WITH csv; <para>
> The name can be any string of less
> than <symbol>NAMEDATALEN</symbol> characters (64 characters
> in
> a standard
> - build). Only printable ASCII characters may be used in the
> - <varname>cluster_name</varname> value. Other characters will be
> - replaced with question marks (<literal>?</literal>). No name
> is shown
> - if this parameter is set to the empty string
> <literal>''</literal> (which is
> + build). Non-ASCII characters used in the
> <varname>cluster_name</varname>
> + will be replaced with hexadecimal strings. No name is shown if
> this
> + parameter is set to the empty string <literal>''</literal>
> (which is the default). This parameter can only be set at server start.
> </para>
> </listitem>
>
> Same review as for the first patch hunk.
Fixed like above. You can refer the patch.
> 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 <varname>application_name</varname>
> in a foreign server, note that it will be truncated to less than
> - <symbol>NAMEDATALEN</symbol> characters and anything other than
> - printable ASCII characters will be replaced with question
> - marks (<literal>?</literal>).
> + <symbol>NAMEDATALEN</symbol> characters and non-ASCII
> characters
> will be
> + replaced with hexadecimal strings.
> See <xref linkend="guc-application-name"/> for details.
> </para>
>
> Same review as for the first patch hunk.
Fixed like above.
> 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.
> 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?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v3_doc_fix.patch (3.2K, ../../TYAPR01MB586607D53C42E59DBB00FD9AF5C3A@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-v3_doc_fix.patch)
download | inline diff:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d42b7d63ee..e639f1d26d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6886,9 +6886,9 @@ local0.* /var/log/postgresql
The name will be displayed in the <structname>pg_stat_activity</structname> view
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
- Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ Characters that are not printable ASCII, like <literal>\x03</literal>,
+ are replaced with the <productname>PostgreSQL</productname>
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte value</link>.
</para>
</listitem>
</varlistentry>
@@ -7883,11 +7883,13 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
<para>
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
- build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ build).
+ Characters that are not printable ASCII, like <literal>\x03</literal>,
+ are replaced with the <productname>PostgreSQL</productname>
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte value</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default). This parameter can only
+ be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..f6b60ffdba 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1064,12 +1064,12 @@ postgres=# SELECT postgres_fdw_disconnect_all();
</para>
<para>
<varname>postgres_fdw.application_name</varname> can be any string
- of any length and contain even non-ASCII characters. However when
- it's passed to and used as <varname>application_name</varname>
+ of any length and contain even characters that are not printable ASCII.
+ However when it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
<symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ printable ASCII characters are replaced with the <productname>PostgreSQL</productname>
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte value</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
@ 2023-09-26 17:45 ` Karl O. Pinc <[email protected]>
2023-09-26 18:01 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Tom Lane <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 2 replies; 22+ messages in thread
From: Karl O. Pinc @ 2023-09-26 17:45 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'jian he' <[email protected]>; Postgres hackers <[email protected]>
On Tue, 26 Sep 2023 13:40:26 +0000
"Hayato Kuroda (Fujitsu)" <[email protected]> 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
> <literal>\x03</literal>, are replaced with the
> <productname>PostgreSQL</productname> <link
> linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
> value</link>. ```
>
> > 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 <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
@ 2023-09-26 18:01 ` Tom Lane <[email protected]>
2023-09-26 21:50 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 13:00 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 2 replies; 22+ messages in thread
From: Tom Lane @ 2023-09-26 18:01 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; 'jian he' <[email protected]>; Postgres hackers <[email protected]>
"Karl O. Pinc" <[email protected]> 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.
A smell test that works for native speakers (not sure how helpful
it is for others) is: if the sentence would read well with commas
or parens added before and after the clause, then it's probably
non-restrictive and should use "which". If it looks wrong that way
then it's a restrictive clause and should use "that".
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 18:01 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Tom Lane <[email protected]>
@ 2023-09-26 21:50 ` Karl O. Pinc <[email protected]>
1 sibling, 0 replies; 22+ messages in thread
From: Karl O. Pinc @ 2023-09-26 21:50 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; 'jian he' <[email protected]>; Postgres hackers <[email protected]>
Sep 26, 2023 1:10:55 PM Tom Lane <[email protected]>:
> "Karl O. Pinc" <[email protected]> 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.
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 18:01 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Tom Lane <[email protected]>
@ 2023-09-27 13:00 ` Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 0 replies; 22+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-09-27 13:00 UTC (permalink / raw)
To: 'Tom Lane' <[email protected]>; Karl O. Pinc <[email protected]>; +Cc: 'jian he' <[email protected]>; Postgres hackers <[email protected]>
Dear Tom,
> 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.
>
> A smell test that works for native speakers (not sure how helpful
> it is for others) is: if the sentence would read well with commas
> or parens added before and after the clause, then it's probably
> non-restrictive and should use "which". If it looks wrong that way
> then it's a restrictive clause and should use "that".
Thanks for giving your opinion. The suggestion is quite helpful for me,
non-native speaker. If you check my patch [1] I'm very happy.
[1]: https://www.postgresql.org/message-id/TYAPR01MB58663EB061888B2715A39217F5C2A%40TYAPR01MB5866.jpnprd0...
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
@ 2023-09-27 12:58 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-09-27 12:58 UTC (permalink / raw)
To: 'Karl O. Pinc' <[email protected]>; +Cc: 'jian he' <[email protected]>; Postgres hackers <[email protected]>
Dear Karl,
Thank you for reviewing!
> 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 it is not hard limit, but I followed this.
> 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?
> 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.
Thanks. Actually, I think it should be backpatched to PG16 because the commit was
done last year. I will make the patch for it after deciding the explanation.
>
> 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 ...".
>
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".
[1]: https://www.postgresql.org/message-id/CAHut%2BPvEbKC8ABA_daX-XPNOTFzuAmHGhjPj%3DtPZYQskRHECOg%40mail...
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
From c3a2c68fd74bce14af6df7c773317d496cf75209 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v32] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 17 +++++++++++------
doc/src/sgml/postgres-fdw.sgml | 7 ++++---
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38684af5b1..587bdf3d72 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6893,8 +6893,10 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ The bytes of other characters are replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
</para>
</listitem>
</varlistentry>
@@ -7890,10 +7892,13 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ The bytes of other characters are replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..9894d61a98 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,10 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters and the bytes of other than
+ printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ values</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
From 5f4dc139a3a401d22793e49d9a51caa00ba2ecad Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v3] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 17 +++++++++++------
doc/src/sgml/postgres-fdw.sgml | 7 ++++---
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38684af5b1..fdd93002bc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6893,8 +6893,10 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters will be replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
</para>
</listitem>
</varlistentry>
@@ -7890,10 +7892,13 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ Other characters will be replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..522f736144 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,10 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters and anything other than printable
+ ASCII characters will be replaced with <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ values</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
Attachments:
[text/plain] Both_v3-0001-Fix-description-for-handling-of-non-printable-AS.txt (3.9K, ../../TYAPR01MB58663EB061888B2715A39217F5C2A@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-Both_v3-0001-Fix-description-for-handling-of-non-printable-AS.txt)
download | inline diff:
From c3a2c68fd74bce14af6df7c773317d496cf75209 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v32] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 17 +++++++++++------
doc/src/sgml/postgres-fdw.sgml | 7 ++++---
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38684af5b1..587bdf3d72 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6893,8 +6893,10 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ The bytes of other characters are replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
</para>
</listitem>
</varlistentry>
@@ -7890,10 +7892,13 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ The bytes of other characters are replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..9894d61a98 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,10 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters and the bytes of other than
+ printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ values</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
[application/octet-stream] v3-0001-Fix-description-for-handling-of-non-printable-AS.patch (4.5K, ../../TYAPR01MB58663EB061888B2715A39217F5C2A@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v3-0001-Fix-description-for-handling-of-non-printable-AS.patch)
download | inline diff:
From 8e756854de95794f3d30584cee70b94b5822e424 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 12:12:16 +0000
Subject: [PATCH v31] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 22 ++++++++++++++--------
doc/src/sgml/postgres-fdw.sgml | 12 +++++++-----
2 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38684af5b1..9163e55c6d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6892,9 +6892,11 @@ local0.* /var/log/postgresql
The name will be displayed in the <structname>pg_stat_activity</structname> view
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
- Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ The bytes of the string which are not printable ASCII, like
+ <literal>\x03</literal>, are replaced with the
+ <productname>PostgreSQL</productname> <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ value</link>.
</para>
</listitem>
</varlistentry>
@@ -7889,11 +7891,15 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
<para>
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
- build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ build).
+ The bytes of the string which are not printable ASCII, like
+ <literal>\x03</literal>, are replaced with the
+ <productname>PostgreSQL</productname> <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ value</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default). This parameter can only
+ be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..dcbcf84911 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1064,12 +1064,14 @@ postgres=# SELECT postgres_fdw_disconnect_all();
</para>
<para>
<varname>postgres_fdw.application_name</varname> can be any string
- of any length and contain even non-ASCII characters. However when
- it's passed to and used as <varname>application_name</varname>
+ of any length and contain even characters that are not printable ASCII.
+ However when it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters and the bytes of the string that
+ are not printable ASCII characters are replaced with the
+ <productname>PostgreSQL</productname> <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ value</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
[text/plain] Peter_v3-0001-Fix-description-for-handling-of-non-printable-AS.txt (3.9K, ../../TYAPR01MB58663EB061888B2715A39217F5C2A@TYAPR01MB5866.jpnprd01.prod.outlook.com/4-Peter_v3-0001-Fix-description-for-handling-of-non-printable-AS.txt)
download | inline diff:
From 5f4dc139a3a401d22793e49d9a51caa00ba2ecad Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v3] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 17 +++++++++++------
doc/src/sgml/postgres-fdw.sgml | 7 ++++---
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38684af5b1..fdd93002bc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6893,8 +6893,10 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters will be replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
</para>
</listitem>
</varlistentry>
@@ -7890,10 +7892,13 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ Other characters will be replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..522f736144 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,10 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters and anything other than printable
+ ASCII characters will be replaced with <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ values</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
@ 2023-09-27 13:59 ` Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Karl O. Pinc @ 2023-09-27 13:59 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'jian he' <[email protected]>; Postgres hackers <[email protected]>; 'Peter Smith' <[email protected]>
On Wed, 27 Sep 2023 12:58:54 +0000
"Hayato Kuroda (Fujitsu)" <[email protected]> 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 <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
@ 2023-09-27 23:49 ` Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Peter Smith @ 2023-09-27 23:49 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>
On Wed, Sep 27, 2023 at 11:59 PM Karl O. Pinc <[email protected]> wrote:
>
> On Wed, 27 Sep 2023 12:58:54 +0000
> "Hayato Kuroda (Fujitsu)" <[email protected]> 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...
~~~
+ <varname>application_name</varname> value.
+ The bytes of other characters are replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
V
+ <varname>cluster_name</varname> value.
+ The bytes of other characters are replaced with
+ <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
+ byte values</link>.
V
+ <symbol>NAMEDATALEN</symbol> characters and the bytes of other than
+ printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
+ values</link>.
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
<link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
byte values</link>.
======
[1] https://www.postgresql.org/message-id/20230927085924.4198c3d2%40slate.karlpinc.com
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
@ 2023-09-28 00:30 ` Karl O. Pinc <[email protected]>
2023-09-28 01:13 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 02:48 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 2 replies; 22+ messages in thread
From: Karl O. Pinc @ 2023-09-28 00:30 UTC (permalink / raw)
To: Peter Smith <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
On Thu, 28 Sep 2023 09:49:03 +1000
Peter Smith <[email protected]> wrote:
> On Wed, Sep 27, 2023 at 11:59 PM Karl O. Pinc <[email protected]>
> wrote:
> >
> > On Wed, 27 Sep 2023 12:58:54 +0000
> > "Hayato Kuroda (Fujitsu)" <[email protected]> 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...
>
> ~~~
>
> + <varname>application_name</varname> value.
> + The bytes of other characters are replaced with
> + <link linkend="sql-syntax-strings-escape">C-style escaped
> hexadecimal
> + byte values</link>.
>
> V
>
> + <varname>cluster_name</varname> value.
> + The bytes of other characters are replaced with
> + <link linkend="sql-syntax-strings-escape">C-style escaped
> hexadecimal
> + byte values</link>.
>
> V
>
> + <symbol>NAMEDATALEN</symbol> characters and the bytes of other
> than
> + printable ASCII characters are replaced with <link
> + linkend="sql-syntax-strings-escape">C-style escaped
> hexadecimal byte
> + values</link>.
>
>
> 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
> <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
> byte values</link>.
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 <[email protected]>
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.
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
@ 2023-09-28 01:13 ` Peter Smith <[email protected]>
2023-09-28 01:19 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Tom Lane <[email protected]>
2023-09-28 02:50 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 2 replies; 22+ messages in thread
From: Peter Smith @ 2023-09-28 01:13 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
On Thu, Sep 28, 2023 at 10:30 AM Karl O. Pinc <[email protected]> wrote:
>
> On Thu, 28 Sep 2023 09:49:03 +1000
> Peter Smith <[email protected]> wrote:
>
> > On Wed, Sep 27, 2023 at 11:59 PM Karl O. Pinc <[email protected]>
> > wrote:
> > >
> > > On Wed, 27 Sep 2023 12:58:54 +0000
> > > "Hayato Kuroda (Fujitsu)" <[email protected]> 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...
> >
> > ~~~
> >
> > + <varname>application_name</varname> value.
> > + The bytes of other characters are replaced with
> > + <link linkend="sql-syntax-strings-escape">C-style escaped
> > hexadecimal
> > + byte values</link>.
> >
> > V
> >
> > + <varname>cluster_name</varname> value.
> > + The bytes of other characters are replaced with
> > + <link linkend="sql-syntax-strings-escape">C-style escaped
> > hexadecimal
> > + byte values</link>.
> >
> > V
> >
> > + <symbol>NAMEDATALEN</symbol> characters and the bytes of other
> > than
> > + printable ASCII characters are replaced with <link
> > + linkend="sql-syntax-strings-escape">C-style escaped
> > hexadecimal byte
> > + values</link>.
> >
> >
> > 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
> > <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
> > byte values</link>.
>
> 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.
>
I had in mind something like a SHIFT-JIS encoding where a single
"character" may include some trail bytes that happen to be in the
ASCII printable range. AFAIK because the new logic is processing
bytes, not characters, I thought the end result could be a mix of
escaped and unescaped bytes for the single SJIS character. In that
context, I felt "The bytes of other characters" was not quite
accurate.
But now looking at PostgreSQL-supported character sets [1] I saw SJIS
is not supported anyhow. Unfortunately, I am not familiar enough with
other encodings to know if there is still a chance of similar
printable ASCII trail bytes so I am fine with whatever wording is
chosen.
> 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?
>
+1.
======
[1] https://www.postgresql.org/docs/current/multibyte.html
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-28 01:13 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
@ 2023-09-28 01:19 ` Tom Lane <[email protected]>
2023-09-28 01:41 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Tom Lane @ 2023-09-28 01:19 UTC (permalink / raw)
To: Peter Smith <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; Hayato Kuroda (Fujitsu) <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>
Peter Smith <[email protected]> writes:
> I had in mind something like a SHIFT-JIS encoding where a single
> "character" may include some trail bytes that happen to be in the
> ASCII printable range. AFAIK because the new logic is processing
> bytes, not characters, I thought the end result could be a mix of
> escaped and unescaped bytes for the single SJIS character.
It will not, because ...
> But now looking at PostgreSQL-supported character sets [1] I saw SJIS
> is not supported anyhow. Unfortunately, I am not familiar enough with
> other encodings to know if there is still a chance of similar
> printable ASCII trail bytes so I am fine with whatever wording is
> chosen.
... trailing bytes that could be mistaken for ASCII are precisely
the property that causes us to reject an encoding as not backend-safe.
So this code doesn't need to consider that hazard, and processing the
string byte-by-byte is perfectly OK.
I'd be inclined to keep the text as simple as possible and not focus on
the distinction between bytes and characters.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-28 01:13 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 01:19 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Tom Lane <[email protected]>
@ 2023-09-28 01:41 ` Peter Smith <[email protected]>
0 siblings, 0 replies; 22+ messages in thread
From: Peter Smith @ 2023-09-28 01:41 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; Hayato Kuroda (Fujitsu) <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>
On Thu, Sep 28, 2023 at 11:19 AM Tom Lane <[email protected]> wrote:
>
> ... trailing bytes that could be mistaken for ASCII are precisely
> the property that causes us to reject an encoding as not backend-safe.
Oh, that is good to know. Thanks for the information.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-28 01:13 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
@ 2023-09-28 02:50 ` Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 0 replies; 22+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-09-28 02:50 UTC (permalink / raw)
To: 'Peter Smith' <[email protected]>; Karl O. Pinc <[email protected]>; +Cc: jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
Dear Peter,
Thank you for reviewing!
> > > > > 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...
Okay, C was chosen.
> > >
> > > ~~~
> > >
> > > + <varname>application_name</varname> value.
> > > + The bytes of other characters are replaced with
> > > + <link linkend="sql-syntax-strings-escape">C-style escaped
> > > hexadecimal
> > > + byte values</link>.
> > >
> > > V
> > >
> > > + <varname>cluster_name</varname> value.
> > > + The bytes of other characters are replaced with
> > > + <link linkend="sql-syntax-strings-escape">C-style escaped
> > > hexadecimal
> > > + byte values</link>.
> > >
> > > V
> > >
> > > + <symbol>NAMEDATALEN</symbol> characters and the bytes of
> other
> > > than
> > > + printable ASCII characters are replaced with <link
> > > + linkend="sql-syntax-strings-escape">C-style escaped
> > > hexadecimal byte
> > > + values</link>.
> > >
> > >
> > > 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
> > > <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
> > > byte values</link>.
Hmm, I felt that using exactly the same wording seemed strange here, so similar
words were used. Also, based on the comment [1], "byte" was removed.
>
> I had in mind something like a SHIFT-JIS encoding where a single
> "character" may include some trail bytes that happen to be in the
> ASCII printable range. AFAIK because the new logic is processing
> bytes, not characters, I thought the end result could be a mix of
> escaped and unescaped bytes for the single SJIS character. In that
> context, I felt "The bytes of other characters" was not quite
> accurate.
>
> But now looking at PostgreSQL-supported character sets [1] I saw SJIS
> is not supported anyhow. Unfortunately, I am not familiar enough with
> other encodings to know if there is still a chance of similar
> printable ASCII trail bytes so I am fine with whatever wording is
> chosen.
Based on the discussion [1], I did not handle the part.
>
> > 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?
> >
>
> +1.
OK, divided.
New patch is available in [2].
[1]: https://www.postgresql.org/message-id/803569.1695863971%40sss.pgh.pa.us
[2]: https://www.postgresql.org/message-id/TYAPR01MB5866DD962CA4FC03E338C6BBF5C1A%40TYAPR01MB5866.jpnprd0...
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
@ 2023-09-28 02:48 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:23 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-09-28 02:48 UTC (permalink / raw)
To: 'Karl O. Pinc' <[email protected]>; Peter Smith <[email protected]>; +Cc: jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
Dear Karl,
Thank you for giving comments! PSA new version.
I attached two patches - one is for HEAD, and another one is for REL_16_STABLE
branch. As shown below, PG16 has the same behavior.
```
psql (16beta3)
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)
```
> > > > 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.
Okay, basically I used C.
> > >
> > > 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...
> >
> > ~~~
> >
> > + <varname>application_name</varname> value.
> > + The bytes of other characters are replaced with
> > + <link linkend="sql-syntax-strings-escape">C-style escaped
> > hexadecimal
> > + byte values</link>.
> >
> > V
> >
> > + <varname>cluster_name</varname> value.
> > + The bytes of other characters are replaced with
> > + <link linkend="sql-syntax-strings-escape">C-style escaped
> > hexadecimal
> > + byte values</link>.
> >
> > V
> >
> > + <symbol>NAMEDATALEN</symbol> characters and the bytes of
> other
> > than
> > + printable ASCII characters are replaced with <link
> > + linkend="sql-syntax-strings-escape">C-style escaped
> > hexadecimal byte
> > + values</link>.
> >
> >
> > 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
> > <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal
> > byte values</link>.
>
> 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.)
Right, here we cannot use exactly the same sentence.
>
> 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.
Based on the suggestion [1], I removed the word "byte".
(Sorry, but a comment from senior members has higher priority)
>
> 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.
Right, the sentence is separated into two.
[1]: https://www.postgresql.org/message-id/803569.1695863971%40sss.pgh.pa.us
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] REL_16_v4-0001-Fix-description-for-handling-of-non-printable-ASC.patch (3.8K, ../../TYAPR01MB5866DD962CA4FC03E338C6BBF5C1A@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-REL_16_v4-0001-Fix-description-for-handling-of-non-printable-ASC.patch)
download | inline diff:
From 8f56b1cd93157a3c8b8f1c703c2752c0727e798c Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v40] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 15 +++++++++------
doc/src/sgml/postgres-fdw.sgml | 6 +++---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4647539e40..126c147fc6 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7012,8 +7012,9 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
</para>
</listitem>
</varlistentry>
@@ -8009,10 +8010,12 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..c177fd41bc 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,9 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters.
+ Anything other than printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
[application/octet-stream] v4-0001-Fix-description-for-handling-of-non-printable-ASC.patch (3.8K, ../../TYAPR01MB5866DD962CA4FC03E338C6BBF5C1A@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v4-0001-Fix-description-for-handling-of-non-printable-ASC.patch)
download | inline diff:
From 50dd04d524932b4cca41f4b8b27c474405ec2d9f Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v4] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 15 +++++++++------
doc/src/sgml/postgres-fdw.sgml | 6 +++---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38684af5b1..924309af26 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6893,8 +6893,9 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
</para>
</listitem>
</varlistentry>
@@ -7890,10 +7891,12 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..c177fd41bc 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,9 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters.
+ Anything other than printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-28 02:48 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
@ 2023-09-28 03:23 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:54 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Michael Paquier <[email protected]>
2023-09-28 04:23 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
0 siblings, 2 replies; 22+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-09-28 03:23 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; Peter Smith <[email protected]>; 'Karl O. Pinc' <[email protected]>; +Cc: jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
> I attached two patches - one is for HEAD, and another one is for REL_16_STABLE
> branch. As shown below, PG16 has the same behavior.
Hmm, cfbot got angry because it tried to apply both of patches. To avoid it, I repost renamed patch.
(I'm happy if we can specify the target branch of patches)
Sorry for inconvenience.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
From 8f56b1cd93157a3c8b8f1c703c2752c0727e798c Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v40] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 15 +++++++++------
doc/src/sgml/postgres-fdw.sgml | 6 +++---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4647539e40..126c147fc6 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7012,8 +7012,9 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
</para>
</listitem>
</varlistentry>
@@ -8009,10 +8010,12 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..c177fd41bc 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,9 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters.
+ Anything other than printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
Attachments:
[text/plain] REL_16_v4-0001-Fix-description-for-handling-of-non-printable-ASC.txt (3.8K, ../../TYAPR01MB586639C3B11FB5C9EE30AA29F5C1A@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-REL_16_v4-0001-Fix-description-for-handling-of-non-printable-ASC.txt)
download | inline diff:
From 8f56b1cd93157a3c8b8f1c703c2752c0727e798c Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v40] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 15 +++++++++------
doc/src/sgml/postgres-fdw.sgml | 6 +++---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4647539e40..126c147fc6 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7012,8 +7012,9 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
</para>
</listitem>
</varlistentry>
@@ -8009,10 +8010,12 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..c177fd41bc 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,9 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters.
+ Anything other than printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
[application/octet-stream] v4-0001-Fix-description-for-handling-of-non-printable-ASC.patch (3.8K, ../../TYAPR01MB586639C3B11FB5C9EE30AA29F5C1A@TYAPR01MB5866.jpnprd01.prod.outlook.com/3-v4-0001-Fix-description-for-handling-of-non-printable-ASC.patch)
download | inline diff:
From 50dd04d524932b4cca41f4b8b27c474405ec2d9f Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 27 Sep 2023 07:31:07 +0000
Subject: [PATCH v4] Fix description for handling of non-printable ASCII
characters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
45b1a67a changed the behavior when characters that are not printable ASCII were
used for three configuration parameters (application_name, cluster_name, and
postgres_fdw.application_name), but it was not documented. This commit fixes
that.
PG15 and prior:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
------------------
?????????
(1 row)
```
PG16 and later:
```
postgres=# SET application_name TO 'あああ';
SET
postgres=# SHOW application_name ;
application_name
--------------------------------------
\xe3\x81\x82\xe3\x81\x82\xe3\x81\x82
(1 row)
```
---
doc/src/sgml/config.sgml | 15 +++++++++------
doc/src/sgml/postgres-fdw.sgml | 6 +++---
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38684af5b1..924309af26 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6893,8 +6893,9 @@ local0.* /var/log/postgresql
and included in CSV log entries. It can also be included in regular
log entries via the <xref linkend="guc-log-line-prefix"/> parameter.
Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
</para>
</listitem>
</varlistentry>
@@ -7890,10 +7891,12 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
The name can be any string of less
than <symbol>NAMEDATALEN</symbol> characters (64 characters in a standard
build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string <literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ <varname>cluster_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default).
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5062d712e7..c177fd41bc 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -1067,9 +1067,9 @@ postgres=# SELECT postgres_fdw_disconnect_all();
of any length and contain even non-ASCII characters. However when
it's passed to and used as <varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
- <symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ <symbol>NAMEDATALEN</symbol> characters.
+ Anything other than printable ASCII characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
--
2.27.0
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-28 02:48 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:23 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
@ 2023-09-28 03:54 ` Michael Paquier <[email protected]>
2023-09-28 05:58 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Michael Paquier @ 2023-09-28 03:54 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: Peter Smith <[email protected]>; 'Karl O. Pinc' <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
On Thu, Sep 28, 2023 at 03:23:30AM +0000, Hayato Kuroda (Fujitsu) wrote:
> Hmm, cfbot got angry because it tried to apply both of patches. To avoid it, I repost renamed patch.
> (I'm happy if we can specify the target branch of patches)
I was looking at this thread overall, the three v3 flavors of the doc
changes and v4.
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ <varname>application_name</varname> value.
+ Other characters are replaced with <link
+ linkend="sql-syntax-strings-escape">C-style hexadecimal escapes</link>.
The simplicity of the change in v4 seems like the best approach to me,
so +1 for that (including the mention to "C-style").
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-28 02:48 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:23 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:54 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Michael Paquier <[email protected]>
@ 2023-09-28 05:58 ` Karl O. Pinc <[email protected]>
0 siblings, 0 replies; 22+ messages in thread
From: Karl O. Pinc @ 2023-09-28 05:58 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; Peter Smith <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
On Thu, 28 Sep 2023 12:54:33 +0900
Michael Paquier <[email protected]> wrote:
> I was looking at this thread overall, the three v3 flavors of the doc
> changes and v4.
>
> - <varname>application_name</varname> value. Other characters
> will be
> - replaced with question marks (<literal>?</literal>).
> + <varname>application_name</varname> value.
> + Other characters are replaced with <link
> + linkend="sql-syntax-strings-escape">C-style hexadecimal
> escapes</link>.
>
> 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 <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 12:58 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
2023-09-28 00:30 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-28 02:48 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:23 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
@ 2023-09-28 04:23 ` Peter Smith <[email protected]>
1 sibling, 0 replies; 22+ messages in thread
From: Peter Smith @ 2023-09-28 04:23 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>; Tom Lane <[email protected]>
v4 LGTM.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
@ 2023-09-27 06:48 ` Peter Smith <[email protected]>
2023-09-27 12:59 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Peter Smith @ 2023-09-27 06:48 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>
Hi Kuroda-san.
Here are my review comments for your v3 patch.
TBH, I felt the new text descriptions deviated a bit too much from the
originals. IMO only quite a small tweak was needed, so my suggested
text in the comments below reflects that.
======
Commit message.
1.
missing description
======
src/sgml/config.sgml
2. application_name:
- Only printable ASCII characters may be used in the
- <varname>application_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>).
+ Characters that are not printable ASCII, like <literal>\x03</literal>,
+ are replaced with the <productname>PostgreSQL</productname>
+ <link linkend="sql-syntax-strings-escape">C-style escaped
hexadecimal byte value</link>.
BEFORE
Other characters will be replaced with question marks (<literal>?</literal>).
SUGGESTION
Other characters will be replaced with <link
linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
values</link>.
~~~
3. cluster_name:
- build). Only printable ASCII characters may be used in the
- <varname>cluster_name</varname> value. Other characters will be
- replaced with question marks (<literal>?</literal>). No name is shown
- if this parameter is set to the empty string
<literal>''</literal> (which is
- the default). This parameter can only be set at server start.
+ build).
+ Characters that are not printable ASCII, like <literal>\x03</literal>,
+ are replaced with the <productname>PostgreSQL</productname>
+ <link linkend="sql-syntax-strings-escape">C-style escaped
hexadecimal byte value</link>.
+ No name is shown if this parameter is set to the empty string
+ <literal>''</literal> (which is the default). This parameter can only
+ be set at server start.
<same as previous review comment #2>
======
src/sgml/postgres-fdw.sgml
4.
<para>
<varname>postgres_fdw.application_name</varname> can be any string
- of any length and contain even non-ASCII characters. However when
- it's passed to and used as <varname>application_name</varname>
+ of any length and contain even characters that are not printable ASCII.
+ However when it's passed to and used as
<varname>application_name</varname>
in a foreign server, note that it will be truncated to less than
<symbol>NAMEDATALEN</symbol> characters and anything other than
- printable ASCII characters will be replaced with question
- marks (<literal>?</literal>).
+ printable ASCII characters are replaced with the
<productname>PostgreSQL</productname>
+ <link linkend="sql-syntax-strings-escape">C-style escaped
hexadecimal byte value</link>.
See <xref linkend="guc-application-name"/> for details.
</para>
~
AFAICT the first change wasn't necessary.
~
As for the 2nd change:
BEFORE
... and anything other than printable ASCII characters will be
replaced with question marks (<literal>?</literal>).
SUGGESTION
... and anything other than printable ASCII characters will be
replaced with <link linkend="sql-syntax-strings-escape">C-style
escaped hexadecimal byte values</link>.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PGdocs] fix description for handling pf non-ASCII characters
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` RE: [PGdocs] fix description for handling pf non-ASCII characters Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 06:48 ` Re: [PGdocs] fix description for handling pf non-ASCII characters Peter Smith <[email protected]>
@ 2023-09-27 12:59 ` Hayato Kuroda (Fujitsu) <[email protected]>
0 siblings, 0 replies; 22+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2023-09-27 12:59 UTC (permalink / raw)
To: 'Peter Smith' <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; jian he <[email protected]>; Postgres hackers <[email protected]>
Dear Peter,
Thank you for reviewing!
>
> TBH, I felt the new text descriptions deviated a bit too much from the
> originals. IMO only quite a small tweak was needed, so my suggested
> text in the comments below reflects that.
Good point, my patch may be too much.
> Commit message.
>
> 1.
> missing description
Added.
If we should use only printable ascii as a commit message, I can use '\x03'
instead of 'あああ'.
> src/sgml/config.sgml
>
> 2. application_name:
>
> - Only printable ASCII characters may be used in the
> - <varname>application_name</varname> value. Other characters will
> be
> - replaced with question marks (<literal>?</literal>).
> + Characters that are not printable ASCII, like <literal>\x03</literal>,
> + are replaced with the <productname>PostgreSQL</productname>
> + <link linkend="sql-syntax-strings-escape">C-style escaped
> hexadecimal byte value</link>.
>
> BEFORE
> Other characters will be replaced with question marks (<literal>?</literal>).
>
> SUGGESTION
> Other characters will be replaced with <link
> linkend="sql-syntax-strings-escape">C-style escaped hexadecimal byte
> values</link>.
>
> ~~~
>
> 3. cluster_name:
>
> - build). Only printable ASCII characters may be used in the
> - <varname>cluster_name</varname> value. Other characters will be
> - replaced with question marks (<literal>?</literal>). No name is
> shown
> - if this parameter is set to the empty string
> <literal>''</literal> (which is
> - the default). This parameter can only be set at server start.
> + build).
> + Characters that are not printable ASCII, like <literal>\x03</literal>,
> + are replaced with the <productname>PostgreSQL</productname>
> + <link linkend="sql-syntax-strings-escape">C-style escaped
> hexadecimal byte value</link>.
> + No name is shown if this parameter is set to the empty string
> + <literal>''</literal> (which is the default). This parameter can only
> + be set at server start.
>
> <same as previous review comment #2>
>
> ======
> src/sgml/postgres-fdw.sgml
>
> 4.
> <para>
> <varname>postgres_fdw.application_name</varname> can be any
> string
> - of any length and contain even non-ASCII characters. However when
> - it's passed to and used as <varname>application_name</varname>
> + of any length and contain even characters that are not printable ASCII.
> + However when it's passed to and used as
> <varname>application_name</varname>
> in a foreign server, note that it will be truncated to less than
> <symbol>NAMEDATALEN</symbol> characters and anything other than
> - printable ASCII characters will be replaced with question
> - marks (<literal>?</literal>).
> + printable ASCII characters are replaced with the
> <productname>PostgreSQL</productname>
> + <link linkend="sql-syntax-strings-escape">C-style escaped
> hexadecimal byte value</link>.
> See <xref linkend="guc-application-name"/> for details.
> </para>
>
> ~
>
> AFAICT the first change wasn't necessary.
>
> ~
>
> As for the 2nd change:
>
> BEFORE
> ... and anything other than printable ASCII characters will be
> replaced with question marks (<literal>?</literal>).
>
> SUGGESTION
> ... and anything other than printable ASCII characters will be
> replaced with <link linkend="sql-syntax-strings-escape">C-style
> escaped hexadecimal byte values</link>.
They seem good, but they conflict with Karl's comments.
I made three patches based on comments [1], could you check?
[1]: https://www.postgresql.org/message-id/TYAPR01MB58663EB061888B2715A39217F5C2A%40TYAPR01MB5866.jpnprd0...
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 22+ messages in thread
* [PATCH v8 11/17] table_scan_bitmap_next_block counts lossy and exact pages
@ 2024-03-22 21:09 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 22+ messages in thread
From: Melanie Plageman @ 2024-03-22 21:09 UTC (permalink / raw)
Now that the table_scan_bitmap_next_block() callback only returns false
when the bitmap is exhausted, it is simpler to move the management of
the lossy and exact page counters into it. We will eventually remove
this callback and table_scan_bitmap_next_tuple() will update those
counters when a new block is read in.
---
src/backend/access/heap/heapam_handler.c | 8 ++++++--
src/backend/executor/nodeBitmapHeapscan.c | 9 ++-------
src/include/access/tableam.h | 21 +++++++++++++--------
3 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 2ad785e511..266b34fe6b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- bool *recheck, bool *lossy, BlockNumber *blockno)
+ bool *recheck, BlockNumber *blockno,
+ long *lossy_pages, long *exact_pages)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block;
@@ -2267,7 +2268,10 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
- *lossy = tbmres->ntuples < 0;
+ if (tbmres->ntuples < 0)
+ (*lossy_pages)++;
+ else
+ (*exact_pages)++;
/*
* Return true to indicate that a valid block was found and the bitmap is
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 7e73583fe5..96b55507a3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -69,7 +69,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
{
ExprContext *econtext;
TableScanDesc scan;
- bool lossy;
TIDBitmap *tbm;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -267,14 +266,10 @@ new_page:
BitmapAdjustPrefetchIterator(node);
- if (!table_scan_bitmap_next_block(scan, &node->recheck, &lossy, &node->blockno))
+ if (!table_scan_bitmap_next_block(scan, &node->recheck, &node->blockno,
+ &node->lossy_pages, &node->exact_pages))
break;
- if (lossy)
- node->lossy_pages++;
- else
- node->exact_pages++;
-
/*
* If serial, we can error out if the the prefetch block doesn't stay
* ahead of the current block.
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index a820cc8c99..1d4b79a73f 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -797,8 +797,8 @@ typedef struct TableAmRoutine
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
* make sense to perform tuple visibility checks at this time).
*
- * lossy indicates whether or not the block's representation in the bitmap
- * is lossy or exact.
+ * lossy_pages is incremented if the block's representation in the bitmap
+ * is lossy, otherwise, exact_pages is incremented.
*
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
@@ -815,8 +815,10 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- bool *recheck, bool *lossy,
- BlockNumber *blockno);
+ bool *recheck,
+ BlockNumber *blockno,
+ long *lossy_pages,
+ long *exact_pages);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -2013,15 +2015,17 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
/*
* Prepare to fetch / check / return tuples as part of a bitmap table scan.
* `scan` needs to have been started via table_beginscan_bm(). Returns false if
- * there are no more blocks in the bitmap, true otherwise. lossy is set to true
- * if bitmap is lossy for the selected block and false otherwise.
+ * there are no more blocks in the bitmap, true otherwise. lossy_pages is
+ * incremented if bitmap is lossy for the selected block and exact_pages is
+ * incremented otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- bool *recheck, bool *lossy, BlockNumber *blockno)
+ bool *recheck, BlockNumber *blockno,
+ long *lossy_pages, long *exact_pages)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2032,7 +2036,8 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan, recheck,
- lossy, blockno);
+ blockno, lossy_pages,
+ exact_pages);
}
/*
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch"
^ permalink raw reply [nested|flat] 22+ messages in thread
end of thread, other threads:[~2024-03-22 21:09 UTC | newest]
Thread overview: 22+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-09-26 06:03 Re: [PGdocs] fix description for handling pf non-ASCII characters Karl O. Pinc <[email protected]>
2023-09-26 13:40 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-26 17:45 ` Karl O. Pinc <[email protected]>
2023-09-26 18:01 ` Tom Lane <[email protected]>
2023-09-26 21:50 ` Karl O. Pinc <[email protected]>
2023-09-27 13:00 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 12:58 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-27 13:59 ` Karl O. Pinc <[email protected]>
2023-09-27 23:49 ` Peter Smith <[email protected]>
2023-09-28 00:30 ` Karl O. Pinc <[email protected]>
2023-09-28 01:13 ` Peter Smith <[email protected]>
2023-09-28 01:19 ` Tom Lane <[email protected]>
2023-09-28 01:41 ` Peter Smith <[email protected]>
2023-09-28 02:50 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 02:48 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:23 ` Hayato Kuroda (Fujitsu) <[email protected]>
2023-09-28 03:54 ` Michael Paquier <[email protected]>
2023-09-28 05:58 ` Karl O. Pinc <[email protected]>
2023-09-28 04:23 ` Peter Smith <[email protected]>
2023-09-27 06:48 ` Peter Smith <[email protected]>
2023-09-27 12:59 ` Hayato Kuroda (Fujitsu) <[email protected]>
2024-03-22 21:09 [PATCH v8 11/17] table_scan_bitmap_next_block counts lossy and exact pages Melanie Plageman <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox