Thread: Quoting of psql \d output
psql \d always double-quotes table names:
Table "public.xx"
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
"ii" btree (y)
With this patch, double-quotes are not used when not required:
test=> \d xx
Table public.xx
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
ii btree (y)
but does in this case:
test=> \d "xx y"
Table public."xx y"
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
vv btree (y)
This patch uses pg_dump fmtId() to double-quote only when necessary.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: src/bin/psql/Makefile
===================================================================
RCS file: /cvsroot/pgsql-server/src/bin/psql/Makefile,v
retrieving revision 1.38
diff -c -c -r1.38 Makefile
*** src/bin/psql/Makefile 29 Nov 2003 19:52:06 -0000 1.38
--- src/bin/psql/Makefile 23 Dec 2003 04:52:20 -0000
***************
*** 15,25 ****
REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref
! override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) -DFRONTEND
OBJS= command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
startup.o prompt.o variables.o large_obj.o print.o describe.o \
! tab-complete.o mbprint.o
all: submake-libpq submake-libpgport psql
--- 15,26 ----
REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref
! override CPPFLAGS := -I$(top_srcdir)/src/bin/pg_dump -I$(libpq_srcdir) $(CPPFLAGS) -DFRONTEND
OBJS= command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
startup.o prompt.o variables.o large_obj.o print.o describe.o \
! tab-complete.o mbprint.o \
! dumputils.o $(top_builddir)/src/backend/parser/keywords.o
all: submake-libpq submake-libpgport psql
***************
*** 27,32 ****
--- 28,36 ----
$(CC) $(CFLAGS) $(OBJS) $(libpq) $(LDFLAGS) $(LIBS) -o $@
help.o: $(srcdir)/sql_help.h
+
+ dumputils.c: % : $(top_srcdir)/src/bin/pg_dump/%
+ rm -f $@ && $(LN_S) $< .
ifdef PERL
$(srcdir)/sql_help.h: create_help.pl $(wildcard $(REFDOCDIR)/*.sgml)
Index: src/bin/psql/describe.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/bin/psql/describe.c,v
retrieving revision 1.90
diff -c -c -r1.90 describe.c
*** src/bin/psql/describe.c 1 Dec 2003 22:21:54 -0000 1.90
--- src/bin/psql/describe.c 23 Dec 2003 04:52:21 -0000
***************
*** 16,21 ****
--- 16,23 ----
#include "print.h"
#include "variables.h"
+ #include "dumputils.h"
+
#include <ctype.h>
#ifdef WIN32
***************
*** 658,663 ****
--- 660,667 ----
PQExpBufferData tmpbuf;
int cols = 0;
int numrows = 0;
+ char schema_rel_str[NAMEDATALEN * 2 + 6];
+
struct
{
bool hasindex;
***************
*** 812,851 ****
#endif
}
/* Make title */
switch (tableinfo.relkind)
{
case 'r':
! printfPQExpBuffer(&title, _("Table \"%s.%s\""),
! schemaname, relationname);
break;
case 'v':
! printfPQExpBuffer(&title, _("View \"%s.%s\""),
! schemaname, relationname);
break;
case 'S':
! printfPQExpBuffer(&title, _("Sequence \"%s.%s\""),
! schemaname, relationname);
break;
case 'i':
! printfPQExpBuffer(&title, _("Index \"%s.%s\""),
! schemaname, relationname);
break;
case 's':
! printfPQExpBuffer(&title, _("Special relation \"%s.%s\""),
! schemaname, relationname);
break;
case 't':
! printfPQExpBuffer(&title, _("TOAST table \"%s.%s\""),
! schemaname, relationname);
break;
case 'c':
! printfPQExpBuffer(&title, _("Composite type \"%s.%s\""),
! schemaname, relationname);
break;
default:
! printfPQExpBuffer(&title, _("?%c? \"%s.%s\""),
! tableinfo.relkind, schemaname, relationname);
break;
}
--- 816,852 ----
#endif
}
+
+ strcpy(schema_rel_str, fmtId(schemaname));
+ strcat(schema_rel_str, ".");
+ strcat(schema_rel_str, fmtId(relationname));
+
/* Make title */
switch (tableinfo.relkind)
{
case 'r':
! printfPQExpBuffer(&title, _("Table %s"), schema_rel_str);
break;
case 'v':
! printfPQExpBuffer(&title, _("View %s"), schema_rel_str);
break;
case 'S':
! printfPQExpBuffer(&title, _("Sequence %s"), schema_rel_str);
break;
case 'i':
! printfPQExpBuffer(&title, _("Index %s"), schema_rel_str);
break;
case 's':
! printfPQExpBuffer(&title, _("Special relation %s"), schema_rel_str);
break;
case 't':
! printfPQExpBuffer(&title, _("TOAST table %s"), schema_rel_str);
break;
case 'c':
! printfPQExpBuffer(&title, _("Composite type %s"), schema_rel_str);
break;
default:
! printfPQExpBuffer(&title, _("?%c? %s"), tableinfo.relkind, schema_rel_str);
break;
}
***************
*** 887,895 ****
resetPQExpBuffer(&tmpbuf);
appendPQExpBuffer(&tmpbuf, "%s, ", indamname);
/* we assume here that index and table are in same schema */
! appendPQExpBuffer(&tmpbuf, _("for table \"%s.%s\""),
! schemaname, indtable);
if (strlen(indpred))
appendPQExpBuffer(&tmpbuf, ", predicate (%s)", indpred);
--- 888,899 ----
resetPQExpBuffer(&tmpbuf);
appendPQExpBuffer(&tmpbuf, "%s, ", indamname);
+ strcpy(schema_rel_str, fmtId(schemaname));
+ strcat(schema_rel_str, ".");
+ strcat(schema_rel_str, fmtId(indtable));
+
/* we assume here that index and table are in same schema */
! appendPQExpBuffer(&tmpbuf, _("for table %s"), schema_rel_str);
if (strlen(indpred))
appendPQExpBuffer(&tmpbuf, ", predicate (%s)", indpred);
***************
*** 1095,1102 ****
const char *usingpos;
/* Output index name */
! printfPQExpBuffer(&buf, _(" \"%s\""),
! PQgetvalue(result1, i, 0));
/* Label as primary key or unique (but not both) */
appendPQExpBuffer(&buf,
--- 1099,1106 ----
const char *usingpos;
/* Output index name */
! printfPQExpBuffer(&buf, _(" %s"),
! fmtId(PQgetvalue(result1, i, 0)));
/* Label as primary key or unique (but not both) */
appendPQExpBuffer(&buf,
***************
*** 1125,1132 ****
footers[count_footers++] = xstrdup(buf.data);
for (i = 0; i < check_count; i++)
{
! printfPQExpBuffer(&buf, _(" \"%s\" %s"),
! PQgetvalue(result2, i, 1),
PQgetvalue(result2, i, 0));
footers[count_footers++] = xstrdup(buf.data);
--- 1129,1136 ----
footers[count_footers++] = xstrdup(buf.data);
for (i = 0; i < check_count; i++)
{
! printfPQExpBuffer(&buf, _(" %s %s"),
! fmtId(PQgetvalue(result2, i, 1)),
PQgetvalue(result2, i, 0));
footers[count_footers++] = xstrdup(buf.data);
***************
*** 1140,1147 ****
footers[count_footers++] = xstrdup(buf.data);
for (i = 0; i < foreignkey_count; i++)
{
! printfPQExpBuffer(&buf, _(" \"%s\" %s"),
! PQgetvalue(result5, i, 0),
PQgetvalue(result5, i, 1));
footers[count_footers++] = xstrdup(buf.data);
--- 1144,1151 ----
footers[count_footers++] = xstrdup(buf.data);
for (i = 0; i < foreignkey_count; i++)
{
! printfPQExpBuffer(&buf, _(" %s %s"),
! fmtId(PQgetvalue(result5, i, 0)),
PQgetvalue(result5, i, 1));
footers[count_footers++] = xstrdup(buf.data);
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> psql \d always double-quotes table names:
> Table "public.xx"
Yeah, that has bugged me for a while, because it is in fact *wrong* ...
the above representation of a qualified name is incorrect.
> This patch uses pg_dump fmtId() to double-quote only when necessary.
Seems good, but I think your Makefile patch is a brick or two shy of
a load. Don't you need something to make sure that parser/keywords.o
is up to date?
More generally, maybe we ought to think about moving fmtId() and perhaps
other parts of dumputils out of pg_dump and into some
more-easily-accessible library.
regards, tom lane
Bruce Momjian writes: > psql \d always double-quotes table names: Yes, and that is conforming to message style rules. Please leave it at that.
Peter Eisentraut wrote:
> Bruce Momjian writes:
> > psql \d always double-quotes table names:
>
> Yes, and that is conforming to message style rules. Please leave it at that.
But it isn't a message:
Table "public.xx"
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
"ii" btree (y)
There are places where the object name is used in a sentence, and I have
left those alone.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > psql \d always double-quotes table names: > > Table "public.xx" > > Yeah, that has bugged me for a while, because it is in fact *wrong* ... > the above representation of a qualified name is incorrect. > > This patch uses pg_dump fmtId() to double-quote only when necessary. > > Seems good, but I think your Makefile patch is a brick or two shy of > a load. Don't you need something to make sure that parser/keywords.o > is up to date? OK, got it. I didn't notice the submake-backend rule before. Attached. > More generally, maybe we ought to think about moving fmtId() and perhaps > other parts of dumputils out of pg_dump and into some > more-easily-accessible library. I will return to that. Right now we have been putting code shared by the backend and /bin into pgport, but I don't think that is going to be clean for things shared by just /bin, and we have the keywords.c dependency, making this even more complex. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: src/bin/psql/Makefile =================================================================== RCS file: /cvsroot/pgsql-server/src/bin/psql/Makefile,v retrieving revision 1.38 diff -c -c -r1.38 Makefile *** src/bin/psql/Makefile 29 Nov 2003 19:52:06 -0000 1.38 --- src/bin/psql/Makefile 23 Dec 2003 14:55:38 -0000 *************** *** 15,33 **** REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref ! override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) -DFRONTEND OBJS= command.o common.o help.o input.o stringutils.o mainloop.o copy.o \ startup.o prompt.o variables.o large_obj.o print.o describe.o \ ! tab-complete.o mbprint.o ! all: submake-libpq submake-libpgport psql psql: $(OBJS) $(libpq_builddir)/libpq.a ! $(CC) $(CFLAGS) $(OBJS) $(libpq) $(LDFLAGS) $(LIBS) -o $@ help.o: $(srcdir)/sql_help.h ifdef PERL $(srcdir)/sql_help.h: create_help.pl $(wildcard $(REFDOCDIR)/*.sgml) $(PERL) $< $(REFDOCDIR) $@ --- 15,43 ---- REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref ! override CPPFLAGS := -I$(top_srcdir)/src/bin/pg_dump -I$(libpq_srcdir) $(CPPFLAGS) -DFRONTEND OBJS= command.o common.o help.o input.o stringutils.o mainloop.o copy.o \ startup.o prompt.o variables.o large_obj.o print.o describe.o \ ! tab-complete.o mbprint.o \ ! dumputils.o ! EXTRA_OBJS = $(top_builddir)/src/backend/parser/keywords.o ! ! all: submake-libpq submake-libpgport submake-backend psql psql: $(OBJS) $(libpq_builddir)/libpq.a ! $(CC) $(CFLAGS) $(OBJS) $(EXTRA_OBJS) $(libpq) $(LDFLAGS) $(LIBS) -o $@ help.o: $(srcdir)/sql_help.h + dumputils.c: % : $(top_srcdir)/src/bin/pg_dump/% + rm -f $@ && $(LN_S) $< . + + .PHONY: submake-backend + submake-backend: + $(MAKE) -C $(top_builddir)/src/backend/parser keywords.o + ifdef PERL $(srcdir)/sql_help.h: create_help.pl $(wildcard $(REFDOCDIR)/*.sgml) $(PERL) $< $(REFDOCDIR) $@ *************** *** 35,40 **** --- 45,51 ---- $(srcdir)/sql_help.h: @echo "*** Perl is needed to build psql help." endif + distprep: $(srcdir)/sql_help.h Index: src/bin/psql/describe.c =================================================================== RCS file: /cvsroot/pgsql-server/src/bin/psql/describe.c,v retrieving revision 1.90 diff -c -c -r1.90 describe.c *** src/bin/psql/describe.c 1 Dec 2003 22:21:54 -0000 1.90 --- src/bin/psql/describe.c 23 Dec 2003 14:55:39 -0000 *************** *** 16,21 **** --- 16,23 ---- #include "print.h" #include "variables.h" + #include "dumputils.h" + #include <ctype.h> #ifdef WIN32 *************** *** 658,663 **** --- 660,667 ---- PQExpBufferData tmpbuf; int cols = 0; int numrows = 0; + char schema_rel_str[NAMEDATALEN * 2 + 6]; + struct { bool hasindex; *************** *** 812,851 **** #endif } /* Make title */ switch (tableinfo.relkind) { case 'r': ! printfPQExpBuffer(&title, _("Table \"%s.%s\""), ! schemaname, relationname); break; case 'v': ! printfPQExpBuffer(&title, _("View \"%s.%s\""), ! schemaname, relationname); break; case 'S': ! printfPQExpBuffer(&title, _("Sequence \"%s.%s\""), ! schemaname, relationname); break; case 'i': ! printfPQExpBuffer(&title, _("Index \"%s.%s\""), ! schemaname, relationname); break; case 's': ! printfPQExpBuffer(&title, _("Special relation \"%s.%s\""), ! schemaname, relationname); break; case 't': ! printfPQExpBuffer(&title, _("TOAST table \"%s.%s\""), ! schemaname, relationname); break; case 'c': ! printfPQExpBuffer(&title, _("Composite type \"%s.%s\""), ! schemaname, relationname); break; default: ! printfPQExpBuffer(&title, _("?%c? \"%s.%s\""), ! tableinfo.relkind, schemaname, relationname); break; } --- 816,852 ---- #endif } + + strcpy(schema_rel_str, fmtId(schemaname)); + strcat(schema_rel_str, "."); + strcat(schema_rel_str, fmtId(relationname)); + /* Make title */ switch (tableinfo.relkind) { case 'r': ! printfPQExpBuffer(&title, _("Table %s"), schema_rel_str); break; case 'v': ! printfPQExpBuffer(&title, _("View %s"), schema_rel_str); break; case 'S': ! printfPQExpBuffer(&title, _("Sequence %s"), schema_rel_str); break; case 'i': ! printfPQExpBuffer(&title, _("Index %s"), schema_rel_str); break; case 's': ! printfPQExpBuffer(&title, _("Special relation %s"), schema_rel_str); break; case 't': ! printfPQExpBuffer(&title, _("TOAST table %s"), schema_rel_str); break; case 'c': ! printfPQExpBuffer(&title, _("Composite type %s"), schema_rel_str); break; default: ! printfPQExpBuffer(&title, _("?%c? %s"), tableinfo.relkind, schema_rel_str); break; } *************** *** 887,895 **** resetPQExpBuffer(&tmpbuf); appendPQExpBuffer(&tmpbuf, "%s, ", indamname); /* we assume here that index and table are in same schema */ ! appendPQExpBuffer(&tmpbuf, _("for table \"%s.%s\""), ! schemaname, indtable); if (strlen(indpred)) appendPQExpBuffer(&tmpbuf, ", predicate (%s)", indpred); --- 888,899 ---- resetPQExpBuffer(&tmpbuf); appendPQExpBuffer(&tmpbuf, "%s, ", indamname); + strcpy(schema_rel_str, fmtId(schemaname)); + strcat(schema_rel_str, "."); + strcat(schema_rel_str, fmtId(indtable)); + /* we assume here that index and table are in same schema */ ! appendPQExpBuffer(&tmpbuf, _("for table %s"), schema_rel_str); if (strlen(indpred)) appendPQExpBuffer(&tmpbuf, ", predicate (%s)", indpred); *************** *** 1095,1102 **** const char *usingpos; /* Output index name */ ! printfPQExpBuffer(&buf, _(" \"%s\""), ! PQgetvalue(result1, i, 0)); /* Label as primary key or unique (but not both) */ appendPQExpBuffer(&buf, --- 1099,1106 ---- const char *usingpos; /* Output index name */ ! printfPQExpBuffer(&buf, _(" %s"), ! fmtId(PQgetvalue(result1, i, 0))); /* Label as primary key or unique (but not both) */ appendPQExpBuffer(&buf, *************** *** 1125,1132 **** footers[count_footers++] = xstrdup(buf.data); for (i = 0; i < check_count; i++) { ! printfPQExpBuffer(&buf, _(" \"%s\" %s"), ! PQgetvalue(result2, i, 1), PQgetvalue(result2, i, 0)); footers[count_footers++] = xstrdup(buf.data); --- 1129,1136 ---- footers[count_footers++] = xstrdup(buf.data); for (i = 0; i < check_count; i++) { ! printfPQExpBuffer(&buf, _(" %s %s"), ! fmtId(PQgetvalue(result2, i, 1)), PQgetvalue(result2, i, 0)); footers[count_footers++] = xstrdup(buf.data); *************** *** 1140,1147 **** footers[count_footers++] = xstrdup(buf.data); for (i = 0; i < foreignkey_count; i++) { ! printfPQExpBuffer(&buf, _(" \"%s\" %s"), ! PQgetvalue(result5, i, 0), PQgetvalue(result5, i, 1)); footers[count_footers++] = xstrdup(buf.data); --- 1144,1151 ---- footers[count_footers++] = xstrdup(buf.data); for (i = 0; i < foreignkey_count; i++) { ! printfPQExpBuffer(&buf, _(" %s %s"), ! fmtId(PQgetvalue(result5, i, 0)), PQgetvalue(result5, i, 1)); footers[count_footers++] = xstrdup(buf.data);
"Peter Eisentraut" <peter_e@gmx.net> writes:
> Bruce Momjian writes:
>> psql \d always double-quotes table names:
> Yes, and that is conforming to message style rules. Please leave it at that.
The problem is that it is double-quoting a qualified name, which is
something we generally don't produce in messages. I think it's actively
misleading.
A simpler but uglier solution would be to quote the schema and table
names separately:
Table "public"."foo"
regards, tom lane
Hey Bruce, While you're at it - use the same sort of code to conditionally quote index, rule and constraint names ... Chris Bruce Momjian wrote: > psql \d always double-quotes table names: > > Table "public.xx" > Column | Type | Modifiers > --------+---------+----------- > y | integer | > Indexes: > "ii" btree (y) > > > With this patch, double-quotes are not used when not required: > > test=> \d xx > Table public.xx > Column | Type | Modifiers > --------+---------+----------- > y | integer | > Indexes: > ii btree (y) > > > but does in this case: > > test=> \d "xx y" > Table public."xx y" > Column | Type | Modifiers > --------+---------+----------- > y | integer | > Indexes: > vv btree (y) > > This patch uses pg_dump fmtId() to double-quote only when necessary.
Christopher Kings-Lynne wrote: > Hey Bruce, > > While you're at it - use the same sort of code to conditionally quote > index, rule and constraint names ... Those are in the patch. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/psql-ref.sgml =================================================================== RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.101 diff -c -c -r1.101 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 1 Dec 2003 22:21:54 -0000 1.101 --- doc/src/sgml/ref/psql-ref.sgml 23 Dec 2003 04:56:21 -0000 *************** *** 957,962 **** --- 957,963 ---- Lists all available schemas (namespaces). If <replaceable class="parameter">pattern</replaceable> (a regular expression) is specified, only schemas whose names match the pattern are listed. + Non-local temporary schemas are suppressed. </para> </listitem> </varlistentry> Index: src/bin/psql/describe.c =================================================================== RCS file: /cvsroot/pgsql-server/src/bin/psql/describe.c,v retrieving revision 1.90 diff -c -c -r1.90 describe.c *** src/bin/psql/describe.c 1 Dec 2003 22:21:54 -0000 1.90 --- src/bin/psql/describe.c 23 Dec 2003 04:56:22 -0000 *************** *** 1626,1639 **** initPQExpBuffer(&buf); printfPQExpBuffer(&buf, ! "SELECT n.nspname AS \"%s\",\n" ! " u.usename AS \"%s\"\n" "FROM pg_catalog.pg_namespace n LEFT JOIN pg_catalog.pg_user u\n" ! " ON n.nspowner=u.usesysid\n", _("Name"), _("Owner")); ! ! processNamePattern(&buf, pattern, false, false, NULL, "n.nspname", NULL, NULL); --- 1626,1640 ---- initPQExpBuffer(&buf); printfPQExpBuffer(&buf, ! "SELECT n.nspname AS \"%s\",\n" ! " u.usename AS \"%s\"\n" "FROM pg_catalog.pg_namespace n LEFT JOIN pg_catalog.pg_user u\n" ! " ON n.nspowner=u.usesysid\n" ! "WHERE (n.nspname NOT LIKE 'pg\\\\_temp\\\\_%%' OR\n" ! " n.nspname = (pg_catalog.current_schemas(true))[1])\n", /* temp schema is first */ _("Name"), _("Owner")); ! processNamePattern(&buf, pattern, true, false, NULL, "n.nspname", NULL, NULL);
Bruce Momjian writes: > But it isn't a message: > > Table "public.xx" > Column | Type | Modifiers > --------+---------+----------- > y | integer | > Indexes: > "ii" btree (y) By definition, everything that is printed out for human consumption is a message. Everything that is printed out for consumption by an SQL parser (e.g., in pg_dump) is encouraged to use SQL quoting rules. But everything that is intended to be read by humans should use quoting rules that are common in real-life publishing.
Tom Lane writes: > The problem is that it is double-quoting a qualified name, which is > something we generally don't produce in messages. I think it's actively > misleading. While I don't really agree that this is a problem, better solutions would be to not quote the thing at all or write something like Table "%s" (Schema "%s"). However, the message style guidelines are very clear on this point, and before we deviate I'd like to see an adjusted proposal.
Christopher Kings-Lynne writes: > While you're at it - use the same sort of code to conditionally quote > index, rule and constraint names ... That makes you part of the problem, because these sections are impossible to translate correctly.
"Peter Eisentraut" <peter_e@gmx.net> writes:
> While I don't really agree that this is a problem, better solutions would
> be to not quote the thing at all or write something like Table "%s"
> (Schema "%s"). However, the message style guidelines are very clear on
> this point, and before we deviate I'd like to see an adjusted proposal.
As Bruce pointed out, this isn't really a message; it's just a column
heading, and so it's not clear that the message style guidelines apply.
I kinda like the idea of not quoting it at all, actually, since that is
a far simpler solution.
regards, tom lane
Tom Lane wrote:
> "Peter Eisentraut" <peter_e@gmx.net> writes:
> > While I don't really agree that this is a problem, better solutions would
> > be to not quote the thing at all or write something like Table "%s"
> > (Schema "%s"). However, the message style guidelines are very clear on
> > this point, and before we deviate I'd like to see an adjusted proposal.
>
> As Bruce pointed out, this isn't really a message; it's just a column
> heading, and so it's not clear that the message style guidelines apply.
>
> I kinda like the idea of not quoting it at all, actually, since that is
> a far simpler solution.
I looked into doing no quotes at all. Look at this. I have a table "xx y":
test=> \d
List of relations
Schema | Name | Type | Owner
--------+------+-------+----------
public | test | table | postgres
public | xx | table | postgres
public | xx y | table | postgres
(3 rows)
test=> \d xx y
Table public.xx <-- wrong table
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
ii btree (y)
\d: extra argument "y" ignored <-- fails without quotes
test=> \d "xx y"
Table public."xx y"
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
vv btree (y)
test=> create table "Xa" (y int);
CREATE TABLE
test=> \d Xa
Did not find any relation named "Xa". <-- again, requires quotes
test=> \d "Xa"
Table public."Xa"
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Because the backslash commands require quotes to access specific tables,
and because queries will require quotes, I thought displaying quotes
when appropriate was a good idea and a good reminder to users. I also
thought that spaces in names could lead to confusing displays where the
spaces don't clearly identify where the object name begins and ends in
the display --- something Peter was concerned about.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Peter Eisentraut wrote:
> Bruce Momjian writes:
> > But it isn't a message:
> >
> > Table "public.xx"
> > Column | Type | Modifiers
> > --------+---------+-----------
> > y | integer |
> > Indexes:
> > "ii" btree (y)
>
> By definition, everything that is printed out for human consumption is a
> message.
>
> Everything that is printed out for consumption by an SQL parser (e.g., in
> pg_dump) is encouraged to use SQL quoting rules. But everything that is
I see pg_dump using the same rules that I am proposing:
CREATE TABLE "xx y" (
y integer
) WITH OIDS;
CREATE TABLE xx (
y integer
) WITH OIDS;
[ We need that WITH OIDS fixed before 7.5. I think Neil is working on it.]
> intended to be read by humans should use quoting rules that are common in
> real-life publishing.
To me quoting the table name is like quoting the name of a section
heading (which people don't do), and that's really what we are
displaying:
test=> \d xx
Table public.xx
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
vv btree (y)
"Table public.xx" is the section heading, and before you complain that I
quoted it in this sentence, I had to because it is part of a sentence,
not on its own as it is in psql \d display.
You were suggesting no quotes at all. Would this display be OK?
test=> \d "xx y"
Table public.xx y
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
vv btree (y)
Note I had to quote the table name to get \d to accept it. To me that
'y' just sitting along looks strange.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
>>While you're at it - use the same sort of code to conditionally quote >>index, rule and constraint names ... > > > That makes you part of the problem, because these sections are impossible > to translate correctly. Now you've lost me - how is a user-inputted object name translatable? Chris
Christopher Kings-Lynne writes: >>>While you're at it - use the same sort of code to conditionally quote >>> index, rule and constraint names ... >> >> >> That makes you part of the problem, because these sections are >> impossible to translate correctly. > > Now you've lost me - how is a user-inputted object name translatable? Not all languages use "..." as quote symbols, but if you make them part of some string that comes from the backend, it becomes prohibitively hard to translate it correctly.
Tom Lane writes: > As Bruce pointed out, this isn't really a message; it's just a column > heading, and so it's not clear that the message style guidelines apply. How is this different? Just because text is in a box or formatted in a particular way, it doesn't change the nature of the content.
Bruce Momjian writes: > I see pg_dump using the same rules that I am proposing: Which makes my point. > To me quoting the table name is like quoting the name of a section > heading (which people don't do), The message style guidelines specify that you should always quote a %s if it might contain characters that would cause confusion about where the plugged-in value starts and ends, and that you quote every %s the same way, because when the user reads the value, the user just wants to recognize the value and does not care about whether the value might in some context be an SQL identifier or who knows what. When you write section titles, you know the titles ahead of time, so you don't need to guard against confusion. > You were suggesting no quotes at all. Would this display be OK? > > test=> \d "xx y" > Table public.xx y No, that doesn't work. Either we write it in proper SQL terms, that is public."xx y" or we write it in human terms, that is Table "xx y" (Schema "public") (or some variation). What I'm trying to say is, since we're addressing humans, we should use the human terms.
"Peter Eisentraut" <peter_e@gmx.net> writes:
> Christopher Kings-Lynne writes:
>> Now you've lost me - how is a user-inputted object name translatable?
> Not all languages use "..." as quote symbols, but if you make them part of
> some string that comes from the backend, it becomes prohibitively hard to
> translate it correctly.
Hm. This gets back to the point we've discussed before: there is some
confusion between SQL's use of quoted identifiers and the customary
English use of quote marks to set off text that should be distinguished
from the surrounding sentence.
Essentially, Bruce's proposed patch moves the use of quotes in \d table
headers to conform to SQL's "technical" use of quotes, while you are
arguing for sticking with the message style guidelines' "human-oriented"
use of quotes.
I can see merit in both positions. But I also see merit in the
compromise position of not using quotes at all. I don't see a strong
need to demarcate table name from surrounding text in a context as
simple as this --- is
Table "foo"
really any easier to read than
Table foo
? And Peter is correct that the former introduces translation issues
when you think about languages that don't customarily use "..." as
quotation marks.
regards, tom lane
Tom Lane wrote:
> "Peter Eisentraut" <peter_e@gmx.net> writes:
> > Christopher Kings-Lynne writes:
> >> Now you've lost me - how is a user-inputted object name translatable?
>
> > Not all languages use "..." as quote symbols, but if you make them part of
> > some string that comes from the backend, it becomes prohibitively hard to
> > translate it correctly.
>
> Hm. This gets back to the point we've discussed before: there is some
> confusion between SQL's use of quoted identifiers and the customary
> English use of quote marks to set off text that should be distinguished
> from the surrounding sentence.
True. In the "Table classname.tablename" case, there is no requirement
for quotes in English, but there is for technical reasons. Imagine:
Table My class.My table
Looks kind of strange without quotes:
Table "My class"."My table"
Still looks strange, but better. :-)
> Essentially, Bruce's proposed patch moves the use of quotes in \d table
> headers to conform to SQL's "technical" use of quotes, while you are
> arguing for sticking with the message style guidelines' "human-oriented"
> use of quotes.
>
> I can see merit in both positions. But I also see merit in the
> compromise position of not using quotes at all. I don't see a strong
> need to demarcate table name from surrounding text in a context as
> simple as this --- is
> Table "foo"
> really any easier to read than
> Table foo
> ? And Peter is correct that the former introduces translation issues
> when you think about languages that don't customarily use "..." as
> quotation marks.
As I saw, Peter was suggesting to always use quotes to match the message
guidelines:
Table "myclass"."mytable"
or
Table "mytable" (Class "myclass")
However, I have not seen anyone prefer that except Peter so that will
not be adopted unless Peter can find more people that agree with him.
So it seems we can either go with no quotes, or smart quotes (which my
patch implemented). I feel my patch does the best of both worlds, by
quoting as needed, and as the psql \d commands actually require anyway,
and as used by pg_dump and in SQL queries.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> So it seems we can either go with no quotes, or smart quotes (which my
> patch implemented). I feel my patch does the best of both worlds, by
> quoting as needed, and as the psql \d commands actually require anyway,
> and as used by pg_dump and in SQL queries.
You have not responded to Peter's point that quotes need translation.
If we adopt the viewpoint that this is an SQL representation of the
table name, and not a human-oriented one, then double quotes are correct
regardless of the language in use. However, we agreed during
development of the message style guidelines that we would *not* use
strict SQL quoting in messages. I have not seen a good reason given
to ignore that general policy in this particular case.
I think if we change it here we will also have to revisit hundreds of
places in the backend, such as this one:
regression=# select * from public.bar;
ERROR: relation "public.bar" does not exist
and indeed the whole question of what we are using quotes for in
messages becomes open again.
At this point I think I'm voting with Peter, for no change at all.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > So it seems we can either go with no quotes, or smart quotes (which my
> > patch implemented). I feel my patch does the best of both worlds, by
> > quoting as needed, and as the psql \d commands actually require anyway,
> > and as used by pg_dump and in SQL queries.
>
> You have not responded to Peter's point that quotes need translation.
>
> If we adopt the viewpoint that this is an SQL representation of the
> table name, and not a human-oriented one, then double quotes are correct
> regardless of the language in use. However, we agreed during
> development of the message style guidelines that we would *not* use
> strict SQL quoting in messages. I have not seen a good reason given
> to ignore that general policy in this particular case.
>
> I think if we change it here we will also have to revisit hundreds of
> places in the backend, such as this one:
> regression=# select * from public.bar;
> ERROR: relation "public.bar" does not exist
> and indeed the whole question of what we are using quotes for in
> messages becomes open again.
>
> At this point I think I'm voting with Peter, for no change at all.
The issue for me is that in an ereport() are putting the relation name
inside a sentence/message, which psql is not doing. Right now it is
doing:
Table "xx.yy"
That has the worse of all worlds --- quotes, always, and invalid (not
"xx"."yy"). I suppose the only thing good about it is that translators
can change the quotes to something else, but to me the quotes don't make
much sense at all in this context, and I assume others complained about
this looking ugly, which is why I tried to fix it in the first place.
Also consider that users have to use quotes to see a table that needs
quotes with \d. They can't use their local quoting rules anyway.
This is the current output:
test=> \d xx
Table "public.xx"
Column | Type | Modifiers
--------+---------+-----------
y | integer |
Indexes:
"ii" btree (y)
Notice the class.tablename are quoted, and the index name.
I am starting to like the idea of putting the class name after the table
name. The class name is usually insignificant:
Table xx (Class yy)
I do see French changing the quotes in their .po file, while other
languages leave them the same. Perhaps we should just drop the quotes
entirely.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > I think if we change it here we will also have to revisit hundreds of > places in the backend, such as this one: > regression=# select * from public.bar; > ERROR: relation "public.bar" does not exist > and indeed the whole question of what we are using quotes for in > messages becomes open again. I remember someone once posted Oracle's message style guidelines, and they actually specify that you are not supposed to write 'foo.bar' in messages, but you are supposed to write 'schema foo, table bar' or some permutation. Personally, I like this rule, but it seems prohibitively hard and/or cumbersome to implement it everywhere in an i18n-safe way. But we might want to use it when it's easily possible. The alternative is using the dotted notation, and in that case we should use SQL quotation rules because that is the only way to be internally consistent. (Smart quotation or full quotation is another matter.) In that case the generated string falls under the "already supplies its own quotes" rule and the outer format string should not put the %s in quotes again. (Yes, that means that psql should be changed somehow.)