On Sat, Jul 23, 2022 at 05:44:28PM +0900, Michael Paquier wrote: > Changing get_altertable_subcmdtypes() to return a set of rows made of > (subcommand, object description) is what I actually meant upthread as > it feels natural given a CollectedCommand in input, and as > pg_event_trigger_ddl_commands() only gives access to a set of > CollectedCommands. This is also a test module so > there is no issue in changing the existing function definitions. > > But your point would be to have a new function that takes in input a > CollectedATSubcmd, returning back the object address or its > description? How would you make sure that a subcommand maps to a > correct object address?
FWIW, I was thinking about something among the lines of 0002 on top of Hou's patch. -- Michael
From df118fedd1204ac4596de59ad2ef87cb1f734a35 Mon Sep 17 00:00:00 2001 From: Hou Zhijie <houzj.f...@cn.fujitsu.com> Date: Wed, 13 Jul 2022 17:39:13 +0800 Subject: [PATCH v2 1/2] Collect ObjectAddress for ATTACH DETACH PARTITION --- src/backend/commands/tablecmds.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 7fbee0c1f7..9cf4671da0 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5193,11 +5193,11 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, cur_pass, context); Assert(cmd != NULL); if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - ATExecAttachPartition(wqueue, rel, (PartitionCmd *) cmd->def, - context); + address = ATExecAttachPartition(wqueue, rel, (PartitionCmd *) cmd->def, + context); else - ATExecAttachPartitionIdx(wqueue, rel, - ((PartitionCmd *) cmd->def)->name); + address = ATExecAttachPartitionIdx(wqueue, rel, + ((PartitionCmd *) cmd->def)->name); break; case AT_DetachPartition: cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode, @@ -5205,12 +5205,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Assert(cmd != NULL); /* ATPrepCmd ensures it must be a table */ Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); - ATExecDetachPartition(wqueue, tab, rel, - ((PartitionCmd *) cmd->def)->name, - ((PartitionCmd *) cmd->def)->concurrent); + address = ATExecDetachPartition(wqueue, tab, rel, + ((PartitionCmd *) cmd->def)->name, + ((PartitionCmd *) cmd->def)->concurrent); break; case AT_DetachPartitionFinalize: - ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name); + address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name); break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", -- 2.36.1
From a013fb2948986302490f09c5e44a3b510030f293 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Sat, 23 Jul 2022 19:53:07 +0900
Subject: [PATCH v2 2/2] Extend test_ddl_deparse for ALTER TABLE ..
ATTACH/DETACH PARTITION
---
.../test_ddl_deparse/expected/alter_table.out | 19 ++++++--
.../expected/create_table.out | 8 ++--
.../test_ddl_deparse/expected/create_view.out | 2 +-
.../expected/test_ddl_deparse.out | 4 +-
.../test_ddl_deparse/sql/alter_table.sql | 5 ++
.../test_ddl_deparse/sql/test_ddl_deparse.sql | 4 +-
.../test_ddl_deparse--1.0.sql | 6 ++-
.../test_ddl_deparse/test_ddl_deparse.c | 46 +++++++++++++++----
8 files changed, 68 insertions(+), 26 deletions(-)
diff --git a/src/test/modules/test_ddl_deparse/expected/alter_table.out b/src/test/modules/test_ddl_deparse/expected/alter_table.out
index 141060fbdc..ec22d688b1 100644
--- a/src/test/modules/test_ddl_deparse/expected/alter_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out
@@ -9,21 +9,30 @@ NOTICE: DDL test: type simple, tag CREATE TABLE
ALTER TABLE parent ADD COLUMN b serial;
NOTICE: DDL test: type simple, tag CREATE SEQUENCE
NOTICE: DDL test: type alter table, tag ALTER TABLE
-NOTICE: subcommand: ADD COLUMN (and recurse)
+NOTICE: subcommand: type ADD COLUMN (and recurse) desc column b of table parent
NOTICE: DDL test: type simple, tag ALTER SEQUENCE
ALTER TABLE parent RENAME COLUMN b TO c;
NOTICE: DDL test: type simple, tag ALTER TABLE
ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
NOTICE: DDL test: type alter table, tag ALTER TABLE
-NOTICE: subcommand: ADD CONSTRAINT (and recurse)
+NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint a_pos on table parent
CREATE TABLE part (
a int
) PARTITION BY RANGE (a);
NOTICE: DDL test: type simple, tag CREATE TABLE
CREATE TABLE part1 PARTITION OF part FOR VALUES FROM (1) to (100);
NOTICE: DDL test: type simple, tag CREATE TABLE
+CREATE TABLE part2 (a int);
+NOTICE: DDL test: type simple, tag CREATE TABLE
+ALTER TABLE part ATTACH PARTITION part2 FOR VALUES FROM (101) to (200);
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ATTACH PARTITION desc table part2
+ALTER TABLE part DETACH PARTITION part2;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DETACH PARTITION desc table part2
+DROP TABLE part2;
ALTER TABLE part ADD PRIMARY KEY (a);
NOTICE: DDL test: type alter table, tag ALTER TABLE
-NOTICE: subcommand: SET NOT NULL
-NOTICE: subcommand: SET NOT NULL
-NOTICE: subcommand: ADD INDEX
+NOTICE: subcommand: type SET NOT NULL desc column a of table part
+NOTICE: subcommand: type SET NOT NULL desc column a of table part1
+NOTICE: subcommand: type ADD INDEX desc index part_pkey
diff --git a/src/test/modules/test_ddl_deparse/expected/create_table.out b/src/test/modules/test_ddl_deparse/expected/create_table.out
index 0f2a2c164e..2178ce83e9 100644
--- a/src/test/modules/test_ddl_deparse/expected/create_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/create_table.out
@@ -77,8 +77,8 @@ NOTICE: DDL test: type simple, tag CREATE TABLE
NOTICE: DDL test: type simple, tag CREATE INDEX
NOTICE: DDL test: type simple, tag CREATE INDEX
NOTICE: DDL test: type alter table, tag ALTER TABLE
-NOTICE: subcommand: ADD CONSTRAINT (and recurse)
-NOTICE: subcommand: ADD CONSTRAINT (and recurse)
+NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint fkey_table_datatype_id_fkey on table fkey_table
+NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint fkey_big_id on table fkey_table
-- Typed table
CREATE TABLE employees OF employee_type (
PRIMARY KEY (name),
@@ -86,7 +86,7 @@ CREATE TABLE employees OF employee_type (
);
NOTICE: DDL test: type simple, tag CREATE TABLE
NOTICE: DDL test: type alter table, tag ALTER TABLE
-NOTICE: subcommand: SET NOT NULL
+NOTICE: subcommand: type SET NOT NULL desc column name of table employees
NOTICE: DDL test: type simple, tag CREATE INDEX
-- Inheritance
CREATE TABLE person (
@@ -136,7 +136,7 @@ CREATE TABLE like_fkey_table (
);
NOTICE: DDL test: type simple, tag CREATE TABLE
NOTICE: DDL test: type alter table, tag ALTER TABLE
-NOTICE: subcommand: ALTER COLUMN SET DEFAULT (precooked)
+NOTICE: subcommand: type ALTER COLUMN SET DEFAULT (precooked) desc column id of table like_fkey_table
NOTICE: DDL test: type simple, tag CREATE INDEX
NOTICE: DDL test: type simple, tag CREATE INDEX
-- Volatile table types
diff --git a/src/test/modules/test_ddl_deparse/expected/create_view.out b/src/test/modules/test_ddl_deparse/expected/create_view.out
index 2ae4e2d225..4ae0f4978e 100644
--- a/src/test/modules/test_ddl_deparse/expected/create_view.out
+++ b/src/test/modules/test_ddl_deparse/expected/create_view.out
@@ -8,7 +8,7 @@ CREATE OR REPLACE VIEW static_view AS
SELECT 'bar'::TEXT AS col;
NOTICE: DDL test: type simple, tag CREATE VIEW
NOTICE: DDL test: type alter table, tag CREATE VIEW
-NOTICE: subcommand: REPLACE RELOPTIONS
+NOTICE: subcommand: type REPLACE RELOPTIONS desc <NULL>
CREATE VIEW datatype_view AS
SELECT * FROM datatype_table;
NOTICE: DDL test: type simple, tag CREATE VIEW
diff --git a/src/test/modules/test_ddl_deparse/expected/test_ddl_deparse.out b/src/test/modules/test_ddl_deparse/expected/test_ddl_deparse.out
index 4a5ea9e9ed..fc657ff49b 100644
--- a/src/test/modules/test_ddl_deparse/expected/test_ddl_deparse.out
+++ b/src/test/modules/test_ddl_deparse/expected/test_ddl_deparse.out
@@ -28,9 +28,9 @@ BEGIN
-- if alter table, log more
IF cmdtype = 'alter table' THEN
FOR r2 IN SELECT *
- FROM unnest(public.get_altertable_subcmdtypes(r.command))
+ FROM public.get_altertable_subcmdinfo(r.command)
LOOP
- RAISE NOTICE ' subcommand: %', r2.unnest;
+ RAISE NOTICE ' subcommand: type % desc %', r2.cmdtype, r2.objdesc;
END LOOP;
END IF;
END LOOP;
diff --git a/src/test/modules/test_ddl_deparse/sql/alter_table.sql b/src/test/modules/test_ddl_deparse/sql/alter_table.sql
index dec53a0640..b0989570d5 100644
--- a/src/test/modules/test_ddl_deparse/sql/alter_table.sql
+++ b/src/test/modules/test_ddl_deparse/sql/alter_table.sql
@@ -18,4 +18,9 @@ CREATE TABLE part (
CREATE TABLE part1 PARTITION OF part FOR VALUES FROM (1) to (100);
+CREATE TABLE part2 (a int);
+ALTER TABLE part ATTACH PARTITION part2 FOR VALUES FROM (101) to (200);
+ALTER TABLE part DETACH PARTITION part2;
+DROP TABLE part2;
+
ALTER TABLE part ADD PRIMARY KEY (a);
diff --git a/src/test/modules/test_ddl_deparse/sql/test_ddl_deparse.sql b/src/test/modules/test_ddl_deparse/sql/test_ddl_deparse.sql
index e257a215e4..a4716153df 100644
--- a/src/test/modules/test_ddl_deparse/sql/test_ddl_deparse.sql
+++ b/src/test/modules/test_ddl_deparse/sql/test_ddl_deparse.sql
@@ -29,9 +29,9 @@ BEGIN
-- if alter table, log more
IF cmdtype = 'alter table' THEN
FOR r2 IN SELECT *
- FROM unnest(public.get_altertable_subcmdtypes(r.command))
+ FROM public.get_altertable_subcmdinfo(r.command)
LOOP
- RAISE NOTICE ' subcommand: %', r2.unnest;
+ RAISE NOTICE ' subcommand: type % desc %', r2.cmdtype, r2.objdesc;
END LOOP;
END IF;
END LOOP;
diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse--1.0.sql b/src/test/modules/test_ddl_deparse/test_ddl_deparse--1.0.sql
index 093005ad80..861d843690 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse--1.0.sql
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse--1.0.sql
@@ -11,6 +11,8 @@ CREATE FUNCTION get_command_tag(pg_ddl_command)
RETURNS text IMMUTABLE STRICT
AS 'MODULE_PATHNAME' LANGUAGE C;
-CREATE FUNCTION get_altertable_subcmdtypes(pg_ddl_command)
- RETURNS text[] IMMUTABLE STRICT
+CREATE FUNCTION get_altertable_subcmdinfo(IN cmd pg_ddl_command,
+ OUT cmdtype text,
+ OUT objdesc text)
+ RETURNS SETOF record IMMUTABLE STRICT
AS 'MODULE_PATHNAME' LANGUAGE C;
diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
index 9476c3f76e..4476c8a90e 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -11,6 +11,8 @@
#include "postgres.h"
#include "catalog/pg_type.h"
+#include "funcapi.h"
+#include "nodes/execnodes.h"
#include "tcop/deparse_utility.h"
#include "tcop/utility.h"
#include "utils/builtins.h"
@@ -19,7 +21,7 @@ PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(get_command_type);
PG_FUNCTION_INFO_V1(get_command_tag);
-PG_FUNCTION_INFO_V1(get_altertable_subcmdtypes);
+PG_FUNCTION_INFO_V1(get_altertable_subcmdinfo);
/*
* Return the textual representation of the struct type used to represent a
@@ -82,20 +84,30 @@ get_command_tag(PG_FUNCTION_ARGS)
* command.
*/
Datum
-get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
+get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
{
CollectedCommand *cmd = (CollectedCommand *) PG_GETARG_POINTER(0);
- ArrayBuildState *astate = NULL;
ListCell *cell;
+ ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
if (cmd->type != SCT_AlterTable)
elog(ERROR, "command is not ALTER TABLE");
+ SetSingleFuncCall(fcinfo, 0);
+
+ if (list_length(cmd->d.alterTable.subcmds) == 0)
+ elog(ERROR, "empty alter table subcommand list");
+
foreach(cell, cmd->d.alterTable.subcmds)
{
CollectedATSubcmd *sub = lfirst(cell);
AlterTableCmd *subcmd = castNode(AlterTableCmd, sub->parsetree);
const char *strtype;
+ Datum values[2];
+ bool nulls[2];
+
+ memset(values, 0, sizeof(values));
+ memset(nulls, 0, sizeof(nulls));
switch (subcmd->subtype)
{
@@ -279,18 +291,32 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
case AT_GenericOptions:
strtype = "SET OPTIONS";
break;
+ case AT_DetachPartition:
+ strtype = "DETACH PARTITION";
+ break;
+ case AT_AttachPartition:
+ strtype = "ATTACH PARTITION";
+ break;
+ case AT_DetachPartitionFinalize:
+ strtype = "DETACH PARTITION ... FINALIZE";
+ break;
default:
strtype = "unrecognized";
break;
}
- astate =
- accumArrayResult(astate, CStringGetTextDatum(strtype),
- false, TEXTOID, CurrentMemoryContext);
+ values[0] = CStringGetTextDatum(strtype);
+ if (OidIsValid(sub->address.objectId))
+ {
+ char *objdesc;
+ objdesc = getObjectDescription((const ObjectAddress *) &sub->address, false);
+ values[1] = CStringGetTextDatum(objdesc);
+ }
+ else
+ nulls[1] = true;
+
+ tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
}
- if (astate == NULL)
- elog(ERROR, "empty alter table subcommand list");
-
- PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate, CurrentMemoryContext));
+ return (Datum) 0;
}
--
2.36.1
signature.asc
Description: PGP signature
