extended stats objects are the only thing written like "%s"."%s"

Started by Justin Pryzbyover 4 years ago5 messages

pryzby@telsasoft.com

over 4 years ago

commit a4d75c86bf15220df22de0a92c819ecef9db3849
Author: Tomas Vondra <tomas.vondra@postgresql.org>
Date: Fri Mar 26 23:22:01 2021 +0100

Extended statistics on expressions

This commit added to psql/describe.c:

+                                       /* statistics object name (qualified with namespace) */
+                                       appendPQExpBuffer(&buf, "\"%s\".\"%s\"",
+                                                                         PQgetvalue(result, i, 2),
+                                                                         PQgetvalue(result, i, 3));

Everywhere else the double quotes are around the whole "schema.object" rather
than both separately: "schema"."object". The code handling servers before v14
has the same thing, since:

commit bc085205c8a425fcaa54e27c6dcd83101130439b
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri May 12 14:59:23 2017 -0300

Change CREATE STATISTICS syntax

src/bin/psql/describe.c- /* statistics object name (qualified with namespace) */
src/bin/psql/describe.c: appendPQExpBuffer(&buf, "\"%s\".\"%s\" (",
src/bin/psql/describe.c- PQgetvalue(result, i, 2),
src/bin/psql/describe.c- PQgetvalue(result, i, 3));

That seems to have been first added in the patch here, but AFAIT not
specifically discussed.
/messages/by-id/20170511221330.5akgbsoyx6wm4u34@alvherre.pgsql

At the time the patch was commited, it was the only place that used
"schema"."object":
$ git show bc085205c8a425fcaa54e27c6dcd83101130439b:src/bin/psql/describe.c |grep '\\"\.\\"'
appendPQExpBuffer(&buf, "\"%s\".\"%s\" (",

And it's still the only place, not just in describe.c, but the entire project.
$ git grep -Fc '\"%s\".\"%s\"' '*.c'
src/bin/psql/describe.c:2

I actually don't like writing it as "a.b" since it doesn't work to copy+paste
that, because that means an object called "a.b" in the default schema.
But I think for consistency it should be done the same here as everywhere else.

I noticed that Peter E recently changed amcheck in the direction of consistency:
| 4279e5bc8c pg_amcheck: Message style and structuring improvements

I propose to change extended stats objects to be shown the same as everywhere
else, with double quotes around the whole %s.%s:
$ git grep '\\"%s\.%s\\"' '*.c' |wc -l
126

This affects 9 lines of output in regression tests.

Note that check constraints and indexes have the same schema as their table, so
\d doesn't show a schema at all, and quotes the name of the object. That
distinction may be relevant to how stats objects ended up being quoted like
this.

--
Justin

Alvaro Herrera

alvherre@alvh.no-ip.org

over 4 years ago

In reply to: Justin Pryzby (#1)

Re: extended stats objects are the only thing written like "%s"."%s"

On 2021-Aug-28, Justin Pryzby wrote:

Note that check constraints and indexes have the same schema as their table, so
\d doesn't show a schema at all, and quotes the name of the object. That
distinction may be relevant to how stats objects ended up being quoted like
this.

Yeah, this was the rationale for including the schema name here.

I think using "%s.%s" as is done everywhere else is pretty much
pointless. It's not usable as an object identifier, since you have to
make sure to remove the existing quotes, and unless the names work
without quotes, you have to add different quotes. So it looks «nice»
but it's functionally more work.

--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
"But static content is just dynamic content that isn't moving!"
http://smylers.hates-software.com/2007/08/15/fe244d0c.html

Tom Lane

tgl@sss.pgh.pa.us

over 4 years ago

In reply to: Alvaro Herrera (#2)

Re: extended stats objects are the only thing written like "%s"."%s"

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

I think using "%s.%s" as is done everywhere else is pretty much
pointless. It's not usable as an object identifier, since you have to
make sure to remove the existing quotes, and unless the names work
without quotes, you have to add different quotes. So it looks «nice»
but it's functionally more work.

I think what we are doing there is following the message style
guideline that says to put double quotes around inserted strings.
In this case schema.object (as a whole) is the inserted string.
People often confuse this with SQL double-quoted identifiers, but it
has nothing whatsoever to do with SQL's rules. (It's easier to make
sense of this rule in translations where the quote marks are not
ASCII double-quotes ... like your example with «nice».)

In short: Justin is right, this should not be done this way.

regards, tom lane

Alvaro Herrera

alvherre@alvh.no-ip.org

over 4 years ago

In reply to: Tom Lane (#3)

Re: extended stats objects are the only thing written like "%s"."%s"

On 2021-Aug-28, Tom Lane wrote:

I think what we are doing there is following the message style
guideline that says to put double quotes around inserted strings.
In this case schema.object (as a whole) is the inserted string.
People often confuse this with SQL double-quoted identifiers, but it
has nothing whatsoever to do with SQL's rules. (It's easier to make
sense of this rule in translations where the quote marks are not
ASCII double-quotes ... like your example with «nice».)

In short: Justin is right, this should not be done this way.

I don't agree with the way we're applying the message guidelines here,
but since this is the only place where we do this, I've changed it to
the idiomatic way of quoting names.

I only backpatched to 14 in order to avoid messing with established
output format in released branches, but if people really hate the extra
quotes with a passion I'm not opposed to backpatching further.

--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"World domination is proceeding according to plan" (Andrew Morton)

Peter Eisentraut

peter.eisentraut@enterprisedb.com

over 4 years ago

In reply to: Alvaro Herrera (#4)

Re: extended stats objects are the only thing written like "%s"."%s"

On 30.08.21 20:06, Alvaro Herrera wrote:

On 2021-Aug-28, Tom Lane wrote:

I think what we are doing there is following the message style
guideline that says to put double quotes around inserted strings.
In this case schema.object (as a whole) is the inserted string.
People often confuse this with SQL double-quoted identifiers, but it
has nothing whatsoever to do with SQL's rules. (It's easier to make
sense of this rule in translations where the quote marks are not
ASCII double-quotes ... like your example with «nice».)

In short: Justin is right, this should not be done this way.

I don't agree with the way we're applying the message guidelines here,
but since this is the only place where we do this, I've changed it to
the idiomatic way of quoting names.

I agree that the current situation is not satisfactory. We should think
about extending the guidelines to cover this.

Note that it's not necessarily enough to say, leave \"%s\".\"%s\"
untranslated. For example, this could create inconsistencies with
analogous messages that don't include a schema qualification. Also,
unless we are being careful about escaping double-quoted strings inside
the substituted strings, it wouldn't be entirely correct either.

A comprehensive approach across the tree would be preferable, perhaps
with additional APIs to support it. Also, the question when schema
qualifications should be printed or not should be answered.