Hi, On Thu, Jun 8, 2023 at 9:12 PM shveta malik <shveta.ma...@gmail.com> wrote: > > > Please find new set of patches addressing below: > a) Issue mentioned by Wang-san in [1], > b) Comments from Peter given in [2] > c) Comments from Amit given in the last 2 emails. > > [1]: > https://www.postgresql.org/message-id/OS3PR01MB62750D43D4F7F075B33BD2609E52A%40OS3PR01MB6275.jpnprd01.prod.outlook.com > [2]: > https://www.postgresql.org/message-id/CAHut%2BPv9vPbUQc0fzrKmDkKOsS_bj-hup_E%2BsLHNEX%2B6F%2BSY5Q%40mail.gmail.com > > Thank You Vignesh for handling (a), Ajin for handling (b), Shi-san and > Hou-san for contributing in (c). >
I have some questions about DDL deparser: I've tested deparsed and reformed DDL statements with the following function and event trigger borrowed from the regression tests: CREATE OR REPLACE FUNCTION test_ddl_deparse() RETURNS event_trigger LANGUAGE plpgsql AS $$ DECLARE r record; deparsed_json text; BEGIN FOR r IN SELECT * FROM pg_event_trigger_ddl_commands() -- Some TABLE commands generate sequence-related commands, also deparse them. WHERE command_tag in ('ALTER FOREIGN TABLE', 'ALTER TABLE', 'CREATE FOREIGN TABLE', 'CREATE TABLE', 'CREATE TABLE AS', 'DROP FOREIGN TABLE', 'DROP TABLE', 'ALTER SEQUENCE', 'CREATE SEQUENCE', 'DROP SEQUENCE') LOOP deparsed_json = pg_catalog.ddl_deparse_to_json(r.command); RAISE NOTICE 'deparsed json: %', deparsed_json; RAISE NOTICE 're-formed command: %', pg_catalog.ddl_deparse_expand_command(deparsed_json); END LOOP; END; $$; CREATE EVENT TRIGGER test_ddl_deparse ON ddl_command_end EXECUTE PROCEDURE test_ddl_deparse(); The query "CREATE TABLE TEST AS SELECT 1" is deparsed and reformed by DDL deparse to: CREATE TABLE public.test ("?column?" pg_catalog.int4 STORAGE PLAIN ) I agree that we need to deparse CTAS in such a way for logical replication but IIUC DDL deparse feature (e.g., ddl_deparse_to_json() and ddl_deparse_expand_command()) is a generic feature in principle so I'm concerned that there might be users who want to get the DDL statement that is actually executed. If so, we might want to have a switch to get the actual DDL statement instead. Also, the table and column data type are schema qualified, but is there any reason why the reformed query doesn't explicitly specify tablespace, toast compression and access method? Since their default values depend on GUC parameters, the table could be created in a different tablespace on the subscriber if the subscriber set a different value to default_tablespace GUC parameter. IIUC the reason why we use schema-qualified names instead of sending along with search_path is to prevent tables from being created in a different schema depending on search_patch setting on the subscriber. So I wondered why we don't do that for other GUC-depends propety. --- I got a SEGV in the following case: =# create event trigger test_trigger ON ddl_command_start execute function publication_deparse_ddl_command_end(); CREATE EVENT TRIGGER =# create table t (); Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com