Collect ObjectAddress for ATTACH DETACH PARTITION to use in event trigger
Hi hackers,
I noticed that we didn't collect the ObjectAddress returned by
ATExec[Attach|Detach]Partition. I think collecting this information can make it
easier for users to get the partition OID of the attached or detached table in
the event trigger. So how about collecting it like the attached patch ?
Best regards,
Hou zhijie
Attachments:
0001-Collect-ObjectAddress-for-ATTACH-DETACH-PARTITION.patchapplication/octet-stream; name=0001-Collect-ObjectAddress-for-ATTACH-DETACH-PARTITION.patchDownload
From 1139ca2fa85b85478fcd9565afe38617cbd4b8c6 Mon Sep 17 00:00:00 2001
From: "Hou Zhijie" <houzj.fnst@cn.fujitsu.com>
Date: Wed, 13 Jul 2022 17:39:13 +0800
Subject: [PATCH] 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 ef5b34a..6b462f5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5189,11 +5189,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,
@@ -5201,12 +5201,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.7.2.windows.1
On Wednesday, July 13, 2022 5:58 PM Hou, Zhijie wrote:
Hi hackers,
I noticed that we didn't collect the ObjectAddress returned by
ATExec[Attach|Detach]Partition. I think collecting this information can make it
easier for users to get the partition OID of the attached or detached table in
the event trigger. So how about collecting it like the attached patch ?
Added to next CF.
Best regards,
Hou zhijie
Hi,
I noticed that we didn't collect the ObjectAddress returned by
ATExec[Attach|Detach]Partition. I think collecting this information can make it
easier for users to get the partition OID of the attached or detached table in
the event trigger. So how about collecting it like the attached patch ?Added to next CF.
Sounds good. I grepped ATExecXXX() functions called in ATExecCmd(),
and I confirmed that all returned values have been collected except them.
While checking test code test about EVENT TRIGGER,
I found there were no tests related with partitions in that.
How about adding them?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
On Fri, Jul 15, 2022 at 03:21:30AM +0000, kuroda.hayato@fujitsu.com wrote:
Sounds good. I grepped ATExecXXX() functions called in ATExecCmd(),
and I confirmed that all returned values have been collected except them.While checking test code test about EVENT TRIGGER,
I found there were no tests related with partitions in that.
How about adding them?
Agreed. It would be good to track down what changes once those
ObjectAddresses are collected.
--
Michael
On Friday, July 15, 2022 11:41 AM Michael Paquier <michael@paquier.xyz> wrote:
Hi,
On Fri, Jul 15, 2022 at 03:21:30AM +0000, kuroda.hayato@fujitsu.com wrote:
Sounds good. I grepped ATExecXXX() functions called in ATExecCmd(),
and I confirmed that all returned values have been collected except them.While checking test code test about EVENT TRIGGER, I found there were
no tests related with partitions in that.
How about adding them?Agreed. It would be good to track down what changes once those
ObjectAddresses are collected.
Thanks for having a look. It was a bit difficult to add a test for this.
Because we currently don't have a user function which can return these
collected ObjectAddresses for ALTER TABLE. And It seems we don't have tests for
already collected ObjectAddresses as well :(
The collected ObjectAddresses is in
"currentEventTriggerState->currentCommand->d.alterTable.subcmds.address" while
the public function pg_event_trigger_ddl_commands doesn't return these
information. It can only be used in user defined event trigger function (C
code).
If we want to add some tests for both already existed and newly added
ObjectAddresses, we might need to add some test codes in test_ddl_deparse.c.
What do you think ?
Best regards,
Hou zhijie
Dear Hou-san,
Thanks for having a look. It was a bit difficult to add a test for this.
Because we currently don't have a user function which can return these
collected ObjectAddresses for ALTER TABLE. And It seems we don't have tests for
already collected ObjectAddresses as well :(
The collected ObjectAddresses is in
"currentEventTriggerState->currentCommand->d.alterTable.subcmds.address" while
the public function pg_event_trigger_ddl_commands doesn't return these
information. It can only be used in user defined event trigger function (C
code).
Thanks for explaining. I did not know the reason why the test is not in event_trigger.sql.
If we want to add some tests for both already existed and newly added
ObjectAddresses, we might need to add some test codes in test_ddl_deparse.c.
What do you think ?
I thought tests for ObjectAddresses should be added to test_ddl_deparse.c, but
it might be bigger because there were many ATExecXXX() functions.
I thought they could be added separately in another thread or patch.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
On Fri, Jul 15, 2022 at 11:39 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
On Friday, July 15, 2022 11:41 AM Michael Paquier <michael@paquier.xyz> wrote:
Hi,
On Fri, Jul 15, 2022 at 03:21:30AM +0000, kuroda.hayato@fujitsu.com wrote:
Sounds good. I grepped ATExecXXX() functions called in ATExecCmd(),
and I confirmed that all returned values have been collected except them.While checking test code test about EVENT TRIGGER, I found there were
no tests related with partitions in that.
How about adding them?Agreed. It would be good to track down what changes once those
ObjectAddresses are collected.Thanks for having a look. It was a bit difficult to add a test for this.
Because we currently don't have a user function which can return these
collected ObjectAddresses for ALTER TABLE. And It seems we don't have tests for
already collected ObjectAddresses as well :(The collected ObjectAddresses is in
"currentEventTriggerState->currentCommand->d.alterTable.subcmds.address" while
the public function pg_event_trigger_ddl_commands doesn't return these
information. It can only be used in user defined event trigger function (C
code).If we want to add some tests for both already existed and newly added
ObjectAddresses, we might need to add some test codes in test_ddl_deparse.c.
What do you think ?
Right. But, I noticed that get_altertable_subcmdtypes() doesn't handle
AT_AttachPartition or AT_DetachPartition. We can handle those and at
least have a test for those in test_ddl_deparse\sql\slter_table.sql. I
know it is not directly related to your patch but that way we will at
least have some tests for Attach/Detach partition and in the future
when we extend it to test ObjectAddresses of subcommands that would be
handy. Feel free to write a separate patch for the same.
--
With Regards,
Amit Kapila.
On Wed, Jul 20, 2022 at 04:36:13PM +0530, Amit Kapila wrote:
Right. But, I noticed that get_altertable_subcmdtypes() doesn't handle
AT_AttachPartition or AT_DetachPartition. We can handle those and at
least have a test for those in test_ddl_deparse\sql\slter_table.sql. I
know it is not directly related to your patch but that way we will at
least have some tests for Attach/Detach partition and in the future
when we extend it to test ObjectAddresses of subcommands that would be
handy. Feel free to write a separate patch for the same.
Yeah, that could be a separate patch. On top of that, what about
reworking get_altertable_subcmdtypes() so as it returns one row for
each CollectedCommand, as of (type text, address text)? We have
already getObjectDescription() to transform the ObjectAddress from the
collected command to a proper string.
--
Michael
On Thu, Jul 21, 2022 at 11:53 AM Michael Paquier <michael@paquier.xyz> wrote:
On Wed, Jul 20, 2022 at 04:36:13PM +0530, Amit Kapila wrote:
Right. But, I noticed that get_altertable_subcmdtypes() doesn't handle
AT_AttachPartition or AT_DetachPartition. We can handle those and at
least have a test for those in test_ddl_deparse\sql\slter_table.sql. I
know it is not directly related to your patch but that way we will at
least have some tests for Attach/Detach partition and in the future
when we extend it to test ObjectAddresses of subcommands that would be
handy. Feel free to write a separate patch for the same.Yeah, that could be a separate patch. On top of that, what about
reworking get_altertable_subcmdtypes() so as it returns one row for
each CollectedCommand, as of (type text, address text)? We have
already getObjectDescription() to transform the ObjectAddress from the
collected command to a proper string.
Yeah, that would be a good idea but I think instead of changing
get_altertable_subcmdtypes(), can we have a new function say
get_altertable_subcmdinfo() that returns additional information from
address. The other alternative could be that instead of returning the
address as a string, we can return some fields as a set of records
(one row for each subcommand) as we do in
pg_event_trigger_ddl_commands().
--
With Regards,
Amit Kapila.
On Fri, Jul 22, 2022 at 02:26:02PM +0530, Amit Kapila wrote:
Yeah, that would be a good idea but I think instead of changing
get_altertable_subcmdtypes(), can we have a new function say
get_altertable_subcmdinfo() that returns additional information from
address. The other alternative could be that instead of returning the
address as a string, we can return some fields as a set of records
(one row for each subcommand) as we do in
pg_event_trigger_ddl_commands().
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?
--
Michael
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
Attachments:
v2-0001-Collect-ObjectAddress-for-ATTACH-DETACH-PARTITION.patchtext/x-diff; charset=us-asciiDownload
From df118fedd1204ac4596de59ad2ef87cb1f734a35 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.fnst@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
v2-0002-Extend-test_ddl_deparse-for-ALTER-TABLE-.-ATTACH-.patchtext/x-diff; charset=us-asciiDownload
From a013fb2948986302490f09c5e44a3b510030f293 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@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
On Sat, Jul 23, 2022 at 4:28 PM Michael Paquier <michael@paquier.xyz> wrote:
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.
What I intended to say is similar to what you have done in the patch
but in a new function. OTOH, your point that it is okay to change
function signature/name in the test module seems reasonable to me.
--
With Regards,
Amit Kapila.
On Mon, Jul 25, 2022 at 08:42:18AM +0530, Amit Kapila wrote:
What I intended to say is similar to what you have done in the patch
but in a new function. OTOH, your point that it is okay to change
function signature/name in the test module seems reasonable to me.
Thanks. Let's do with the function change then. As introduced
orginally in b488c58, it returns an array that gets just unnested
once, so I'd like to think that it had better be a SRF from the
start but things are what they are.
--
Michael
On Saturday, July 23, 2022 6:58 PM Michael Paquier <michael@paquier.xyz> wrote:
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.
Thanks for the patches. The patches look good to me.
BTW, while reviewing it, I found there are some more subcommands that the
get_altertable_subcmdtypes() doesn't handle(e.g., ADD/DROP/SET IDENTITY and re ADD
STAT). Shall we fix them all while on it ?
Attach a minor patch to fix those which is based on the v2 patch set.
Best regards,
Hou zj
Attachments:
v2-0003-Add-support-for-some-missed-commands-in-test_ddl_dep_patchapplication/octet-stream; name=v2-0003-Add-support-for-some-missed-commands-in-test_ddl_dep_patchDownload
From 1f8ff61d92b97e168f8c418d2833be92fa68a9a1 Mon Sep 17 00:00:00 2001
From: "Hou Zhijie" <houzj.fnst@cn.fujitsu.com>
Date: Mon, 25 Jul 2022 11:30:18 +0800
Subject: [PATCH] Add support for some missed commands in test_ddl_deparse
---
.../test_ddl_deparse/expected/alter_table.out | 25 +++++++++++++++++++
.../test_ddl_deparse/sql/alter_table.sql | 12 +++++++++
.../test_ddl_deparse/test_ddl_deparse.c | 12 +++++++++
3 files changed, 49 insertions(+)
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 ec22d688b1..a2a4fcc0bc 100644
--- a/src/test/modules/test_ddl_deparse/expected/alter_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out
@@ -36,3 +36,28 @@ NOTICE: DDL test: type alter table, tag ALTER TABLE
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
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET NOT NULL desc column a of table parent
+NOTICE: subcommand: type SET NOT NULL desc column a of table child
+NOTICE: subcommand: type SET NOT NULL desc column a of table grandchild
+ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY;
+NOTICE: DDL test: type simple, tag CREATE SEQUENCE
+NOTICE: DDL test: type simple, tag ALTER SEQUENCE
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ADD IDENTITY desc column a of table parent
+ALTER TABLE parent ALTER COLUMN a SET GENERATED BY DEFAULT;
+NOTICE: DDL test: type simple, tag ALTER SEQUENCE
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET IDENTITY desc column a of table parent
+ALTER TABLE parent ALTER COLUMN a DROP IDENTITY;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DROP IDENTITY desc column a of table parent
+CREATE STATISTICS parent_stat (dependencies) ON a, c FROM parent;
+NOTICE: DDL test: type simple, tag CREATE STATISTICS
+ALTER TABLE parent ALTER COLUMN c TYPE numeric;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table parent
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table child
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table grandchild
+NOTICE: subcommand: type (re) ADD STATS desc statistics object parent_stat
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 b0989570d5..f663347cc1 100644
--- a/src/test/modules/test_ddl_deparse/sql/alter_table.sql
+++ b/src/test/modules/test_ddl_deparse/sql/alter_table.sql
@@ -24,3 +24,15 @@ ALTER TABLE part DETACH PARTITION part2;
DROP TABLE part2;
ALTER TABLE part ADD PRIMARY KEY (a);
+
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+
+ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY;
+
+ALTER TABLE parent ALTER COLUMN a SET GENERATED BY DEFAULT;
+
+ALTER TABLE parent ALTER COLUMN a DROP IDENTITY;
+
+CREATE STATISTICS parent_stat (dependencies) ON a, c FROM parent;
+
+ALTER TABLE parent ALTER COLUMN c TYPE numeric;
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 4476c8a90e..2296410388 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -300,6 +300,18 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
case AT_DetachPartitionFinalize:
strtype = "DETACH PARTITION ... FINALIZE";
break;
+ case AT_AddIdentity:
+ strtype = "ADD IDENTITY";
+ break;
+ case AT_SetIdentity:
+ strtype = "SET IDENTITY";
+ break;
+ case AT_DropIdentity:
+ strtype = "DROP IDENTITY";
+ break;
+ case AT_ReAddStatistics:
+ strtype = "(re) ADD STATS";
+ break;
default:
strtype = "unrecognized";
break;
--
2.18.4
On 2022-Jul-25, houzj.fnst@fujitsu.com wrote:
BTW, while reviewing it, I found there are some more subcommands that the
get_altertable_subcmdtypes() doesn't handle(e.g., ADD/DROP/SET IDENTITY and re ADD
STAT). Shall we fix them all while on it ?Attach a minor patch to fix those which is based on the v2 patch set.
Yeah, I suppose these are all commands that were added after the last
serious round of event trigger hacking, so it would be good to have
everything on board.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"En las profundidades de nuestro inconsciente hay una obsesiva necesidad
de un universo lógico y coherente. Pero el universo real se halla siempre
un paso más allá de la lógica" (Irulan)
On Mon, Jul 25, 2022 at 09:25:07AM +0000, houzj.fnst@fujitsu.com wrote:
BTW, while reviewing it, I found there are some more subcommands that the
get_altertable_subcmdtypes() doesn't handle(e.g., ADD/DROP/SET IDENTITY and re ADD
STAT). Shall we fix them all while on it ?Attach a minor patch to fix those which is based on the v2 patch set.
@@ -300,6 +300,18 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
[ ... ]
default:
strtype = "unrecognized";
break;
Removing the "default" clause would help here as we would get compiler
warnings if there is anything missing. One way to do things is to set
strtype to NULL before going through the switch and have a failsafe as
some commands are internal so they may not be worth adding to the
output.
--
Michael
On Monday, July 25, 2022 6:26 PM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Jul 25, 2022 at 09:25:07AM +0000, houzj.fnst@fujitsu.com wrote:
BTW, while reviewing it, I found there are some more subcommands that
the
get_altertable_subcmdtypes() doesn't handle(e.g., ADD/DROP/SET
IDENTITY and re ADD STAT). Shall we fix them all while on it ?Attach a minor patch to fix those which is based on the v2 patch set.
@@ -300,6 +300,18 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
[ ... ]
default:
strtype = "unrecognized";
break;Removing the "default" clause would help here as we would get compiler
warnings if there is anything missing. One way to do things is to set strtype to
NULL before going through the switch and have a failsafe as some commands
are internal so they may not be worth adding to the output.
Thanks for the suggestion. I have removed the default and found some missed
subcommands in 0003 patch. Attach the new version patch here
(The 0001 and 0002 is unchanged).
Best regards,
Hou zj
Attachments:
v3-0003-Add-support-for-some-missed-commands-in-test_ddl_.patchapplication/octet-stream; name=v3-0003-Add-support-for-some-missed-commands-in-test_ddl_.patchDownload
From 10240299a0dc7fdda818c4449380ca4d23c5aa24 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.fnst@cn.fujitsu.com>
Date: Mon, 25 Jul 2022 11:30:18 +0800
Subject: [PATCH v3 3/3] Add support for some missed commands in
test_ddl_deparse
---
.../test_ddl_deparse/expected/alter_table.out | 49 ++++++++++++++++++++++
.../modules/test_ddl_deparse/sql/alter_table.sql | 28 +++++++++++++
.../modules/test_ddl_deparse/test_ddl_deparse.c | 30 +++++++++++--
3 files changed, 104 insertions(+), 3 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 ec22d68..7b0cec5 100644
--- a/src/test/modules/test_ddl_deparse/expected/alter_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out
@@ -36,3 +36,52 @@ NOTICE: DDL test: type alter table, tag ALTER TABLE
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
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET NOT NULL desc column a of table parent
+NOTICE: subcommand: type SET NOT NULL desc column a of table child
+NOTICE: subcommand: type SET NOT NULL desc column a of table grandchild
+ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY;
+NOTICE: DDL test: type simple, tag CREATE SEQUENCE
+NOTICE: DDL test: type simple, tag ALTER SEQUENCE
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ADD IDENTITY desc column a of table parent
+ALTER TABLE parent ALTER COLUMN a SET GENERATED BY DEFAULT;
+NOTICE: DDL test: type simple, tag ALTER SEQUENCE
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET IDENTITY desc column a of table parent
+ALTER TABLE parent ALTER COLUMN a DROP IDENTITY;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DROP IDENTITY desc column a of table parent
+CREATE STATISTICS parent_stat (dependencies) ON a, c FROM parent;
+NOTICE: DDL test: type simple, tag CREATE STATISTICS
+ALTER TABLE parent ALTER COLUMN c TYPE numeric;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table parent
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table child
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table grandchild
+NOTICE: subcommand: type (re) ADD STATS desc statistics object parent_stat
+CREATE TABLE tbl (
+ a int generated always as (b::int * 2) stored,
+ b text
+);
+NOTICE: DDL test: type simple, tag CREATE TABLE
+ALTER TABLE tbl ALTER COLUMN a DROP EXPRESSION;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DROP EXPRESSION desc column a of table tbl
+ALTER TABLE tbl ALTER COLUMN b SET COMPRESSION pglz;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET COMPRESSION desc column b of table tbl
+CREATE TYPE comptype AS (r float8);
+NOTICE: DDL test: type simple, tag CREATE TYPE
+CREATE DOMAIN dcomptype AS comptype;
+NOTICE: DDL test: type simple, tag CREATE DOMAIN
+ALTER DOMAIN dcomptype ADD CONSTRAINT c1 check ((value).r > 0);
+NOTICE: DDL test: type simple, tag ALTER DOMAIN
+ALTER TYPE comptype ALTER ATTRIBUTE r TYPE bigint;
+NOTICE: DDL test: type alter table, tag ALTER TYPE
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column r of composite type comptype
+NOTICE: subcommand: type (re) ADD DOMAIN CONSTRAINT desc type dcomptype
+ALTER TABLE tbl SET ACCESS METHOD heap;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET ACCESS METHOD desc <NULL>
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 b098957..6c3d577 100644
--- a/src/test/modules/test_ddl_deparse/sql/alter_table.sql
+++ b/src/test/modules/test_ddl_deparse/sql/alter_table.sql
@@ -24,3 +24,31 @@ ALTER TABLE part DETACH PARTITION part2;
DROP TABLE part2;
ALTER TABLE part ADD PRIMARY KEY (a);
+
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+
+ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY;
+
+ALTER TABLE parent ALTER COLUMN a SET GENERATED BY DEFAULT;
+
+ALTER TABLE parent ALTER COLUMN a DROP IDENTITY;
+
+CREATE STATISTICS parent_stat (dependencies) ON a, c FROM parent;
+
+ALTER TABLE parent ALTER COLUMN c TYPE numeric;
+
+CREATE TABLE tbl (
+ a int generated always as (b::int * 2) stored,
+ b text
+);
+
+ALTER TABLE tbl ALTER COLUMN a DROP EXPRESSION;
+
+ALTER TABLE tbl ALTER COLUMN b SET COMPRESSION pglz;
+
+CREATE TYPE comptype AS (r float8);
+CREATE DOMAIN dcomptype AS comptype;
+ALTER DOMAIN dcomptype ADD CONSTRAINT c1 check ((value).r > 0);
+ALTER TYPE comptype ALTER ATTRIBUTE r TYPE bigint;
+
+ALTER TABLE tbl SET ACCESS METHOD heap;
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 4476c8a..0ad9b7a 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -102,7 +102,7 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
{
CollectedATSubcmd *sub = lfirst(cell);
AlterTableCmd *subcmd = castNode(AlterTableCmd, sub->parsetree);
- const char *strtype;
+ const char *strtype = NULL;
Datum values[2];
bool nulls[2];
@@ -132,6 +132,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
case AT_SetNotNull:
strtype = "SET NOT NULL";
break;
+ case AT_DropExpression:
+ strtype = "DROP EXPRESSION";
+ break;
case AT_CheckNotNull:
strtype = "CHECK NOT NULL";
break;
@@ -147,6 +150,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
case AT_SetStorage:
strtype = "SET STORAGE";
break;
+ case AT_SetCompression:
+ strtype = "SET COMPRESSION";
+ break;
case AT_DropColumn:
strtype = "DROP COLUMN";
break;
@@ -168,6 +174,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
case AT_ReAddConstraint:
strtype = "(re) ADD CONSTRAINT";
break;
+ case AT_ReAddDomainConstraint:
+ strtype = "(re) ADD DOMAIN CONSTRAINT";
+ break;
case AT_AlterConstraint:
strtype = "ALTER CONSTRAINT";
break;
@@ -213,6 +222,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
case AT_DropOids:
strtype = "DROP OIDS";
break;
+ case AT_SetAccessMethod:
+ strtype = "SET ACCESS METHOD";
+ break;
case AT_SetTableSpace:
strtype = "SET TABLESPACE";
break;
@@ -300,11 +312,23 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
case AT_DetachPartitionFinalize:
strtype = "DETACH PARTITION ... FINALIZE";
break;
- default:
- strtype = "unrecognized";
+ case AT_AddIdentity:
+ strtype = "ADD IDENTITY";
+ break;
+ case AT_SetIdentity:
+ strtype = "SET IDENTITY";
+ break;
+ case AT_DropIdentity:
+ strtype = "DROP IDENTITY";
+ break;
+ case AT_ReAddStatistics:
+ strtype = "(re) ADD STATS";
break;
}
+ if (strtype == NULL)
+ strtype = "unrecognized";
+
values[0] = CStringGetTextDatum(strtype);
if (OidIsValid(sub->address.objectId))
{
--
2.7.2.windows.1
v3-0001-Collect-ObjectAddress-for-ATTACH-DETACH-PARTITION.patchapplication/octet-stream; name=v3-0001-Collect-ObjectAddress-for-ATTACH-DETACH-PARTITION.patchDownload
From 331c8a4d180da1aee89d0edac0e7761af7cf2710 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.fnst@cn.fujitsu.com>
Date: Wed, 13 Jul 2022 17:39:13 +0800
Subject: [PATCH v3 1/3] 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 7fbee0c..9cf4671 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.7.2.windows.1
v3-0002-Extend-test_ddl_deparse-for-ALTER-TABLE-.-ATTACH-.patchapplication/octet-stream; name=v3-0002-Extend-test_ddl_deparse-for-ALTER-TABLE-.-ATTACH-.patchDownload
From 99aa0f6a11505267f9c7bbd319215f2388ed9289 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Sat, 23 Jul 2022 19:53:07 +0900
Subject: [PATCH v3 2/3] Extend test_ddl_deparse for ALTER TABLE ..
ATTACH/DETACH PARTITION
---
.../test_ddl_deparse/expected/alter_table.out | 19 ++++++---
.../test_ddl_deparse/expected/create_table.out | 8 ++--
.../test_ddl_deparse/expected/create_view.out | 2 +-
.../test_ddl_deparse/expected/test_ddl_deparse.out | 4 +-
.../modules/test_ddl_deparse/sql/alter_table.sql | 5 +++
.../test_ddl_deparse/sql/test_ddl_deparse.sql | 4 +-
.../test_ddl_deparse/test_ddl_deparse--1.0.sql | 6 ++-
.../modules/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 141060f..ec22d68 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 0f2a2c1..2178ce8 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 2ae4e2d..4ae0f49 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 4a5ea9e..fc657ff 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 dec53a0..b098957 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 e257a21..a471615 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 093005a..861d843 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 9476c3f..4476c8a 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;
- if (astate == NULL)
- elog(ERROR, "empty alter table subcommand list");
+ tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
+ }
- PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate, CurrentMemoryContext));
+ return (Datum) 0;
}
--
2.7.2.windows.1
On Tue, Jul 26, 2022 at 01:00:41PM +0000, houzj.fnst@fujitsu.com wrote:
Thanks for the suggestion. I have removed the default and found some missed
subcommands in 0003 patch. Attach the new version patch here
(The 0001 and 0002 is unchanged).
I have reviewed what you have here, and I found that the change is too
timid, with a coverage of 32% for test_ddl_deparse. Attached is an
updated patch, that provides coverage for the most obvious cases I
could see in tablecmds.c, bringing the coverage to 64% here.
Some cases are straight-forward, like the four cases for RLS or the
three subcases for RelOptions (where we'd better return an address
even if doing doing for the replace case). Some cases that I have not
included here would need more thoughts, like constraint validation and
drop or even SET ACCESS METHOD, so I have discarded for now all the
cases where we don't (or cannot) report properly an ObjectAddress
yet.
There is also a fancy case with DROP COLUMN, where we get an
ObjectAddress referring to the column already dropped, aka roughly a
".....pg_dropped.N.....", and it is not like we should switch to only
a reference of the table here because we want to know the name of the
column dropped. I have discarded this last one as well, for now.
All that could be expanded in more patches (triggers are an easy one),
but what I have here is already a good cut.
--
Michael
Attachments:
v4-0001-Improve-coverage-for-test_ddl_deparse.patchtext/x-diff; charset=us-asciiDownload
From a386086af62c661f0e41a8789266e0fc13579d83 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Sat, 30 Jul 2022 15:54:38 +0900
Subject: [PATCH v4] Improve coverage for test_ddl_deparse
---
src/backend/commands/tablecmds.c | 56 +++++---
.../test_ddl_deparse/expected/alter_table.out | 122 +++++++++++++++++-
.../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 | 54 ++++++++
.../test_ddl_deparse/sql/test_ddl_deparse.sql | 4 +-
.../test_ddl_deparse--1.0.sql | 6 +-
.../test_ddl_deparse/test_ddl_deparse.c | 73 +++++++++--
9 files changed, 279 insertions(+), 50 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e7aef2f6b0..ef95332d95 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -576,7 +576,7 @@ static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
const char *tablespacename, LOCKMODE lockmode);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace);
-static void ATExecSetRelOptions(Relation rel, List *defList,
+static ObjectAddress ATExecSetRelOptions(Relation rel, List *defList,
AlterTableType operation,
LOCKMODE lockmode);
static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
@@ -592,8 +592,8 @@ static ObjectAddress ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKM
static void ATExecDropOf(Relation rel, LOCKMODE lockmode);
static void ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode);
static void ATExecGenericOptions(Relation rel, List *options);
-static void ATExecSetRowSecurity(Relation rel, bool rls);
-static void ATExecForceNoForceRowSecurity(Relation rel, bool force_rls);
+static ObjectAddress ATExecSetRowSecurity(Relation rel, bool rls);
+static ObjectAddress ATExecForceNoForceRowSecurity(Relation rel, bool force_rls);
static ObjectAddress ATExecSetCompression(Relation rel,
const char *column, Node *newValue, LOCKMODE lockmode);
@@ -5115,7 +5115,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
case AT_SetRelOptions: /* SET (...) */
case AT_ResetRelOptions: /* RESET (...) */
case AT_ReplaceRelOptions: /* replace entire option list */
- ATExecSetRelOptions(rel, (List *) cmd->def, cmd->subtype, lockmode);
+ address = ATExecSetRelOptions(rel, (List *) cmd->def, cmd->subtype, lockmode);
break;
case AT_EnableTrig: /* ENABLE TRIGGER name */
ATExecEnableDisableTrigger(rel, cmd->name,
@@ -5183,16 +5183,16 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
ATExecReplicaIdentity(rel, (ReplicaIdentityStmt *) cmd->def, lockmode);
break;
case AT_EnableRowSecurity:
- ATExecSetRowSecurity(rel, true);
+ address = ATExecSetRowSecurity(rel, true);
break;
case AT_DisableRowSecurity:
- ATExecSetRowSecurity(rel, false);
+ address = ATExecSetRowSecurity(rel, false);
break;
case AT_ForceRowSecurity:
- ATExecForceNoForceRowSecurity(rel, true);
+ address = ATExecForceNoForceRowSecurity(rel, true);
break;
case AT_NoForceRowSecurity:
- ATExecForceNoForceRowSecurity(rel, false);
+ address = ATExecForceNoForceRowSecurity(rel, false);
break;
case AT_GenericOptions:
ATExecGenericOptions(rel, (List *) cmd->def);
@@ -5202,11 +5202,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,
@@ -5214,12 +5214,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",
@@ -14102,7 +14102,7 @@ ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacen
/*
* Set, reset, or replace reloptions.
*/
-static void
+static ObjectAddress
ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
LOCKMODE lockmode)
{
@@ -14117,9 +14117,13 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
bool repl_null[Natts_pg_class];
bool repl_repl[Natts_pg_class];
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
+ ObjectAddress address;
if (defList == NIL && operation != AT_ReplaceRelOptions)
- return; /* nothing to do */
+ {
+ ObjectAddressSet(address, RelationRelationId, RelationGetRelid(rel));
+ return address; /* nothing to do */
+ }
pgclass = table_open(RelationRelationId, RowExclusiveLock);
@@ -14298,7 +14302,9 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
table_close(toastrel, NoLock);
}
+ ObjectAddressSet(address, RelationRelationId, RelationGetRelid(rel));
table_close(pgclass, RowExclusiveLock);
+ return address;
}
/*
@@ -15992,12 +15998,13 @@ ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode
/*
* ALTER TABLE ENABLE/DISABLE ROW LEVEL SECURITY
*/
-static void
+static ObjectAddress
ATExecSetRowSecurity(Relation rel, bool rls)
{
Relation pg_class;
Oid relid;
HeapTuple tuple;
+ ObjectAddress address;
relid = RelationGetRelid(rel);
@@ -16012,19 +16019,24 @@ ATExecSetRowSecurity(Relation rel, bool rls)
((Form_pg_class) GETSTRUCT(tuple))->relrowsecurity = rls;
CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+ ObjectAddressSet(address, RelationRelationId, relid);
+
table_close(pg_class, RowExclusiveLock);
heap_freetuple(tuple);
+
+ return address;
}
/*
* ALTER TABLE FORCE/NO FORCE ROW LEVEL SECURITY
*/
-static void
+static ObjectAddress
ATExecForceNoForceRowSecurity(Relation rel, bool force_rls)
{
Relation pg_class;
Oid relid;
HeapTuple tuple;
+ ObjectAddress address;
relid = RelationGetRelid(rel);
@@ -16038,8 +16050,12 @@ ATExecForceNoForceRowSecurity(Relation rel, bool force_rls)
((Form_pg_class) GETSTRUCT(tuple))->relforcerowsecurity = force_rls;
CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+ ObjectAddressSet(address, RelationRelationId, relid);
+
table_close(pg_class, RowExclusiveLock);
heap_freetuple(tuple);
+
+ return address;
}
/*
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..14151fe021 100644
--- a/src/test/modules/test_ddl_deparse/expected/alter_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out
@@ -2,6 +2,18 @@ CREATE TABLE parent (
a int
);
NOTICE: DDL test: type simple, tag CREATE TABLE
+ALTER TABLE parent SET (fillfactor = 50);
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET RELOPTIONS desc table parent
+ALTER TABLE parent RESET (fillfactor);
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type RESET RELOPTIONS desc table parent
+CREATE INDEX parent_index ON parent(a);
+NOTICE: DDL test: type simple, tag CREATE INDEX
+ALTER TABLE parent CLUSTER ON parent_index;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type CLUSTER desc index parent_index
+DROP INDEX parent_index;
CREATE TABLE child () INHERITS (parent);
NOTICE: DDL test: type simple, tag CREATE TABLE
CREATE TABLE grandchild () INHERITS (child);
@@ -9,21 +21,119 @@ 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);
+-- Constraint, no recursion
+ALTER TABLE ONLY grandchild 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 desc constraint a_pos on table grandchild
+-- Constraint, with recursion
+ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
+NOTICE: merging constraint "a_pos" with inherited definition
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+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
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET NOT NULL desc column a of table parent
+NOTICE: subcommand: type SET NOT NULL desc column a of table child
+NOTICE: subcommand: type SET NOT NULL desc column a of table grandchild
+ALTER TABLE parent ALTER COLUMN a DROP NOT NULL;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DROP NOT NULL desc column a of table parent
+NOTICE: subcommand: type DROP NOT NULL desc column a of table child
+NOTICE: subcommand: type DROP NOT NULL desc column a of table grandchild
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET NOT NULL desc column a of table parent
+NOTICE: subcommand: type SET NOT NULL desc column a of table child
+NOTICE: subcommand: type SET NOT NULL desc column a of table grandchild
+ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY;
+NOTICE: DDL test: type simple, tag CREATE SEQUENCE
+NOTICE: DDL test: type simple, tag ALTER SEQUENCE
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ADD IDENTITY desc column a of table parent
+ALTER TABLE parent ALTER COLUMN a SET GENERATED BY DEFAULT;
+NOTICE: DDL test: type simple, tag ALTER SEQUENCE
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET IDENTITY desc column a of table parent
+ALTER TABLE parent ALTER COLUMN a DROP IDENTITY;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DROP IDENTITY desc column a of table parent
+ALTER TABLE parent ALTER COLUMN a SET STATISTICS 100;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET STATS desc column a of table parent
+NOTICE: subcommand: type SET STATS desc column a of table child
+NOTICE: subcommand: type SET STATS desc column a of table grandchild
+ALTER TABLE parent ALTER COLUMN a SET STORAGE PLAIN;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET STORAGE desc column a of table parent
+NOTICE: subcommand: type SET STORAGE desc column a of table child
+NOTICE: subcommand: type SET STORAGE desc column a of table grandchild
+ALTER TABLE parent ENABLE ROW LEVEL SECURITY;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ENABLE ROW SECURITY desc table parent
+ALTER TABLE parent NO FORCE ROW LEVEL SECURITY;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type NO FORCE ROW SECURITY desc table parent
+ALTER TABLE parent FORCE ROW LEVEL SECURITY;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type FORCE ROW SECURITY desc table parent
+ALTER TABLE parent DISABLE ROW LEVEL SECURITY;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DISABLE ROW SECURITY desc table parent
+CREATE STATISTICS parent_stat (dependencies) ON a, c FROM parent;
+NOTICE: DDL test: type simple, tag CREATE STATISTICS
+ALTER TABLE parent ALTER COLUMN c TYPE numeric;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table parent
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table child
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table grandchild
+NOTICE: subcommand: type (re) ADD STATS desc statistics object parent_stat
+ALTER TABLE parent ALTER COLUMN c SET DEFAULT 0;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type ALTER COLUMN SET DEFAULT desc column c of table parent
+NOTICE: subcommand: type ALTER COLUMN SET DEFAULT desc column c of table child
+NOTICE: subcommand: type ALTER COLUMN SET DEFAULT desc column c of table grandchild
+CREATE TABLE tbl (
+ a int generated always as (b::int * 2) stored,
+ b text
+);
+NOTICE: DDL test: type simple, tag CREATE TABLE
+ALTER TABLE tbl ALTER COLUMN a DROP EXPRESSION;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type DROP EXPRESSION desc column a of table tbl
+ALTER TABLE tbl ALTER COLUMN b SET COMPRESSION pglz;
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: type SET COMPRESSION desc column b of table tbl
+CREATE TYPE comptype AS (r float8);
+NOTICE: DDL test: type simple, tag CREATE TYPE
+CREATE DOMAIN dcomptype AS comptype;
+NOTICE: DDL test: type simple, tag CREATE DOMAIN
+ALTER DOMAIN dcomptype ADD CONSTRAINT c1 check ((value).r > 0);
+NOTICE: DDL test: type simple, tag ALTER DOMAIN
+ALTER TYPE comptype ALTER ATTRIBUTE r TYPE bigint;
+NOTICE: DDL test: type alter table, tag ALTER TYPE
+NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column r of composite type comptype
+NOTICE: subcommand: type (re) ADD DOMAIN CONSTRAINT desc type dcomptype
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..cf5d711f99 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 view static_view
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..24a5b9b3bf 100644
--- a/src/test/modules/test_ddl_deparse/sql/alter_table.sql
+++ b/src/test/modules/test_ddl_deparse/sql/alter_table.sql
@@ -2,6 +2,13 @@ CREATE TABLE parent (
a int
);
+ALTER TABLE parent SET (fillfactor = 50);
+ALTER TABLE parent RESET (fillfactor);
+
+CREATE INDEX parent_index ON parent(a);
+ALTER TABLE parent CLUSTER ON parent_index;
+DROP INDEX parent_index;
+
CREATE TABLE child () INHERITS (parent);
CREATE TABLE grandchild () INHERITS (child);
@@ -10,6 +17,9 @@ ALTER TABLE parent ADD COLUMN b serial;
ALTER TABLE parent RENAME COLUMN b TO c;
+-- Constraint, no recursion
+ALTER TABLE ONLY grandchild ADD CONSTRAINT a_pos CHECK (a > 0);
+-- Constraint, with recursion
ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
CREATE TABLE part (
@@ -18,4 +28,48 @@ 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);
+
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+ALTER TABLE parent ALTER COLUMN a DROP NOT NULL;
+ALTER TABLE parent ALTER COLUMN a SET NOT NULL;
+
+ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY;
+
+ALTER TABLE parent ALTER COLUMN a SET GENERATED BY DEFAULT;
+
+ALTER TABLE parent ALTER COLUMN a DROP IDENTITY;
+
+ALTER TABLE parent ALTER COLUMN a SET STATISTICS 100;
+
+ALTER TABLE parent ALTER COLUMN a SET STORAGE PLAIN;
+
+ALTER TABLE parent ENABLE ROW LEVEL SECURITY;
+ALTER TABLE parent NO FORCE ROW LEVEL SECURITY;
+ALTER TABLE parent FORCE ROW LEVEL SECURITY;
+ALTER TABLE parent DISABLE ROW LEVEL SECURITY;
+
+CREATE STATISTICS parent_stat (dependencies) ON a, c FROM parent;
+
+ALTER TABLE parent ALTER COLUMN c TYPE numeric;
+
+ALTER TABLE parent ALTER COLUMN c SET DEFAULT 0;
+
+CREATE TABLE tbl (
+ a int generated always as (b::int * 2) stored,
+ b text
+);
+
+ALTER TABLE tbl ALTER COLUMN a DROP EXPRESSION;
+
+ALTER TABLE tbl ALTER COLUMN b SET COMPRESSION pglz;
+
+CREATE TYPE comptype AS (r float8);
+CREATE DOMAIN dcomptype AS comptype;
+ALTER DOMAIN dcomptype ADD CONSTRAINT c1 check ((value).r > 0);
+ALTER TYPE comptype ALTER ATTRIBUTE r TYPE bigint;
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..bdbe05ceeb 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;
+ const char *strtype = "unrecognized";
+ Datum values[2];
+ bool nulls[2];
+
+ memset(values, 0, sizeof(values));
+ memset(nulls, 0, sizeof(nulls));
switch (subcmd->subtype)
{
@@ -120,6 +132,9 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
case AT_SetNotNull:
strtype = "SET NOT NULL";
break;
+ case AT_DropExpression:
+ strtype = "DROP EXPRESSION";
+ break;
case AT_CheckNotNull:
strtype = "CHECK NOT NULL";
break;
@@ -135,6 +150,9 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
case AT_SetStorage:
strtype = "SET STORAGE";
break;
+ case AT_SetCompression:
+ strtype = "SET COMPRESSION";
+ break;
case AT_DropColumn:
strtype = "DROP COLUMN";
break;
@@ -156,6 +174,9 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
case AT_ReAddConstraint:
strtype = "(re) ADD CONSTRAINT";
break;
+ case AT_ReAddDomainConstraint:
+ strtype = "(re) ADD DOMAIN CONSTRAINT";
+ break;
case AT_AlterConstraint:
strtype = "ALTER CONSTRAINT";
break;
@@ -201,6 +222,9 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
case AT_DropOids:
strtype = "DROP OIDS";
break;
+ case AT_SetAccessMethod:
+ strtype = "SET ACCESS METHOD";
+ break;
case AT_SetTableSpace:
strtype = "SET TABLESPACE";
break;
@@ -279,18 +303,41 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
case AT_GenericOptions:
strtype = "SET OPTIONS";
break;
- default:
- strtype = "unrecognized";
+ case AT_DetachPartition:
+ strtype = "DETACH PARTITION";
+ break;
+ case AT_AttachPartition:
+ strtype = "ATTACH PARTITION";
+ break;
+ case AT_DetachPartitionFinalize:
+ strtype = "DETACH PARTITION ... FINALIZE";
+ break;
+ case AT_AddIdentity:
+ strtype = "ADD IDENTITY";
+ break;
+ case AT_SetIdentity:
+ strtype = "SET IDENTITY";
+ break;
+ case AT_DropIdentity:
+ strtype = "DROP IDENTITY";
+ break;
+ case AT_ReAddStatistics:
+ strtype = "(re) ADD STATS";
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
On Saturday, July 30, 2022 3:15 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jul 26, 2022 at 01:00:41PM +0000, houzj.fnst@fujitsu.com wrote:
Thanks for the suggestion. I have removed the default and found some
missed subcommands in 0003 patch. Attach the new version patch here
(The 0001 and 0002 is unchanged).I have reviewed what you have here, and I found that the change is too timid,
with a coverage of 32% for test_ddl_deparse. Attached is an updated patch, that
provides coverage for the most obvious cases I could see in tablecmds.c,
bringing the coverage to 64% here.
Thanks ! the patch looks better now.
Some cases are straight-forward, like the four cases for RLS or the three
subcases for RelOptions (where we'd better return an address even if doing
doing for the replace case).
I am not against returning the objaddr for cases related to RLS and RelOption.
But just to confirm, do you have a use case to use the returned address(relation itself)
for RLS or RelOptions in event trigger ? I asked this because when I tried to
deparse the subcommand of ALTER TABLE. It seems enough to use the information
inside the parse tree to deparse the RLS and RelOptions related subcommands.
Best regards,
Hou Zhijie
On Sat, Jul 30, 2022 at 01:13:52PM +0000, houzj.fnst@fujitsu.com wrote:
I am not against returning the objaddr for cases related to RLS and RelOption.
But just to confirm, do you have a use case to use the returned address(relation itself)
for RLS or RelOptions in event trigger ? I asked this because when I tried to
deparse the subcommand of ALTER TABLE. It seems enough to use the information
inside the parse tree to deparse the RLS and RelOptions related subcommands.
You are right here, there is little point in returning the relation
itself. I have removed these modifications, added a couple of extra
commands for some extra coverage, and applied all that. I have
finished by splitting the extension of test_ddl_deparse/ and the
addition of ObjectAddress for the attach/detach into their own commit,
mainly for clarity.
--
Michael
On Sunday, July 31, 2022 12:12 PM Michael Paquier <michael@paquier.xyz> wrote:
On Sat, Jul 30, 2022 at 01:13:52PM +0000, houzj.fnst@fujitsu.com wrote:
I am not against returning the objaddr for cases related to RLS and
RelOption.
But just to confirm, do you have a use case to use the returned
address(relation itself) for RLS or RelOptions in event trigger ? I
asked this because when I tried to deparse the subcommand of ALTER
TABLE. It seems enough to use the information inside the parse tree todeparse the RLS and RelOptions related subcommands.
You are right here, there is little point in returning the relation itself. I have
removed these modifications, added a couple of extra commands for some
extra coverage, and applied all that. I have finished by splitting the extension
of test_ddl_deparse/ and the addition of ObjectAddress for the attach/detach
into their own commit, mainly for clarity.
Thanks!
Best regards,
Hou Zhijie