Date: Sat, 18 May 2024 03:56:36 +0200
From: Erik Wienhold <ewie@ewie.name>
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Hannu Krosing <hannuk@google.com>,
	Peter Eisentraut <peter@eisentraut.org>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: CREATE TABLE creates a composite type corresponding to the table
 row, which is and is not there
Message-ID: <1cd8a224-4038-43ba-bb11-a4cd8d015462@ewie.name>
References: <nb4cfr26yhqfdirpkuwqxiaalfgloceuhfnmyor7w2pwgud5wl@c3mb7yd6uz4q>
 <itvvmotar2z565azdxbtsc7ajpymer6xmqc7tzifzw4zuxnxo6@puia7azj3x5c>
 <CAKFQuwYcV+AZMbyzHyDdxFhp_GH5Nty_GL1U5JkPYJeya0MLYQ@mail.gmail.com>
 <73jtqglviqor5ozqrthjmau7nj4xi7z4obzaqflf7cvliaf4em@4obr7wqlqkye>
 <CAKFQuwYCqsapMBvQm9hTpD7urTaaOtJbpkpe7GaXr5ksMT58rA@mail.gmail.com>
 <efxy4vnyrwhowyyv7vk6s7ps6bwkyizdmaehs7mppjzapssvxl@n5nuyelnyfcj>
 <CA+TgmoZ4cUNV8BbMz6anMTsbcKkCQSs+V_ZWdCd5=2SB9pe67Q@mail.gmail.com>
 <CAKFQuwZQ2+YAerqxo4zJ0mreowBqe9T0k0KPvAaqwgvFNGOR5A@mail.gmail.com>
 <e905f37d-7535-4df7-b171-ce1e569f4b40@ewie.name>
 <CAKFQuwZkVT0RDoYzt-v=-+330xaEaVo=vqLOqezuLZZ5X5-3oA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="p7whpo2b3ruhdelr"
Content-Disposition: inline
In-Reply-To: 
 <CAKFQuwZkVT0RDoYzt-v=-+330xaEaVo=vqLOqezuLZZ5X5-3oA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/1cd8a224-4038-43ba-bb11-a4cd8d015462%40ewie.name>
Precedence: bulk


--p7whpo2b3ruhdelr
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On 2024-05-18 03:27 +0200, David G. Johnston wrote:
> > On 2024-05-16 17:47 +0200, David G. Johnston wrote:
> > > We have a glossary.
> 
> If sticking with stand-alone composite type as a formal term we should
> document it in the glossary.  It's unclear whether this will survive review
> though.  With the wording provided in this patch it doesn't really add
> enough to continue a strong defense of it.

Oh, I thought you meant we already have that term in the glossary (I
haven't checked until now).  Let's see if we can convince Robert of the
rewording.

> > It's now a separate error message (like I already had in v1) which
> > states that the specified type must not be a row type of another table
> > (based on Peter's feedback).  And the hint directs the user to CREATE
> > TYPE.
> >
> > In passing, I also quoted the type name in the existing error message
> > for consistency.  I saw that table names etc. are already quoted in
> > other error messages.
> >
> >
> The hint and the quoting both violate the documented rules for these things:
> 
> https://www.postgresql.org/docs/current/error-style-guide.html#ERROR-STYLE-GUIDE-QUOTES
> 
> There are functions in the backend that will double-quote their own output
> as needed (for example, format_type_be()). Do not put additional quotes
> around the output of such functions.
> 
> https://www.postgresql.org/docs/current/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION
> 
> Detail and hint messages: Use complete sentences, and end each with a
> period. Capitalize the first word of sentences.

Thanks, I didn't know that guideline.  Both fixed in v6.

-- 
Erik

--p7whpo2b3ruhdelr
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v6-0001-Document-that-typed-tables-rely-on-CREATE-TYPE.patch"

From 39d2dc9b58dfa3b68245070648ecdf9eed892c7a Mon Sep 17 00:00:00 2001
From: Erik Wienhold <ewie@ewie.name>
Date: Fri, 8 Mar 2024 04:21:56 +0100
Subject: [PATCH v6] Document that typed tables rely on CREATE TYPE

CREATE TABLE OF requires a stand-alone composite type that is not the
row type of another table.  Clarify that with a reference to CREATE TYPE
in the docs.  Also report a separate error message in this case.

Reworded docs provided by David G. Johnston.
---
 doc/src/sgml/ref/create_table.sgml        | 16 ++++++++--------
 src/backend/commands/tablecmds.c          |  9 ++++++++-
 src/test/regress/expected/typed_table.out |  7 ++++++-
 src/test/regress/sql/typed_table.sql      |  4 ++++
 4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index f19306e776..11458be9cf 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -249,18 +249,18 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     <listitem>
      <para>
       Creates a <firstterm>typed table</firstterm>, which takes its
-      structure from the specified composite type (name optionally
-      schema-qualified).  A typed table is tied to its type; for
-      example the table will be dropped if the type is dropped
-      (with <literal>DROP TYPE ... CASCADE</literal>).
+      structure from an existing (name optionally schema-qualified) stand-alone
+      composite type (i.e., created using <xref linkend="sql-createtype"/>)
+      though it still produces a new composite type as well.  The table will
+      have a dependency on the referenced type such that cascaded alter and
+      drop actions on the type will propagate to the table.
      </para>
 
      <para>
-      When a typed table is created, then the data types of the
-      columns are determined by the underlying composite type and are
-      not specified by the <literal>CREATE TABLE</literal> command.
+      A typed table always has the same column names and data types as the
+      type it is derived from, and you cannot specify additional columns.
       But the <literal>CREATE TABLE</literal> command can add defaults
-      and constraints to the table and can specify storage parameters.
+      and constraints to the table, as well as specify storage parameters.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 313c782cae..2331a9185a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6935,8 +6935,15 @@ check_of_type(HeapTuple typetuple)
 		 * the type before the typed table creation/conversion commits.
 		 */
 		relation_close(typeRelation, NoLock);
+
+		if (!typeOk)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg("type %s must not be a row type of another table",
+							format_type_be(typ->oid)),
+					 errhint("Must be a stand-alone composite type created with CREATE TYPE.")));
 	}
-	if (!typeOk)
+	else
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("type %s is not a composite type",
diff --git a/src/test/regress/expected/typed_table.out b/src/test/regress/expected/typed_table.out
index 2e47ecbcf5..5743e74978 100644
--- a/src/test/regress/expected/typed_table.out
+++ b/src/test/regress/expected/typed_table.out
@@ -89,8 +89,13 @@ drop cascades to function get_all_persons()
 drop cascades to table persons2
 drop cascades to table persons3
 CREATE TABLE persons5 OF stuff; -- only CREATE TYPE AS types may be used
-ERROR:  type stuff is not a composite type
+ERROR:  type stuff must not be a row type of another table
+HINT:  Must be a stand-alone composite type created with CREATE TYPE.
 DROP TABLE stuff;
+CREATE TYPE simple AS ENUM ('a');
+CREATE TABLE of_simple OF simple; -- not a composite type
+ERROR:  type simple is not a composite type
+DROP TYPE simple;
 -- implicit casting
 CREATE TYPE person_type AS (id int, name text);
 CREATE TABLE persons OF person_type;
diff --git a/src/test/regress/sql/typed_table.sql b/src/test/regress/sql/typed_table.sql
index 9ef0cdfcc7..d9b9398857 100644
--- a/src/test/regress/sql/typed_table.sql
+++ b/src/test/regress/sql/typed_table.sql
@@ -50,6 +50,10 @@ CREATE TABLE persons5 OF stuff; -- only CREATE TYPE AS types may be used
 
 DROP TABLE stuff;
 
+CREATE TYPE simple AS ENUM ('a');
+CREATE TABLE of_simple OF simple; -- not a composite type
+DROP TYPE simple;
+
 
 -- implicit casting
 
-- 
2.45.1


--p7whpo2b3ruhdelr--