public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v3] create table (like .. including ACCESS METHOD) 2+ messages / 2 participants [nested] [flat]
* [PATCH v3] create table (like .. including ACCESS METHOD) @ 2020-11-15 22:54 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-11-15 22:54 UTC (permalink / raw) --- 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 --T6xhMxlHU34Bk0ad-- ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: support for MERGE @ 2022-02-27 17:35 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Alvaro Herrera @ 2022-02-27 17:35 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Pg Hackers <[email protected]>; Simon Riggs <[email protected]>; Tomas Vondra <[email protected]>; Zhihong Yu <[email protected]>; Daniel Westermann <[email protected]>; Amit Langote <[email protected]>; Justin Pryzby <[email protected]>; Japin Li <[email protected]>; Erik Rijkers <[email protected]>; Jaime Casanova <[email protected]> On 2022-Jan-28, Andres Freund wrote: > Any chance you could split this into something more reviewable? The overall > diff stats are: 102 files changed, 8589 insertions(+), 234 deletions(-) thats > pretty hard to really review. Incremental commits don't realy help with that. I'll work on splitting this next week. > One thing from skimming: There's not enough documentation about the approach > imo - it's a complicated enough feature to deserve more than the one paragraph > in src/backend/executor/README. Sure, I'll have a look at that. > I'm still [1],[2] uncomfortable with execMerge.c kinda-but-not-really being an > executor node. I think we should make a decision on code arrangement here. From my perspective, MERGE isn't its own executor node; rather it's just another "method" in ModifyTable. Which makes sense, given that all it does is call parts of INSERT, UPDATE, DELETE which are the other ModifyTable methods. Having a separate file doesn't strike me as great, but on the other hand it's true that merely moving all the execMerge.c code into nodeModifyTable.c makes the latter too large. However I don't want to create a .h file that means exposing all those internal functions to the outside world. My ideal would be to have each INSERT, UPDATE, DELETE, MERGE as its own separate .c file, which would be #included from nodeModifyTable.c. We don't use that pattern anywhere though. Any opposition to that? (The prototypes for each file would have to live in nodeModifyTable.c itself.) > > diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c > > index 1a9c1ac290..280ac40e63 100644 > > --- a/src/backend/commands/trigger.c > > +++ b/src/backend/commands/trigger.c > > This stuff seems like one candidate for splitting out. Yeah, I had done that. It's now posted as 0001. > > + /* > > + * We maintain separate transition tables for UPDATE/INSERT/DELETE since > > + * MERGE can run all three actions in a single statement. Note that UPDATE > > + * needs both old and new transition tables whereas INSERT needs only new, > > + * and DELETE needs only old. > > + */ > > + > > + /* "old" transition table for UPDATE, if any */ > > + Tuplestorestate *old_upd_tuplestore; > > + /* "new" transition table for UPDATE, if any */ > > + Tuplestorestate *new_upd_tuplestore; > > + /* "old" transition table for DELETE, if any */ > > + Tuplestorestate *old_del_tuplestore; > > + /* "new" transition table INSERT, if any */ > > + Tuplestorestate *new_ins_tuplestore; > > + > > TupleTableSlot *storeslot; /* for converting to tuplestore's format */ > > }; > > Do we need to do something slightly clever about the memory limits for the > tuplestore? Each of them having a separate work_mem makes nodeModifyTable.c > one memory hungry node (the worst of all maybe?). Not sure how that would work. The memory handling is inside tuplestore.c IIRC ... I think that would require some largish refactoring. Merely dividing by four doesn't seem like a great answer either. Perhaps we could divide by two and be optimistic about it. > > + /* > > + * Project the tuple. In case of a partitioned table, the > > + * projection was already built to use the root's descriptor, > > + * so we don't need to map the tuple here. > > + */ > > + actionInfo.actionState = action; > > + insert_slot = ExecProject(action->mas_proj); > > + > > + (void) ExecInsert(mtstate, rootRelInfo, > > + insert_slot, slot, > > + estate, &actionInfo, > > + mtstate->canSetTag); > > + InstrCountFiltered1(&mtstate->ps, 1); > > What happens if somebody concurrently inserts a conflicting row? An open question to which I don't have any good answer RN. -- Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/ "Cuando mañana llegue pelearemos segun lo que mañana exija" (Mowgli) ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2022-02-27 17:35 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-11-15 22:54 [PATCH v3] create table (like .. including ACCESS METHOD) Justin Pryzby <[email protected]> 2022-02-27 17:35 Re: support for MERGE Alvaro Herrera <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox