Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-05-17 Thread Erik Wienhold
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
>From 39d2dc9b58dfa3b68245070648ecdf9eed892c7a Mon Sep 17 00:00:00 2001
From: Erik Wienhold 
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 numeric_literal, REM
 
  
   Creates a typed table, 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 DROP TYPE ... CASCADE).
+  structure from an existing (name optionally schema-qualified) stand-alone
+  composite type (i.e., created using )
+  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.
  
 
  
-  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 CREATE TABLE 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 CREATE TABLE 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.
  
 

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 

Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-05-17 Thread David G. Johnston
On Fri, May 17, 2024 at 4:57 PM Erik Wienhold  wrote:

> On 2024-05-16 17:47 +0200, David G. Johnston wrote:
> > On Wed, May 15, 2024 at 8:46 AM Robert Haas 
> wrote:
> >
> > > On Thu, Apr 4, 2024 at 12:41 AM Erik Wienhold  wrote:
> > > > Thanks, fixed in v4.  Looks like American English prefers that comma
> and
> > > > it's also more common in our docs.
> > >
> > > Reviewing this patch:
> > >
> > > -  Creates a typed table, 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 DROP TYPE ... CASCADE).
> > > +  Creates a typed table, which takes its
> > > structure
> > > +  from an existing (name optionally schema-qualified) stand-alone
> > > composite
> > > +  type (i.e., created using )
> 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.
> > >
> > > It would be better if this diff didn't reflow the unchanged portions
> > > of the paragraph.
>
> Right.  I now reformatted it so that first line remains unchanged.  But
> the rest of the para is still a complete rewrite.
>
> > > I agree that it's a good idea to mention that the table must have been
> > > created using CREATE TYPE .. AS here, but I disagree with the rest of
> > > the rewording in this hunk. I think we could just add "creating using
> > > CREATE TYPE" to the end of the first sentence, with an xref, and leave
> > > the rest as it is.
> >
> >
> >
> > > I don't see a reason to mention that the typed
> > > table also spawns a rowtype; that's just standard CREATE TABLE
> > > behavior and not really relevant here.
> >
> >
> > I figured it wouldn't be immediately obvious that the system would
> create a
> > second type with identical structure.  Of course, in order for SELECT tbl
> > FROM tbl; to work it must indeed do so.  I'm not married to pointing out
> > this dynamic explicitly though.
> >
> >
> > > And I don't understand what the
> > > rest of the rewording does for us.
> > >
> >
> > It calls out the explicit behavior that the table's columns can change
> due
> > to actions on the underlying type.  Mentioning this unique behavior seems
> > worth a sentence.
> >
> >
> > >   
> > > -  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 CREATE TABLE 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 CREATE TABLE 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.
> > >   
> > >
> > > I don't see how this is better.
> > >
> >
> > I'll agree this is more of a stylistic change, but mainly because the
> talk
> > about data types reasonably implies the other items the patch explicitly
> > mentions - names and additional columns.
>
> I prefer David's changes to both paras because right now the details of
> typed tables are spread over the respective CREATE and ALTER commands
> for types and tables.  Or maybe we should add those details to the
> existing "Typed Tables" section at the very end of CREATE TABLE?
>
> > > - errmsg("type %s is not a composite type",
> > > + errmsg("type %s is not a stand-alone composite type",
> > >
> > > I agree with Peter's complaint that people aren't going to understand
> > > what a stand-alone composite type means when they see the revised
> > > error message; to really help people, we're going to need to do better
> > > than this, I think.
> > >
> > >
> > 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.



> 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 

Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-05-17 Thread Erik Wienhold
On 2024-05-16 17:47 +0200, David G. Johnston wrote:
> On Wed, May 15, 2024 at 8:46 AM Robert Haas  wrote:
> 
> > On Thu, Apr 4, 2024 at 12:41 AM Erik Wienhold  wrote:
> > > Thanks, fixed in v4.  Looks like American English prefers that comma and
> > > it's also more common in our docs.
> >
> > Reviewing this patch:
> >
> > -  Creates a typed table, 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 DROP TYPE ... CASCADE).
> > +  Creates a typed table, which takes its
> > structure
> > +  from an existing (name optionally schema-qualified) stand-alone
> > composite
> > +  type (i.e., created using ) 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.
> >
> > It would be better if this diff didn't reflow the unchanged portions
> > of the paragraph.

Right.  I now reformatted it so that first line remains unchanged.  But
the rest of the para is still a complete rewrite.

> > I agree that it's a good idea to mention that the table must have been
> > created using CREATE TYPE .. AS here, but I disagree with the rest of
> > the rewording in this hunk. I think we could just add "creating using
> > CREATE TYPE" to the end of the first sentence, with an xref, and leave
> > the rest as it is.
> 
> 
> 
> > I don't see a reason to mention that the typed
> > table also spawns a rowtype; that's just standard CREATE TABLE
> > behavior and not really relevant here.
> 
> 
> I figured it wouldn't be immediately obvious that the system would create a
> second type with identical structure.  Of course, in order for SELECT tbl
> FROM tbl; to work it must indeed do so.  I'm not married to pointing out
> this dynamic explicitly though.
> 
> 
> > And I don't understand what the
> > rest of the rewording does for us.
> >
> 
> It calls out the explicit behavior that the table's columns can change due
> to actions on the underlying type.  Mentioning this unique behavior seems
> worth a sentence.
> 
> 
> >   
> > -  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 CREATE TABLE 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 CREATE TABLE 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.
> >   
> >
> > I don't see how this is better.
> >
> 
> I'll agree this is more of a stylistic change, but mainly because the talk
> about data types reasonably implies the other items the patch explicitly
> mentions - names and additional columns.

I prefer David's changes to both paras because right now the details of
typed tables are spread over the respective CREATE and ALTER commands
for types and tables.  Or maybe we should add those details to the
existing "Typed Tables" section at the very end of CREATE TABLE?

> > - errmsg("type %s is not a composite type",
> > + errmsg("type %s is not a stand-alone composite type",
> >
> > I agree with Peter's complaint that people aren't going to understand
> > what a stand-alone composite type means when they see the revised
> > error message; to really help people, we're going to need to do better
> > than this, I think.
> >
> >
> We have a glossary.
> 
> That said, leave the wording as-is and add a conditional hint: The
> composite type must not also be a table.

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.

-- 
Erik
>From 575bfec362bde5bf77ccb793bd8e2d78679c062f Mon Sep 17 00:00:00 2001
From: Erik Wienhold 
Date: Fri, 8 Mar 2024 04:21:56 +0100
Subject: [PATCH v5] 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  | 11 +--
 src/test/regress/expected/typed_table.out |  7 ++-
 src/test/regress/sql/typed_table.sql  |  4 
 4 files 

Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-05-16 Thread David G. Johnston
On Wed, May 15, 2024 at 8:46 AM Robert Haas  wrote:

> On Thu, Apr 4, 2024 at 12:41 AM Erik Wienhold  wrote:
> > Thanks, fixed in v4.  Looks like American English prefers that comma and
> > it's also more common in our docs.
>
> Reviewing this patch:
>
> -  Creates a typed table, 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 DROP TYPE ... CASCADE).
> +  Creates a typed table, which takes its
> structure
> +  from an existing (name optionally schema-qualified) stand-alone
> composite
> +  type (i.e., created using ) 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.
>
> It would be better if this diff didn't reflow the unchanged portions
> of the paragraph.
>
> I agree that it's a good idea to mention that the table must have been
> created using CREATE TYPE .. AS here, but I disagree with the rest of
> the rewording in this hunk. I think we could just add "creating using
> CREATE TYPE" to the end of the first sentence, with an xref, and leave
> the rest as it is.



> I don't see a reason to mention that the typed
> table also spawns a rowtype; that's just standard CREATE TABLE
> behavior and not really relevant here.


I figured it wouldn't be immediately obvious that the system would create a
second type with identical structure.  Of course, in order for SELECT tbl
FROM tbl; to work it must indeed do so.  I'm not married to pointing out
this dynamic explicitly though.


> And I don't understand what the
> rest of the rewording does for us.
>

It calls out the explicit behavior that the table's columns can change due
to actions on the underlying type.  Mentioning this unique behavior seems
worth a sentence.


>   
> -  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 CREATE TABLE 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 CREATE TABLE 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.
>   
>
> I don't see how this is better.
>

I'll agree this is more of a stylistic change, but mainly because the talk
about data types reasonably implies the other items the patch explicitly
mentions - names and additional columns.


> - errmsg("type %s is not a composite type",
> + errmsg("type %s is not a stand-alone composite type",
>
> I agree with Peter's complaint that people aren't going to understand
> what a stand-alone composite type means when they see the revised
> error message; to really help people, we're going to need to do better
> than this, I think.
>
>
We have a glossary.

That said, leave the wording as-is and add a conditional hint: The
composite type must not also be a table.

David J.


Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-05-15 Thread Robert Haas
On Thu, Apr 4, 2024 at 12:41 AM Erik Wienhold  wrote:
> Thanks, fixed in v4.  Looks like American English prefers that comma and
> it's also more common in our docs.

Reviewing this patch:

-  Creates a typed table, 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 DROP TYPE ... CASCADE).
+  Creates a typed table, which takes its structure
+  from an existing (name optionally schema-qualified) stand-alone composite
+  type (i.e., created using ) 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.

It would be better if this diff didn't reflow the unchanged portions
of the paragraph.

I agree that it's a good idea to mention that the table must have been
created using CREATE TYPE .. AS here, but I disagree with the rest of
the rewording in this hunk. I think we could just add "creating using
CREATE TYPE" to the end of the first sentence, with an xref, and leave
the rest as it is. I don't see a reason to mention that the typed
table also spawns a rowtype; that's just standard CREATE TABLE
behavior and not really relevant here. And I don't understand what the
rest of the rewording does for us.

  
-  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 CREATE TABLE 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 CREATE TABLE 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.
  

I don't see how this is better.

- errmsg("type %s is not a composite type",
+ errmsg("type %s is not a stand-alone composite type",

I agree with Peter's complaint that people aren't going to understand
what a stand-alone composite type means when they see the revised
error message; to really help people, we're going to need to do better
than this, I think.

-- 
Robert Haas
EDB: http://www.enterprisedb.com




Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-04-17 Thread Peter Eisentraut

On 29.03.24 02:42, David G. Johnston wrote:
We do use the term "stand-alone composite" in create type so I'm 
inclined to use it instead of "composite created with CREATE TYPE"; 
especially in the error messages; I'm a bit more willing to add the 
cross-reference to create type in the user docs.


I'm not sure this would have helped.  If you see this in the error 
message, then there is no additional guidance what a "stand-alone 
composite type" and a not-"stand-alone composite type" are.


Maybe it's possible to catch the forbidden cases more explicitly and 
come up with more helpful error messages along the lines of "cannot 
create a typed table based on the row type of another table".






Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-04-03 Thread Erik Wienhold
On 2024-04-04 03:29 +0200, David G. Johnston wrote:
> On Thu, Mar 28, 2024 at 8:02 PM Erik Wienhold  wrote:
> 
> > Thanks, that sounds better.  I incorporated that with some minor edits
> > in the attached v3.
> >
> 
> You added my missing ( but dropped the comma after "i.e."

Thanks, fixed in v4.  Looks like American English prefers that comma and
it's also more common in our docs.

-- 
Erik
>From 8b29c5852762bacb637fab021a06b12ab5cd7f93 Mon Sep 17 00:00:00 2001
From: Erik Wienhold 
Date: Fri, 8 Mar 2024 04:21:56 +0100
Subject: [PATCH v4] Document that typed tables rely on CREATE TYPE

CREATE TABLE OF requires a stand-alone composite type.  Clarify that in
the error message.  Also reword the docs to better explain the
connection between created table and stand-alone composite type.

Reworded docs provided by David G. Johnston.
---
 doc/src/sgml/ref/create_table.sgml| 18 +-
 src/backend/commands/tablecmds.c  |  2 +-
 src/test/regress/expected/typed_table.out |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index dfb7822985..586ccb190b 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -249,19 +249,19 @@ WITH ( MODULUS numeric_literal, REM
 OF type_name
 
  
-  Creates a typed table, 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 DROP TYPE ... CASCADE).
+  Creates a typed table, which takes its structure
+  from an existing (name optionally schema-qualified) stand-alone composite
+  type (i.e., created using ) 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.
  
 
  
-  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 CREATE TABLE 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 CREATE TABLE 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.
  
 

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 317b89f67c..d756d2b200 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6994,7 +6994,7 @@ check_of_type(HeapTuple typetuple)
if (!typeOk)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-errmsg("type %s is not a composite type",
+errmsg("type %s is not a stand-alone composite 
type",
format_type_be(typ->oid;
 }
 
diff --git a/src/test/regress/expected/typed_table.out 
b/src/test/regress/expected/typed_table.out
index 2e47ecbcf5..745fbde811 100644
--- a/src/test/regress/expected/typed_table.out
+++ b/src/test/regress/expected/typed_table.out
@@ -89,7 +89,7 @@ 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 is not a stand-alone composite type
 DROP TABLE stuff;
 -- implicit casting
 CREATE TYPE person_type AS (id int, name text);
-- 
2.44.0



Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-04-03 Thread David G. Johnston
On Thu, Mar 28, 2024 at 8:02 PM Erik Wienhold  wrote:

> Thanks, that sounds better.  I incorporated that with some minor edits
> in the attached v3.
>

Looks good.

You added my missing ( but dropped the comma after "i.e."

diff --git a/doc/src/sgml/ref/create_table.sgml
b/doc/src/sgml/ref/create_table.sgml
index dc69a3f5dc..b2e9e97b93 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -251,7 +251,7 @@ WITH ( MODULUS numeric_literal, REM
  
   Creates a typed table, which takes its
structure
   from an existing (name optionally schema-qualified) stand-alone
composite
-  type (i.e. created using ) though it
+  type (i.e., created using ) 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.

David J.


Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-03-28 Thread Erik Wienhold
On 2024-03-29 02:42 +0100, David G. Johnston wrote:
> For consideration for the doc portion.  The existing wording is too
> imprecise for my liking and just tacking on "expects...create type" is
> jarring.
> 
> """
> Creates a typed table, which takes it structure from an existing (name
> optionally schema-qualified) stand-alone composite type i.e., one created
> using CREATE TYPE) though it still produces a new composite type as well.
> The table will have a dependency to the referenced type such cascaded alter
> and drop actions on the type will propagate to the table.
> 
> 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
> CREATE TABLE command can add defaults and constraints to the table, as well
> as specify storage parameters.
> """

Thanks, that sounds better.  I incorporated that with some minor edits
in the attached v3.

> We do use the term "stand-alone composite" in create type so I'm inclined
> to use it instead of "composite created with CREATE TYPE"; especially in
> the error messages; I'm a bit more willing to add the cross-reference to
> create type in the user docs.

Okay, changed in v3 as well.  I used "created with CREATE TYPE" in the
error message because I thought it's clearer to the user.  But I see no
reason for not using "stand-alone" here as well if it's the established
term.

-- 
Erik
>From e9a79e6d5e063098eed4f834b18d576089b38499 Mon Sep 17 00:00:00 2001
From: Erik Wienhold 
Date: Fri, 8 Mar 2024 04:21:56 +0100
Subject: [PATCH v3] Document that typed tables rely on CREATE TYPE

CREATE TABLE OF requires a stand-alone composite type.  Clarify that in
the error message.  Also reword the docs to better explain the
connection between created table and stand-alone composite type.

Reworded docs provided by David G. Johnston.
---
 doc/src/sgml/ref/create_table.sgml| 18 +-
 src/backend/commands/tablecmds.c  |  2 +-
 src/test/regress/expected/typed_table.out |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index dfb7822985..5c8c1edaed 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -249,19 +249,19 @@ WITH ( MODULUS numeric_literal, REM
 OF type_name
 
  
-  Creates a typed table, 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 DROP TYPE ... CASCADE).
+  Creates a typed table, which takes its structure
+  from an existing (name optionally schema-qualified) stand-alone composite
+  type (i.e. created using ) 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.
  
 
  
-  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 CREATE TABLE 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 CREATE TABLE 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.
  
 

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6741e721ae..8e9dbe4bee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6990,7 +6990,7 @@ check_of_type(HeapTuple typetuple)
if (!typeOk)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-errmsg("type %s is not a composite type",
+errmsg("type %s is not a stand-alone composite 
type",
format_type_be(typ->oid;
 }
 
diff --git a/src/test/regress/expected/typed_table.out 
b/src/test/regress/expected/typed_table.out
index 2e47ecbcf5..745fbde811 100644
--- a/src/test/regress/expected/typed_table.out
+++ b/src/test/regress/expected/typed_table.out
@@ -89,7 +89,7 @@ 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 is not a stand-alone composite type
 DROP TABLE stuff;
 -- implicit casting
 CREATE TYPE person_type AS (id int, name text);
-- 
2.44.0



Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-03-28 Thread David G. Johnston
On Thu, Mar 7, 2024 at 9:29 PM Erik Wienhold  wrote:

> I wrote:
> > The attached v2 is a simpler patch that instead modifies the existing
> > error message.
>
> Forgot to attach v2.
>
>
For consideration for the doc portion.  The existing wording is too
imprecise for my liking and just tacking on "expects...create type" is
jarring.

"""
Creates a typed table, which takes it structure from an existing (name
optionally schema-qualified) stand-alone composite type i.e., one created
using CREATE TYPE) though it still produces a new composite type as well.
The table will have a dependency to the referenced type such cascaded alter
and drop actions on the type will propagate to the table.

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
CREATE TABLE command can add defaults and constraints to the table, as well
as specify storage parameters.
"""

We do use the term "stand-alone composite" in create type so I'm inclined
to use it instead of "composite created with CREATE TYPE"; especially in
the error messages; I'm a bit more willing to add the cross-reference to
create type in the user docs.

David J.


Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-03-07 Thread Erik Wienhold
I wrote:
> The attached v2 is a simpler patch that instead modifies the existing
> error message.

Forgot to attach v2.

-- 
Erik
>From 9ab6b625e8f93ae2957144ec7f0ba32cf8a3bb5b Mon Sep 17 00:00:00 2001
From: Erik Wienhold 
Date: Fri, 8 Mar 2024 04:21:56 +0100
Subject: [PATCH v2] Document that typed tables rely on CREATE TYPE

CREATE TABLE OF accepts only composite types that were created with
CREATE TYPE.  Clarify that also in the error message.
---
 doc/src/sgml/ref/create_table.sgml| 2 ++
 src/backend/commands/tablecmds.c  | 2 +-
 src/test/regress/expected/typed_table.out | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index 4cbaaccaf7..889496cf0a 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -254,6 +254,8 @@ WITH ( MODULUS numeric_literal, REM
   schema-qualified).  A typed table is tied to its type; for
   example the table will be dropped if the type is dropped
   (with DROP TYPE ... CASCADE).
+  Expects the composite type to have been created with
+  .
  
 
  
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7014be8039..0f43f349dc 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6979,7 +6979,7 @@ check_of_type(HeapTuple typetuple)
if (!typeOk)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-errmsg("type %s is not a composite type",
+errmsg("type %s is not a composite type 
created with CREATE TYPE",
format_type_be(typ->oid;
 }
 
diff --git a/src/test/regress/expected/typed_table.out 
b/src/test/regress/expected/typed_table.out
index 2e47ecbcf5..9edd744f6a 100644
--- a/src/test/regress/expected/typed_table.out
+++ b/src/test/regress/expected/typed_table.out
@@ -89,7 +89,7 @@ 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 is not a composite type created with CREATE TYPE
 DROP TABLE stuff;
 -- implicit casting
 CREATE TYPE person_type AS (id int, name text);
-- 
2.44.0



Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-03-07 Thread Erik Wienhold
I wrote:
> The attached patch fixes the error message and also documents that
> requirement.

On second thought, adding a separate error message doesn't really make
sense.  The attached v2 is a simpler patch that instead modifies the
existing error message.

-- 
Erik




Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-03-07 Thread Erik Wienhold
On 2024-03-08 01:12 +0100, Hannu Krosing wrote:
> I could not find any explanation of the following behaviour in docs -
> 
> 
> Our documentation for CREATE TABLE says:
> 
> CREATE TABLE also automatically creates a data type that represents
> the composite type corresponding to one row of the table. Therefore,
> tables cannot have the same name as any existing data type in the same
> schema.
>
> But these composite tables are only sometimes there

There's a distinction between stand-alone composite types created with CREATE
TYPE and those created implicitly via CREATE TABLE.  The former is also
called "free-standing" in the docs for pg_type.typrelid[1].

> hannuk=# CREATE TABLE pair(a int, b int);
> CREATE TABLE
> 
> hannuk=# INSERT INTO pair VALUES(1,2);
> INSERT 0 1
> 
> hannuk=# select pg_typeof(p) from pair as p;
>  pg_typeof
> ---
>  pair
> 
> hannuk=# select pg_typeof(pg_typeof(p)) from pair as p;
>  pg_typeof
> ---
>  regtype
> 
> # first case where I can not use the table-defined type
> 
> hannuk=# create table anoter_pair of pair;
> ERROR:  type pair is not a composite type

That error message is simply misleading.  What gets checked here is that
type "pair" was created with CREATE TYPE.  The attached patch fixes the
error message and also documents that requirement.

check_of_type() already addresses this limitation:

/*
 * check_of_type
 *
 * Check whether a type is suitable for CREATE TABLE OF/ALTER TABLE OF. 
 If it
 * isn't suitable, throw an error.  Currently, we require that the type
 * originated with CREATE TYPE AS.  We could support any row type, but 
doing so
 * would require handling a number of extra corner cases in the DDL 
commands.
 * (Also, allowing domain-over-composite would open up a can of worms 
about
 * whether and how the domain's constraints should apply to derived 
tables.)
 */

Not sure what those corner cases are, but table inheritance is one of
them:  I played around with typeOk in check_of_type() to also accept the
composite types implicitly created by CREATE TABLE:

typeOk = (typeRelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE ||
  typeRelation->rd_rel->relkind == RELKIND_RELATION);

With that creating typed tables of parent and child works as expected:

CREATE TABLE parent (a int);
CREATE TABLE child (b int) INHERITS (parent);
CREATE TABLE of_parent OF parent;
CREATE TABLE of_child OF child;
\d parent
   Table "public.parent"
 Column |  Type   | Collation | Nullable | Default 
+-+---+--+-
 a  | integer |   |  | 
Number of child tables: 1 (Use \d+ to list them.)

\d of_parent
 Table "public.of_parent"
 Column |  Type   | Collation | Nullable | Default 
+-+---+--+-
 a  | integer |   |  | 
Typed table of type: parent

\d child
   Table "public.child"
 Column |  Type   | Collation | Nullable | Default 
+-+---+--+-
 a  | integer |   |  | 
 b  | integer |   |  | 
Inherits: parent

\d of_child
  Table "public.of_child"
 Column |  Type   | Collation | Nullable | Default 
+-+---+--+-
 a  | integer |   |  | 
 b  | integer |   |  | 
Typed table of type: child

But adding columns to parent does not change the typed tables:

ALTER TABLE parent ADD c int;
\d parent
   Table "public.parent"
 Column |  Type   | Collation | Nullable | Default 
+-+---+--+-
 a  | integer |   |  | 
 c  | integer |   |  | 
Number of child tables: 1 (Use \d+ to list them.)

\d of_parent
 Table "public.of_parent"
 Column |  Type   | Collation | Nullable | Default 
+-+---+--+-
 a  | integer |   |  | 
Typed table of type: parent

\d child
   Table "public.child"
 Column |  Type   | Collation | Nullable | Default 
+-+---+--+-
 a  | integer |   |  | 
 b  | integer |   |  | 
 c  | integer |   |  | 
Inherits: parent

\d of_child
  Table "public.of_child"
 Column |  Type   | Collation | Nullable | Default 

CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

2024-03-07 Thread Hannu Krosing
I could not find any explanation of the following behaviour in docs -


Our documentation for CREATE TABLE says:

CREATE TABLE also automatically creates a data type that represents
the composite type corresponding to one row of the table. Therefore,
tables cannot have the same name as any existing data type in the same
schema.


But these composite tables are only sometimes there

hannuk=# CREATE TABLE pair(a int, b int);
CREATE TABLE

hannuk=# INSERT INTO pair VALUES(1,2);
INSERT 0 1

hannuk=# select pg_typeof(p) from pair as p;
 pg_typeof
---
 pair

hannuk=# select pg_typeof(pg_typeof(p)) from pair as p;
 pg_typeof
---
 regtype

# first case where I can not use the table-defined type

hannuk=# create table anoter_pair of pair;
ERROR:  type pair is not a composite type

# the type definitely is there as promised

hannuk=# create type pair as (a int, b int);
ERROR:  type "pair" already exists

# and I can create similar type wit other name and use it to create table

hannuk=# create type pair2 as (a int, b int);
CREATE TYPE

hannuk=# create table anoter_pair of pair2;
CREATE TABLE

# and i can even use it in LIKE

hannuk=# CREATE TABLE pair3(like pair2);
CREATE TABLE

# the type is present in pg_type with type 'c' for Composite

hannuk=# select typname, typtype from pg_type where typname = 'pair';
 typname | typtype
-+-
 pair| c
(1 row)

# and I can add comment to the type

hannuk=# COMMENT ON TYPE pair is 'A Shroedingers type';
COMMENT

# but \dT does not show it (second case)

hannuk=# \dT pair
 List of data types
 Schema | Name | Description
+--+-
(0 rows)

---
Hannu