create table like: ACCESS METHOD

Started by Justin Pryzbyabout 5 years ago14 messages

pryzby@telsasoft.com

about 5 years ago

1 attachment(s)

I thought this was a good idea, but didn't hear back when I raised it before.

Failing to preserve access method is arguably a bug, reminiscent of CREATE
STATISTICS and 5564c1181. But maybe it's not important to backpatch a fix in
this case, since access methods are still evolving.

/messages/by-id/20190818193533.GL11185@telsasoft.com
On Sun, Aug 18, 2019 at 02:35:33PM -0500, Justin Pryzby wrote:

. What do you think about pg_restore --no-tableam; similar to
--no-tablespaces, it would allow restoring a table to a different AM:
PGOPTIONS='-c default_table_access_method=zedstore' pg_restore --no-tableam ./pg_dump.dat -d postgres
Otherwise, the dump says "SET default_table_access_method=heap", which
overrides any value from PGOPTIONS and precludes restoring to new AM.

...

Show quoted text

. it'd be nice if there was an ALTER TABLE SET ACCESS METHOD, to allow
migrating data. Otherwise I think the alternative is:
begin; lock t;
CREATE TABLE new_t LIKE (t INCLUDING ALL) USING (zedstore);
INSERT INTO new_t SELECT * FROM t;
for index; do CREATE INDEX...; done
DROP t; RENAME new_t (and all its indices). attach/inherit, etc.
commit;

. Speaking of which, I think LIKE needs a new option for ACCESS METHOD, which
is otherwise lost.

Attachments:

v1-0001-create-table-like-.-including-ACCESS-METHOD.patchtext/x-diff; charset=us-asciiDownload

From 3df7de7f3f2b15c447534bcd7e05c5be79030404 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 15 Nov 2020 16:54:53 -0600
Subject: [PATCH v1] create table (like .. including ACCESS METHOD)

---
 doc/src/sgml/ref/create_table.sgml              | 12 +++++++++++-
 src/backend/parser/gram.y                       |  3 ++-
 src/backend/parser/parse_utilcmd.c              |  7 +++++++
 src/include/nodes/parsenodes.h                  | 17 +++++++++--------
 src/test/regress/expected/create_table_like.out |  2 +-
 src/test/regress/sql/create_table_like.sql      |  2 +-
 6 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 569f4c9da7..cb95177e92 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,7 +87,7 @@ class="parameter">referential_action</replaceable> ] [ ON UPDATE <replaceable cl
 
 <phrase>and <replaceable class="parameter">like_option</replaceable> is:</phrase>
 
-{ INCLUDING | EXCLUDING } { COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
+{ INCLUDING | EXCLUDING } { ACCESS METHOD | COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
 
 <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>
 
@@ -593,6 +593,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       available options are:
 
       <variablelist>
+       <varlistentry>
+        <term><literal>INCLUDING ACCESS METHOD</literal></term>
+        <listitem>
+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
+        </listitem>
+       </varlistentry>
+
        <varlistentry>
         <term><literal>INCLUDING COMMENTS</literal></term>
         <listitem>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 8f341ac006..b32861a04e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3651,7 +3651,8 @@ TableLikeOptionList:
 		;
 
 TableLikeOption:
-				COMMENTS			{ $$ = CREATE_TABLE_LIKE_COMMENTS; }
+				ACCESS METHOD			{ $$ = CREATE_TABLE_LIKE_ACCESSMETHOD; }
+				| COMMENTS			{ $$ = CREATE_TABLE_LIKE_COMMENTS; }
 				| CONSTRAINTS		{ $$ = CREATE_TABLE_LIKE_CONSTRAINTS; }
 				| DEFAULTS			{ $$ = CREATE_TABLE_LIKE_DEFAULTS; }
 				| IDENTITY_P		{ $$ = CREATE_TABLE_LIKE_IDENTITY; }
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 89ee990599..3507fd4738 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -96,6 +96,7 @@ typedef struct
 	bool		ispartitioned;	/* true if table is partitioned */
 	PartitionBoundSpec *partbound;	/* transformed FOR VALUES */
 	bool		ofType;			/* true if statement contains OF typename */
+	char		*accessMethod;	/* table access method */
 } CreateStmtContext;
 
 /* State shared by transformCreateSchemaStmt and its subroutines */
@@ -252,6 +253,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	cxt.ispartitioned = stmt->partspec != NULL;
 	cxt.partbound = stmt->partbound;
 	cxt.ofType = (stmt->ofTypename != NULL);
+	cxt.accessMethod = NULL;
 
 	Assert(!stmt->ofTypename || !stmt->inhRelations);	/* grammar enforces */
 
@@ -345,6 +347,8 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	 */
 	stmt->tableElts = cxt.columns;
 	stmt->constraints = cxt.ckconstraints;
+	if (cxt.accessMethod != NULL)
+		stmt->accessMethod = cxt.accessMethod;
 
 	result = lappend(cxt.blist, stmt);
 	result = list_concat(result, cxt.alist);
@@ -1118,6 +1122,9 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
 	}
 
+	if (table_like_clause->options & CREATE_TABLE_LIKE_ACCESSMETHOD)
+		cxt->accessMethod = get_am_name(relation->rd_rel->relam);
+
 	/*
 	 * We may copy extended statistics if requested, since the representation
 	 * of CreateStatsStmt doesn't depend on column numbers.
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 48a79a7657..589fd0cab0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -677,14 +677,15 @@ typedef struct TableLikeClause
 
 typedef enum TableLikeOption
 {
-	CREATE_TABLE_LIKE_COMMENTS = 1 << 0,
-	CREATE_TABLE_LIKE_CONSTRAINTS = 1 << 1,
-	CREATE_TABLE_LIKE_DEFAULTS = 1 << 2,
-	CREATE_TABLE_LIKE_GENERATED = 1 << 3,
-	CREATE_TABLE_LIKE_IDENTITY = 1 << 4,
-	CREATE_TABLE_LIKE_INDEXES = 1 << 5,
-	CREATE_TABLE_LIKE_STATISTICS = 1 << 6,
-	CREATE_TABLE_LIKE_STORAGE = 1 << 7,
+	CREATE_TABLE_LIKE_ACCESSMETHOD = 1 << 0,
+	CREATE_TABLE_LIKE_COMMENTS = 1 << 1,
+	CREATE_TABLE_LIKE_CONSTRAINTS = 1 << 2,
+	CREATE_TABLE_LIKE_DEFAULTS = 1 << 3,
+	CREATE_TABLE_LIKE_GENERATED = 1 << 4,
+	CREATE_TABLE_LIKE_IDENTITY = 1 << 5,
+	CREATE_TABLE_LIKE_INDEXES = 1 << 6,
+	CREATE_TABLE_LIKE_STATISTICS = 1 << 7,
+	CREATE_TABLE_LIKE_STORAGE = 1 << 8,
 	CREATE_TABLE_LIKE_ALL = PG_INT32_MAX
 } TableLikeOption;
 
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 10d17be23c..cac3c57c76 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -490,7 +490,7 @@ Check constraints:
     "noinh_con_copy_a_check" CHECK (a > 0) NO INHERIT
 
 -- fail, as partitioned tables don't allow NO INHERIT constraints
-CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
+CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL EXCLUDING ACCESS METHOD)
   PARTITION BY LIST (a);
 ERROR:  cannot add NO INHERIT constraint to partitioned table "noinh_con_copy1_parted"
 DROP TABLE noinh_con_copy, noinh_con_copy1;
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 06b76f949d..eda9f39078 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -189,7 +189,7 @@ CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS);
 \d noinh_con_copy1
 
 -- fail, as partitioned tables don't allow NO INHERIT constraints
-CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
+CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL EXCLUDING ACCESS METHOD)
   PARTITION BY LIST (a);
 
 DROP TABLE noinh_con_copy, noinh_con_copy1;
-- 
2.17.0

Michael Paquier

michael@paquier.xyz

about 5 years ago

In reply to: Justin Pryzby (#1)

Re: create table like: ACCESS METHOD

On Wed, Dec 09, 2020 at 02:13:29PM -0600, Justin Pryzby wrote:

I thought this was a good idea, but didn't hear back when I raised it before.

Failing to preserve access method is arguably a bug, reminiscent of CREATE
STATISTICS and 5564c1181. But maybe it's not important to backpatch a fix in
this case, since access methods are still evolving.

Interesting. Access methods for tables are released for more than one
year now, so my take about a backpatch is that this boat has already
sailed. This may give a reason to actually not introduce this
feature.

+ CREATE_TABLE_LIKE_ACCESSMETHOD = 1 << 0,
Nit: wouldn't this be better as ACCESS_METHOD?

 -- fail, as partitioned tables don't allow NO INHERIT constraints
-CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
+CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL EXCLUDING ACCESS METHOD)
   PARTITION BY LIST (a);
This diff means that you are introducing an incompatible change by
forcing any application using CREATE TABLE LIKE for a partitioned
table to exclude access methods.  This is not acceptable, and it may
be better to just ignore this clause instead in this context.

This patch should have more tests. Something could be added in
create_am.sql where there is a fake heap2 as table AM.

+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
Second sentence sounds a bit strange by combining "the" and a GUC
name.  I would just write "Default is default_table_a_m".
--
Michael

Justin Pryzby

pryzby@telsasoft.com

about 5 years ago

In reply to: Michael Paquier (#2)

1 attachment(s)

Re: create table like: ACCESS METHOD

On Fri, Dec 25, 2020 at 03:41:46PM +0900, Michael Paquier wrote:

On Wed, Dec 09, 2020 at 02:13:29PM -0600, Justin Pryzby wrote:

I thought this was a good idea, but didn't hear back when I raised it before.

Failing to preserve access method is arguably a bug, reminiscent of CREATE
STATISTICS and 5564c1181. But maybe it's not important to backpatch a fix in
this case, since access methods are still evolving.

Interesting. Access methods for tables are released for more than one
year now, so my take about a backpatch is that this boat has already
sailed. This may give a reason to actually not introduce this
feature.

Are you saying that since v12/13 didn't preserve the access method, it might be
preferred to never do it ? I think it's reasonable to to not change v12/13 but
the behavior seems like an omission going forward. It's not so important right
now, since AMs aren't widely used.

This might be important for a few cases I can think of easily:

If an readonly AM doesn't support DDL, and table needs to be rebuilt, we'd
handle that by creating a new table LIKE the existing table, preserving its AM,
and then INSERT into it. Like for column type promotion. That's much better
than querying amname FROM pg_class JOIN relam.

ALTER TABLE..ATTACH PARTITION requires a less strong lock than CREATE
TABLE..PARTITION OF, so it's nice to be able to CREATE TABLE LIKE.

To use an alternate AM for historic data, we'd CREATE TABLE LIKE an existing,
populated table before inserting into it. This would support re-creating on a
new AM, or re-creating on the same AM, say, to get rid of dropped columns, or
to re-arrange columns.

-- fail, as partitioned tables don't allow NO INHERIT constraints
-CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
+CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL EXCLUDING ACCESS METHOD)
PARTITION BY LIST (a);
This diff means that you are introducing an incompatible change by
forcing any application using CREATE TABLE LIKE for a partitioned
table to exclude access methods.  This is not acceptable, and it may
be better to just ignore this clause instead in this context.

Ok. This means that
CREATE TABLE (LIKE x INCLUDING ACCESS METHOD) PARTITION BY ...
silently ignores the INCLUDING AM. Is that ok ? I think the alternative is
for INCLUDING to be "ternary" options, defaulting to UNSET=0, when it's ok to
ignore options in contexts where they're not useful.
Maybe we'd need to specially handle INCLUDING ALL, to make options
"soft"/implied rather than explicit.

This patch should have more tests. Something could be added in
create_am.sql where there is a fake heap2 as table AM.

Yes, I had already done that locally.

--
Justin

Attachments:

v2-0001-create-table-like-.-including-ACCESS-METHOD.patchtext/x-diff; charset=us-asciiDownload

From c4c4570f90623aa8508a2749f5ca5e2780ab10df Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 15 Nov 2020 16:54:53 -0600
Subject: [PATCH v2] create table (like .. including ACCESS METHOD)

---
 doc/src/sgml/ref/create_table.sgml              | 12 +++++++++++-
 src/backend/parser/gram.y                       |  3 ++-
 src/backend/parser/parse_utilcmd.c              | 10 ++++++++++
 src/include/nodes/parsenodes.h                  | 17 +++++++++--------
 src/test/regress/expected/create_table_like.out | 12 ++++++++++++
 src/test/regress/sql/create_table_like.sql      |  8 ++++++++
 6 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 569f4c9da7..cb95177e92 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,7 +87,7 @@ class="parameter">referential_action</replaceable> ] [ ON UPDATE <replaceable cl
 
 <phrase>and <replaceable class="parameter">like_option</replaceable> is:</phrase>
 
-{ INCLUDING | EXCLUDING } { COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
+{ INCLUDING | EXCLUDING } { ACCESS METHOD | COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
 
 <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>
 
@@ -593,6 +593,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       available options are:
 
       <variablelist>
+       <varlistentry>
+        <term><literal>INCLUDING ACCESS METHOD</literal></term>
+        <listitem>
+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
+        </listitem>
+       </varlistentry>
+
        <varlistentry>
         <term><literal>INCLUDING COMMENTS</literal></term>
         <listitem>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 8f341ac006..fee0aceb65 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3651,7 +3651,8 @@ TableLikeOptionList:
 		;
 
 TableLikeOption:
-				COMMENTS			{ $$ = CREATE_TABLE_LIKE_COMMENTS; }
+				ACCESS METHOD			{ $$ = CREATE_TABLE_LIKE_ACCESS_METHOD; }
+				| COMMENTS			{ $$ = CREATE_TABLE_LIKE_COMMENTS; }
 				| CONSTRAINTS		{ $$ = CREATE_TABLE_LIKE_CONSTRAINTS; }
 				| DEFAULTS			{ $$ = CREATE_TABLE_LIKE_DEFAULTS; }
 				| IDENTITY_P		{ $$ = CREATE_TABLE_LIKE_IDENTITY; }
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 89ee990599..4eed7e3ea2 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -96,6 +96,7 @@ typedef struct
 	bool		ispartitioned;	/* true if table is partitioned */
 	PartitionBoundSpec *partbound;	/* transformed FOR VALUES */
 	bool		ofType;			/* true if statement contains OF typename */
+	char		*accessMethod;	/* table access method */
 } CreateStmtContext;
 
 /* State shared by transformCreateSchemaStmt and its subroutines */
@@ -252,6 +253,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	cxt.ispartitioned = stmt->partspec != NULL;
 	cxt.partbound = stmt->partbound;
 	cxt.ofType = (stmt->ofTypename != NULL);
+	cxt.accessMethod = NULL;
 
 	Assert(!stmt->ofTypename || !stmt->inhRelations);	/* grammar enforces */
 
@@ -346,6 +348,9 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	stmt->tableElts = cxt.columns;
 	stmt->constraints = cxt.ckconstraints;
 
+	if (cxt.accessMethod != NULL)
+		stmt->accessMethod = cxt.accessMethod;
+
 	result = lappend(cxt.blist, stmt);
 	result = list_concat(result, cxt.alist);
 	result = list_concat(result, save_alist);
@@ -1118,6 +1123,11 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
 	}
 
+	/* CREATE TABLE LIKE ACCESS METHOD is not copied for partitioned table */
+	if ((table_like_clause->options & CREATE_TABLE_LIKE_ACCESS_METHOD) != 0 &&
+		!cxt->ispartitioned)
+		cxt->accessMethod = get_am_name(relation->rd_rel->relam);
+
 	/*
 	 * We may copy extended statistics if requested, since the representation
 	 * of CreateStatsStmt doesn't depend on column numbers.
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 48a79a7657..2d5dfe487e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -677,14 +677,15 @@ typedef struct TableLikeClause
 
 typedef enum TableLikeOption
 {
-	CREATE_TABLE_LIKE_COMMENTS = 1 << 0,
-	CREATE_TABLE_LIKE_CONSTRAINTS = 1 << 1,
-	CREATE_TABLE_LIKE_DEFAULTS = 1 << 2,
-	CREATE_TABLE_LIKE_GENERATED = 1 << 3,
-	CREATE_TABLE_LIKE_IDENTITY = 1 << 4,
-	CREATE_TABLE_LIKE_INDEXES = 1 << 5,
-	CREATE_TABLE_LIKE_STATISTICS = 1 << 6,
-	CREATE_TABLE_LIKE_STORAGE = 1 << 7,
+	CREATE_TABLE_LIKE_ACCESS_METHOD = 1 << 0,
+	CREATE_TABLE_LIKE_COMMENTS = 1 << 1,
+	CREATE_TABLE_LIKE_CONSTRAINTS = 1 << 2,
+	CREATE_TABLE_LIKE_DEFAULTS = 1 << 3,
+	CREATE_TABLE_LIKE_GENERATED = 1 << 4,
+	CREATE_TABLE_LIKE_IDENTITY = 1 << 5,
+	CREATE_TABLE_LIKE_INDEXES = 1 << 6,
+	CREATE_TABLE_LIKE_STATISTICS = 1 << 7,
+	CREATE_TABLE_LIKE_STORAGE = 1 << 8,
 	CREATE_TABLE_LIKE_ALL = PG_INT32_MAX
 } TableLikeOption;
 
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 10d17be23c..a09cd48ed0 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -429,6 +429,18 @@ SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s
  ctlt_all_a_b_stat |        0 | ab stats
 (1 row)
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+ amname  
+---------
+ heapdup
+(1 row)
+
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 NOTICE:  merging multiple inherited definitions of column "a"
 ERROR:  inherited column "a" has a storage parameter conflict
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 06b76f949d..ee6c464851 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -165,6 +165,14 @@ CREATE TABLE ctlt_all (LIKE ctlt1 INCLUDING ALL);
 SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 'ctlt_all'::regclass ORDER BY c.relname, objsubid;
 SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s WHERE classoid = 'pg_statistic_ext'::regclass AND objoid = s.oid AND s.stxrelid = 'ctlt_all'::regclass ORDER BY s.stxname, objsubid;
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
+
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 CREATE TABLE inh_error2 (LIKE ctlt4 INCLUDING STORAGE) INHERITS (ctlt1);
 
-- 
2.17.0

Simon Riggs

simon@2ndquadrant.com

about 5 years ago

In reply to: Justin Pryzby (#3)

Re: create table like: ACCESS METHOD

On Tue, 29 Dec 2020 at 23:08, Justin Pryzby <pryzby@telsasoft.com> wrote:

On Fri, Dec 25, 2020 at 03:41:46PM +0900, Michael Paquier wrote:

On Wed, Dec 09, 2020 at 02:13:29PM -0600, Justin Pryzby wrote:

I thought this was a good idea, but didn't hear back when I raised it before.

Failing to preserve access method is arguably a bug, reminiscent of CREATE
STATISTICS and 5564c1181. But maybe it's not important to backpatch a fix in
this case, since access methods are still evolving.

Interesting. Access methods for tables are released for more than one
year now, so my take about a backpatch is that this boat has already
sailed. This may give a reason to actually not introduce this
feature.

Are you saying that since v12/13 didn't preserve the access method, it might be
preferred to never do it ? I think it's reasonable to not change v12/13 but
the behavior seems like an omission going forward. It's not so important right
now, since AMs aren't widely used.

Omitting copying the AM seems like a bug during
CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
But this does allow you to specify the TableAM by using
default_table_access_method, and to use
CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL) USING (heapdup);
if you wish to set the AM explicitly, so I don't see this as needing backpatch.

Which means that the emphasis for the earlier functionality was
towards one "preferred AM" rather than using multiple AMs at same
time. Allowing this change in later releases makes sense.

Please make sure this is marked as an incompatibility in the release notes.

This patch should have more tests. Something could be added in
create_am.sql where there is a fake heap2 as table AM.

Yes, I had already done that locally.

There are no tests for the new functionality, please could you add some?

--
Simon Riggs http://www.EnterpriseDB.com/

Justin Pryzby

pryzby@telsasoft.com

almost 5 years ago

In reply to: Simon Riggs (#4)

1 attachment(s)

Re: create table like: ACCESS METHOD

On Wed, Dec 30, 2020 at 12:33:56PM +0000, Simon Riggs wrote:

There are no tests for the new functionality, please could you add some?

Did you look at the most recent patch?

+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);

Also, I just realized that Dilip's toast compression patch adds "INCLUDING
COMPRESSION", which is stored in pg_am. That's an implementation detail of
that patch, but it's not intuitive that "including access method" wouldn't
include the compression stored there. So I think this should use "INCLUDING
TABLE ACCESS METHOD" not just ACCESS METHOD.

--
Justin

Attachments:

v3-0001-create-table-like-.-including-ACCESS-METHOD.patchtext/x-diff; charset=us-asciiDownload

From f27fd6291aa10af1ca0be4bc72a656811c8e0c9f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 15 Nov 2020 16:54:53 -0600
Subject: [PATCH v3] create table (like .. including ACCESS METHOD)

---
 doc/src/sgml/ref/create_table.sgml              | 12 +++++++++++-
 src/backend/parser/gram.y                       |  1 +
 src/backend/parser/parse_utilcmd.c              | 10 ++++++++++
 src/include/nodes/parsenodes.h                  |  1 +
 src/test/regress/expected/create_table_like.out | 12 ++++++++++++
 src/test/regress/sql/create_table_like.sql      |  8 ++++++++
 6 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 569f4c9da7..e3c607f6b1 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,7 +87,7 @@ class="parameter">referential_action</replaceable> ] [ ON UPDATE <replaceable cl
 
 <phrase>and <replaceable class="parameter">like_option</replaceable> is:</phrase>
 
-{ INCLUDING | EXCLUDING } { COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
+{ INCLUDING | EXCLUDING } { COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | TABLE ACCESS METHOD | ALL }
 
 <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>
 
@@ -689,6 +689,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
         </listitem>
        </varlistentry>
 
+       <varlistentry>
+        <term><literal>INCLUDING TABLE ACCESS METHOD</literal></term>
+        <listitem>
+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
+        </listitem>
+       </varlistentry>
+
        <varlistentry>
         <term><literal>INCLUDING ALL</literal></term>
         <listitem>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 31c95443a5..719ac838e3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3708,6 +3708,7 @@ TableLikeOption:
 				| INDEXES			{ $$ = CREATE_TABLE_LIKE_INDEXES; }
 				| STATISTICS		{ $$ = CREATE_TABLE_LIKE_STATISTICS; }
 				| STORAGE			{ $$ = CREATE_TABLE_LIKE_STORAGE; }
+				| TABLE ACCESS METHOD { $$ = CREATE_TABLE_LIKE_TABLE_ACCESS_METHOD; }
 				| ALL				{ $$ = CREATE_TABLE_LIKE_ALL; }
 		;
 
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index b31f3afa03..f34f42aae3 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -96,6 +96,7 @@ typedef struct
 	bool		ispartitioned;	/* true if table is partitioned */
 	PartitionBoundSpec *partbound;	/* transformed FOR VALUES */
 	bool		ofType;			/* true if statement contains OF typename */
+	char		*accessMethod;	/* table access method */
 } CreateStmtContext;
 
 /* State shared by transformCreateSchemaStmt and its subroutines */
@@ -252,6 +253,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	cxt.ispartitioned = stmt->partspec != NULL;
 	cxt.partbound = stmt->partbound;
 	cxt.ofType = (stmt->ofTypename != NULL);
+	cxt.accessMethod = NULL;
 
 	Assert(!stmt->ofTypename || !stmt->inhRelations);	/* grammar enforces */
 
@@ -346,6 +348,9 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	stmt->tableElts = cxt.columns;
 	stmt->constraints = cxt.ckconstraints;
 
+	if (cxt.accessMethod != NULL)
+		stmt->accessMethod = cxt.accessMethod;
+
 	result = lappend(cxt.blist, stmt);
 	result = list_concat(result, cxt.alist);
 	result = list_concat(result, save_alist);
@@ -1118,6 +1123,11 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
 	}
 
+	/* ACCESS METHOD doesn't apply and isn't copied for partitioned tables */
+	if ((table_like_clause->options & CREATE_TABLE_LIKE_TABLE_ACCESS_METHOD) != 0 &&
+		!cxt->ispartitioned)
+		cxt->accessMethod = get_am_name(relation->rd_rel->relam);
+
 	/*
 	 * We may copy extended statistics if requested, since the representation
 	 * of CreateStatsStmt doesn't depend on column numbers.
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index dc2bb40926..600856c229 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -685,6 +685,7 @@ typedef enum TableLikeOption
 	CREATE_TABLE_LIKE_INDEXES = 1 << 5,
 	CREATE_TABLE_LIKE_STATISTICS = 1 << 6,
 	CREATE_TABLE_LIKE_STORAGE = 1 << 7,
+	CREATE_TABLE_LIKE_TABLE_ACCESS_METHOD = 1 << 8,
 	CREATE_TABLE_LIKE_ALL = PG_INT32_MAX
 } TableLikeOption;
 
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 10d17be23c..a09cd48ed0 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -429,6 +429,18 @@ SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s
  ctlt_all_a_b_stat |        0 | ab stats
 (1 row)
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+ amname  
+---------
+ heapdup
+(1 row)
+
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 NOTICE:  merging multiple inherited definitions of column "a"
 ERROR:  inherited column "a" has a storage parameter conflict
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 06b76f949d..ee6c464851 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -165,6 +165,14 @@ CREATE TABLE ctlt_all (LIKE ctlt1 INCLUDING ALL);
 SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 'ctlt_all'::regclass ORDER BY c.relname, objsubid;
 SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s WHERE classoid = 'pg_statistic_ext'::regclass AND objoid = s.oid AND s.stxrelid = 'ctlt_all'::regclass ORDER BY s.stxname, objsubid;
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
+
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 CREATE TABLE inh_error2 (LIKE ctlt4 INCLUDING STORAGE) INHERITS (ctlt1);
 
-- 
2.17.0

David Steele

david@pgmasters.net

almost 5 years ago

In reply to: Justin Pryzby (#5)

Re: create table like: ACCESS METHOD

On 1/19/21 4:03 PM, Justin Pryzby wrote:

On Wed, Dec 30, 2020 at 12:33:56PM +0000, Simon Riggs wrote:

There are no tests for the new functionality, please could you add some?

Did you look at the most recent patch?
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
Also, I just realized that Dilip's toast compression patch adds "INCLUDING
COMPRESSION", which is stored in pg_am. That's an implementation detail of
that patch, but it's not intuitive that "including access method" wouldn't
include the compression stored there. So I think this should use "INCLUDING
TABLE ACCESS METHOD" not just ACCESS METHOD.

Simon, do you know when you'll have a chance to review the updated patch
in [1]/messages/by-id/20210119210331.GN8560@telsasoft.com?

Regards,
--
-David
david@pgmasters.net

[1]: /messages/by-id/20210119210331.GN8560@telsasoft.com
/messages/by-id/20210119210331.GN8560@telsasoft.com

Justin Pryzby

pryzby@telsasoft.com

almost 5 years ago

In reply to: Justin Pryzby (#5)

1 attachment(s)

Re: create table like: ACCESS METHOD

On Tue, Jan 19, 2021 at 03:03:31PM -0600, Justin Pryzby wrote:

On Wed, Dec 30, 2020 at 12:33:56PM +0000, Simon Riggs wrote:

There are no tests for the new functionality, please could you add some?

Did you look at the most recent patch?
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);                                                                                                                                                                               
Also, I just realized that Dilip's toast compression patch adds "INCLUDING
COMPRESSION", which is stored in pg_am. That's an implementation detail of
that patch, but it's not intuitive that "including access method" wouldn't
include the compression stored there. So I think this should use "INCLUDING
TABLE ACCESS METHOD" not just ACCESS METHOD.

Since the TOAST patch ended up not using access methods after all, I renamed
this back to "like ACCESS METHOD" (without table).

For now, I left TableLikeOption un-alphabetized.

--
Justin

Attachments:

v4-0001-create-table-like-.-including-ACCESS-METHOD.patchtext/x-diff; charset=us-asciiDownload

From 2da8104a39f65f576d0f221c288502d27211a209 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 15 Nov 2020 16:54:53 -0600
Subject: [PATCH v4] create table (like .. including ACCESS METHOD)

---
 doc/src/sgml/ref/create_table.sgml              | 12 +++++++++++-
 src/backend/parser/gram.y                       |  1 +
 src/backend/parser/parse_utilcmd.c              | 10 ++++++++++
 src/include/nodes/parsenodes.h                  |  1 +
 src/test/regress/expected/create_table_like.out | 12 ++++++++++++
 src/test/regress/sql/create_table_like.sql      |  8 ++++++++
 6 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index c6c248f1e9..dd92fd0e9a 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,7 +87,7 @@ class="parameter">referential_action</replaceable> ] [ ON UPDATE <replaceable cl
 
 <phrase>and <replaceable class="parameter">like_option</replaceable> is:</phrase>
 
-{ INCLUDING | EXCLUDING } { COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
+{ INCLUDING | EXCLUDING } { ACCESS METHOD | COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
 
 <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>
 
@@ -613,6 +613,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       available options are:
 
       <variablelist>
+       <varlistentry>
+        <term><literal>INCLUDING ACCESS METHOD</literal></term>
+        <listitem>
+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
+        </listitem>
+       </varlistentry>
+
        <varlistentry>
         <term><literal>INCLUDING COMMENTS</literal></term>
         <listitem>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index bc43641ffe..383e0671af 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3741,6 +3741,7 @@ TableLikeOption:
 				| STATISTICS		{ $$ = CREATE_TABLE_LIKE_STATISTICS; }
 				| STORAGE			{ $$ = CREATE_TABLE_LIKE_STORAGE; }
 				| COMPRESSION		{ $$ = CREATE_TABLE_LIKE_COMPRESSION; }
+				| ACCESS METHOD 	{ $$ = CREATE_TABLE_LIKE_ACCESS_METHOD; }
 				| ALL				{ $$ = CREATE_TABLE_LIKE_ALL; }
 		;
 
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index aa6c19adad..07e18fa62f 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -97,6 +97,7 @@ typedef struct
 	bool		ispartitioned;	/* true if table is partitioned */
 	PartitionBoundSpec *partbound;	/* transformed FOR VALUES */
 	bool		ofType;			/* true if statement contains OF typename */
+	char		*accessMethod;	/* table access method */
 } CreateStmtContext;
 
 /* State shared by transformCreateSchemaStmt and its subroutines */
@@ -253,6 +254,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	cxt.ispartitioned = stmt->partspec != NULL;
 	cxt.partbound = stmt->partbound;
 	cxt.ofType = (stmt->ofTypename != NULL);
+	cxt.accessMethod = NULL;
 
 	Assert(!stmt->ofTypename || !stmt->inhRelations);	/* grammar enforces */
 
@@ -347,6 +349,9 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	stmt->tableElts = cxt.columns;
 	stmt->constraints = cxt.ckconstraints;
 
+	if (cxt.accessMethod != NULL)
+		stmt->accessMethod = cxt.accessMethod;
+
 	result = lappend(cxt.blist, stmt);
 	result = list_concat(result, cxt.alist);
 	result = list_concat(result, save_alist);
@@ -1137,6 +1142,11 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
 	}
 
+	/* ACCESS METHOD doesn't apply and isn't copied for partitioned tables */
+	if ((table_like_clause->options & CREATE_TABLE_LIKE_ACCESS_METHOD) != 0 &&
+		!cxt->ispartitioned)
+		cxt->accessMethod = get_am_name(relation->rd_rel->relam);
+
 	/*
 	 * We may copy extended statistics if requested, since the representation
 	 * of CreateStatsStmt doesn't depend on column numbers.
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 68425eb2c0..5e2fc9baca 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -696,6 +696,7 @@ typedef enum TableLikeOption
 	CREATE_TABLE_LIKE_STATISTICS = 1 << 6,
 	CREATE_TABLE_LIKE_STORAGE = 1 << 7,
 	CREATE_TABLE_LIKE_COMPRESSION = 1 << 8,
+	CREATE_TABLE_LIKE_ACCESS_METHOD = 1 << 9,
 	CREATE_TABLE_LIKE_ALL = PG_INT32_MAX
 } TableLikeOption;
 
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 10d17be23c..a09cd48ed0 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -429,6 +429,18 @@ SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s
  ctlt_all_a_b_stat |        0 | ab stats
 (1 row)
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+ amname  
+---------
+ heapdup
+(1 row)
+
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 NOTICE:  merging multiple inherited definitions of column "a"
 ERROR:  inherited column "a" has a storage parameter conflict
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 06b76f949d..ee6c464851 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -165,6 +165,14 @@ CREATE TABLE ctlt_all (LIKE ctlt1 INCLUDING ALL);
 SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 'ctlt_all'::regclass ORDER BY c.relname, objsubid;
 SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s WHERE classoid = 'pg_statistic_ext'::regclass AND objoid = s.oid AND s.stxrelid = 'ctlt_all'::regclass ORDER BY s.stxname, objsubid;
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
+
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 CREATE TABLE inh_error2 (LIKE ctlt4 INCLUDING STORAGE) INHERITS (ctlt1);
 
-- 
2.17.0

Justin Pryzby

pryzby@telsasoft.com

over 4 years ago

In reply to: Justin Pryzby (#7)

1 attachment(s)

Re: create table like: ACCESS METHOD

rebased and alphabetized

Attachments:

v5-0001-create-table-like-.-including-ACCESS-METHOD.patchtext/x-diff; charset=us-asciiDownload

From c1420e17cc2036d4bf3a6c1f9366bf1cd03bd831 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 15 Nov 2020 16:54:53 -0600
Subject: [PATCH v5] create table (like .. including ACCESS METHOD)

---
 doc/src/sgml/ref/create_table.sgml            | 22 ++++++++++++++++++-
 src/backend/parser/gram.y                     |  3 ++-
 src/backend/parser/parse_utilcmd.c            | 10 +++++++++
 src/include/nodes/parsenodes.h                | 19 ++++++++--------
 .../regress/expected/create_table_like.out    | 12 ++++++++++
 src/test/regress/sql/create_table_like.sql    |  8 +++++++
 6 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index c6d0a35e50..ab84ca8718 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,7 +87,7 @@ class="parameter">referential_action</replaceable> ] [ ON UPDATE <replaceable cl
 
 <phrase>and <replaceable class="parameter">like_option</replaceable> is:</phrase>
 
-{ INCLUDING | EXCLUDING } { COMMENTS | COMPRESSION | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
+{ INCLUDING | EXCLUDING } { ACCESS METHOD | COMMENTS | COMPRESSION | CONSTRAINTS | DEFAULTS | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
 
 <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>
 
@@ -618,6 +618,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       available options are:
 
       <variablelist>
+       <varlistentry>
+        <term><literal>INCLUDING ACCESS METHOD</literal></term>
+        <listitem>
+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
+        </listitem>
+       </varlistentry>
+
        <varlistentry>
         <term><literal>INCLUDING COMMENTS</literal></term>
         <listitem>
@@ -725,6 +735,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
         </listitem>
        </varlistentry>
 
+       <varlistentry>
+        <term><literal>INCLUDING TABLE ACCESS METHOD</literal></term>
+        <listitem>
+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
+        </listitem>
+       </varlistentry>
+
        <varlistentry>
         <term><literal>INCLUDING ALL</literal></term>
         <listitem>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 9ee90e3f13..c8991cead9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3754,7 +3754,8 @@ TableLikeOptionList:
 		;
 
 TableLikeOption:
-				COMMENTS			{ $$ = CREATE_TABLE_LIKE_COMMENTS; }
+				ACCESS METHOD			{ $$ = CREATE_TABLE_LIKE_ACCESS_METHOD; }
+				| COMMENTS			{ $$ = CREATE_TABLE_LIKE_COMMENTS; }
 				| COMPRESSION		{ $$ = CREATE_TABLE_LIKE_COMPRESSION; }
 				| CONSTRAINTS		{ $$ = CREATE_TABLE_LIKE_CONSTRAINTS; }
 				| DEFAULTS			{ $$ = CREATE_TABLE_LIKE_DEFAULTS; }
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index d5b67d48cf..0cdea516ac 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -97,6 +97,7 @@ typedef struct
 	bool		ispartitioned;	/* true if table is partitioned */
 	PartitionBoundSpec *partbound;	/* transformed FOR VALUES */
 	bool		ofType;			/* true if statement contains OF typename */
+	char		*accessMethod;	/* table access method */
 } CreateStmtContext;
 
 /* State shared by transformCreateSchemaStmt and its subroutines */
@@ -252,6 +253,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	cxt.ispartitioned = stmt->partspec != NULL;
 	cxt.partbound = stmt->partbound;
 	cxt.ofType = (stmt->ofTypename != NULL);
+	cxt.accessMethod = NULL;
 
 	Assert(!stmt->ofTypename || !stmt->inhRelations);	/* grammar enforces */
 
@@ -349,6 +351,9 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	stmt->tableElts = cxt.columns;
 	stmt->constraints = cxt.ckconstraints;
 
+	if (cxt.accessMethod != NULL)
+		stmt->accessMethod = cxt.accessMethod;
+
 	result = lappend(cxt.blist, stmt);
 	result = list_concat(result, cxt.alist);
 	result = list_concat(result, save_alist);
@@ -1139,6 +1144,11 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
 	}
 
+	/* ACCESS METHOD doesn't apply and isn't copied for partitioned tables */
+	if ((table_like_clause->options & CREATE_TABLE_LIKE_ACCESS_METHOD) != 0 &&
+		!cxt->ispartitioned)
+		cxt->accessMethod = get_am_name(relation->rd_rel->relam);
+
 	/*
 	 * We may copy extended statistics if requested, since the representation
 	 * of CreateStatsStmt doesn't depend on column numbers.
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ef73342019..726f15f78a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -689,15 +689,16 @@ typedef struct TableLikeClause
 
 typedef enum TableLikeOption
 {
-	CREATE_TABLE_LIKE_COMMENTS = 1 << 0,
-	CREATE_TABLE_LIKE_COMPRESSION = 1 << 1,
-	CREATE_TABLE_LIKE_CONSTRAINTS = 1 << 2,
-	CREATE_TABLE_LIKE_DEFAULTS = 1 << 3,
-	CREATE_TABLE_LIKE_GENERATED = 1 << 4,
-	CREATE_TABLE_LIKE_IDENTITY = 1 << 5,
-	CREATE_TABLE_LIKE_INDEXES = 1 << 6,
-	CREATE_TABLE_LIKE_STATISTICS = 1 << 7,
-	CREATE_TABLE_LIKE_STORAGE = 1 << 8,
+	CREATE_TABLE_LIKE_ACCESS_METHOD = 1 << 0,
+	CREATE_TABLE_LIKE_COMMENTS = 1 << 1,
+	CREATE_TABLE_LIKE_COMPRESSION = 1 << 2,
+	CREATE_TABLE_LIKE_CONSTRAINTS = 1 << 3,
+	CREATE_TABLE_LIKE_DEFAULTS = 1 << 4,
+	CREATE_TABLE_LIKE_GENERATED = 1 << 5,
+	CREATE_TABLE_LIKE_IDENTITY = 1 << 6,
+	CREATE_TABLE_LIKE_INDEXES = 1 << 7,
+	CREATE_TABLE_LIKE_STATISTICS = 1 << 8,
+	CREATE_TABLE_LIKE_STORAGE = 1 << 9,
 	CREATE_TABLE_LIKE_ALL = PG_INT32_MAX
 } TableLikeOption;
 
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 4dc5e6aa5f..68411e3a84 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -433,6 +433,18 @@ SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s
  ctlt_all_expr_stat |        0 | ab expr stats
 (2 rows)
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+ amname  
+---------
+ heapdup
+(1 row)
+
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 NOTICE:  merging multiple inherited definitions of column "a"
 ERROR:  inherited column "a" has a storage parameter conflict
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 4929d373a2..1845b9adb5 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -167,6 +167,14 @@ CREATE TABLE ctlt_all (LIKE ctlt1 INCLUDING ALL);
 SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 'ctlt_all'::regclass ORDER BY c.relname, objsubid;
 SELECT s.stxname, objsubid, description FROM pg_description, pg_statistic_ext s WHERE classoid = 'pg_statistic_ext'::regclass AND objoid = s.oid AND s.stxrelid = 'ctlt_all'::regclass ORDER BY s.stxname, objsubid;
 
+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
+-- pg_regress helpfully hides the Access Method output, which we need:
+SELECT a.amname FROM pg_class c JOIN pg_am a ON c.relam=a.oid WHERE c.oid='likeamlike'::regclass;
+DROP TABLE likeam, likeamlike;
+DROP ACCESS METHOD heapdup;
+
 CREATE TABLE inh_error1 () INHERITS (ctlt1, ctlt4);
 CREATE TABLE inh_error2 (LIKE ctlt4 INCLUDING STORAGE) INHERITS (ctlt1);
 
-- 
2.17.0

Michael Paquier

michael@paquier.xyz

over 4 years ago

In reply to: Justin Pryzby (#8)

Re: create table like: ACCESS METHOD

On Tue, Jun 01, 2021 at 02:10:45PM -0500, Justin Pryzby wrote:

rebased and alphabetized

+   /* ACCESS METHOD doesn't apply and isn't copied for partitioned tables */
+   if ((table_like_clause->options & CREATE_TABLE_LIKE_ACCESS_METHOD) != 0 &&
+       !cxt->ispartitioned)
+       cxt->accessMethod = get_am_name(relation->rd_rel->relam);
I was thinking about an ERROR here, but all the other options do the
work when specified only if required, so that's fine.  We should have
a test with a partitioned table and the clause specified, though.

+         <para>
+          The table's access method will be copied.  By default, the
+          <literal>default_table_access_method</literal> is used.
+         </para>
Why is there any need to mention default_table_access_method?  This
just inherits the AM from the source table, which has nothing to do
with the default directly.

+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
Rather than creating a custom AM in this test path, I would be
tempted to move that to create_am.sql.
--
Michael

#10

Vik Fearing

vik@postgresfriends.org

over 4 years ago

In reply to: Justin Pryzby (#7)

Re: create table like: ACCESS METHOD

On 3/23/21 1:39 AM, Justin Pryzby wrote:

On Tue, Jan 19, 2021 at 03:03:31PM -0600, Justin Pryzby wrote:

On Wed, Dec 30, 2020 at 12:33:56PM +0000, Simon Riggs wrote:

There are no tests for the new functionality, please could you add some?

Did you look at the most recent patch?

+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);

It seems like this should error to me:

CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
CREATE TABLE likeam1() USING heap;
CREATE TABLE likeam2() USING heapdup;
CREATE TABLE likeamlike(
LIKE likeam1 INCLUDING ACCESS METHOD,
LIKE likeam2 INCLUDING ACCESS METHOD
);

At the very least, the documentation should say that the last one wins.
--
Vik Fearing

#11

Michael Paquier

michael@paquier.xyz

over 4 years ago

In reply to: Vik Fearing (#10)

Re: create table like: ACCESS METHOD

On Fri, Aug 27, 2021 at 12:37:59PM +0200, Vik Fearing wrote:

It seems like this should error to me:

CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
CREATE TABLE likeam1() USING heap;
CREATE TABLE likeam2() USING heapdup;
CREATE TABLE likeamlike(
LIKE likeam1 INCLUDING ACCESS METHOD,
LIKE likeam2 INCLUDING ACCESS METHOD
);

At the very least, the documentation should say that the last one wins.

An error may be annoying once you do an INCLUDING ALL with more than
one relation, no? I'd be fine with just documenting that the last one
wins.
--
Michael

#12

Michael Paquier

michael@paquier.xyz

over 4 years ago

In reply to: Michael Paquier (#9)

Re: create table like: ACCESS METHOD

On Fri, Aug 27, 2021 at 02:38:43PM +0900, Michael Paquier wrote:

+CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE likeam() USING heapdup;
+CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
Rather than creating a custom AM in this test path, I would be
tempted to move that to create_am.sql.

+       /* ACCESS METHOD doesn't apply and isn't copied for partitioned tables */
+       if ((table_like_clause->options & CREATE_TABLE_LIKE_ACCESS_METHOD) != 0 &&
+               !cxt->ispartitioned)
+               cxt->accessMethod = get_am_name(relation->rd_rel->relam);

If the new table is partitioned, this would work. Now I think that we
should also add here a (relation->rd_rel->relkind == RELKIND_RELATION)
to make sure that we only copy an access method if the original
relation is a table. Note that the original relation could be as well
a view, a foreign table or a composite type.

@@ -349,6 +351,9 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
[...]
+       if (cxt.accessMethod != NULL)
+               stmt->accessMethod = cxt.accessMethod;

This bit is something I have been chewing on a bit. It means that if
we find out an AM to copy from any of the LIKE clauses, we would
blindly overwrite the AM defined in an existing CreateStmt. We could
also argue in favor of keeping the original AM defined by USING from
the query rather than having an error. This means to check that
stmt->accessMethod is overwritten only if NULL at this point. Anyway,
the patch is wrong with this implementation.

This makes me actually wonder if this patch is really a good idea at
the end. The interactions with USING and LIKE would be confusing to
the end-user one way or the other. The argument of upthread regarding
INCLUDING ALL or INCLUDING ACCESS METHOD with multiple original
relations also goes in this sense. If we want to move forward here, I
think that we should really be careful and have a clear definition
behind all those corner cases. The patch fails this point for now.
--
Michael

#13

Peter Eisentraut

peter.eisentraut@enterprisedb.com

over 4 years ago

In reply to: Vik Fearing (#10)

Re: create table like: ACCESS METHOD

On 27.08.21 12:37, Vik Fearing wrote:

It seems like this should error to me:

CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
CREATE TABLE likeam1() USING heap;
CREATE TABLE likeam2() USING heapdup;
CREATE TABLE likeamlike(
LIKE likeam1 INCLUDING ACCESS METHOD,
LIKE likeam2 INCLUDING ACCESS METHOD
);

At the very least, the documentation should say that the last one wins.

Hmm. The problem is that the LIKE clause is really a macro that expands
to the column definitions of the other table. So there is, so far, no
such as thing as two LIKE clauses contradicting. Whereas the access
method is a table property. So I don't think this syntax is the right
approach for this feature.

You might think about something like

CREATE TABLE t2 (...) USING (LIKE t1);

At least in terms of how the syntax should be structured.

#14

Michael Paquier

michael@paquier.xyz

over 4 years ago

In reply to: Peter Eisentraut (#13)

Re: create table like: ACCESS METHOD

On Thu, Sep 09, 2021 at 02:30:51PM +0200, Peter Eisentraut wrote:

Hmm. The problem is that the LIKE clause is really a macro that expands to
the column definitions of the other table. So there is, so far, no such as
thing as two LIKE clauses contradicting. Whereas the access method is a
table property. So I don't think this syntax is the right approach for this
feature.

You might think about something like

CREATE TABLE t2 (...) USING (LIKE t1);

At least in terms of how the syntax should be structured.

Good point. I have marked the patch as RwF.
--
Michael