Thread
-
[PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-03T12:58:52Z
Hackers, Attached is a patch adding pg_get_table_ddl(regclass, VARIADIC text[]), extending the existing pg_get_database/role/tablespace_ddl family. relations. It returns the CREATE TABLE statement plus the follow-up ALTER TABLE, CREATE INDEX, CREATE RULE, and CREATE STATISTICS statements needed for a full reconstruction, one per row. *Coverage* -------- *Per-column:* typmod, COLLATE, STORAGE, COMPRESSION (pglz/lz4), GENERATED STORED/VIRTUAL, IDENTITY ALWAYS/BY DEFAULT (with non-default sequence options), DEFAULT, NOT NULL, attoptions. *Table-level:* UNLOGGED, INHERITS, PARTITION BY (RANGE/LIST/HASH), PARTITION OF ... FOR VALUES, USING access method, WITH (reloptions), TABLESPACE, inline CHECK. *Sub-objects:* Indexes, Constraints (PK/UNIQUE/FK/EXCLUDE/named NOT NULL), Rules, extended statistics, REPLICA IDENTITY, ENABLE/FORCE RLS, and child-local DEFAULT overrides. *Triggers and policies are TODOs pending pg_get_trigger_ddl (Phil's re-roll) and pg_get_policy_ddl (Waiting for review/commit)*. Options (pretty, owner, tablespace, and a family of *includes_** toggles for each sub-object class) let callers fine-tune the output. Every clause is omitted when its value equals what the system would reapply on round-trip same default-omission convention as the existing _ddl functions. I deliberately did *not* add pg_get_index_ddl / _constraint_ddl / _rule_ddl / _stat_ddl wrappers. The existing C helpers in ruleutils.c (pg_get_indexdef_string, pg_get_constraintdef_command, etc.) already emit reproducible statements, so pg_get_table_ddl_internal calls them directly. Happy to revisit if reviewers prefer separate SQL-level surfaces. *Testing: * pg_regress test at src/test/regress/sql/pg_get_table_ddl.sql covering the matrix above plus error paths. I kept it as pg_regress rather than TAP (where the sibling _ddl tests live) since exact-.out diffs are the more rigorous check for a deparse function, and the pg_regress restriction that drove the TAP choice for CREATE DATABASE/TABLESPACE doesn't apply to tables. A manual round-trip across IDENTITY/GENERATED/ STORAGE/COMPRESSION/PK produces an empty EXCEPT diff against the original. *Out of scope:* COMMENT ON, GRANT/REVOKE (matching the existing _ddl family), and typed tables (CREATE TABLE name OF type). Feedback is welcome, particularly on the option naming and the decision to reuse the C helpers directly rather than add new SQL wrappers. ----- Regards, Akshay Joshi Principal Engineer | Engineering Manager | pgAdmin Hacker EDB (EnterpriseDB)
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-08T11:30:32Z
Hi Hackers, Attached is the v2 patch, which fixes the Meson build failure, and the rebased patch. On Wed, Jun 3, 2026 at 6:28 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: > Hackers, > > Attached is a patch adding pg_get_table_ddl(regclass, VARIADIC text[]), > extending the existing pg_get_database/role/tablespace_ddl family. > relations. It returns the CREATE TABLE statement plus the follow-up ALTER > TABLE, CREATE INDEX, CREATE RULE, and CREATE STATISTICS statements needed > for a full reconstruction, one per row. > > *Coverage* > -------- > *Per-column:* typmod, COLLATE, STORAGE, COMPRESSION (pglz/lz4), GENERATED > STORED/VIRTUAL, IDENTITY ALWAYS/BY DEFAULT (with non-default sequence > options), DEFAULT, NOT NULL, attoptions. > > *Table-level:* UNLOGGED, INHERITS, PARTITION BY (RANGE/LIST/HASH), > PARTITION OF ... FOR VALUES, USING access method, WITH (reloptions), > TABLESPACE, inline CHECK. > > *Sub-objects:* Indexes, Constraints (PK/UNIQUE/FK/EXCLUDE/named NOT > NULL), Rules, extended statistics, REPLICA IDENTITY, ENABLE/FORCE RLS, and > child-local DEFAULT overrides. > *Triggers and policies are TODOs pending pg_get_trigger_ddl (Phil's > re-roll) and pg_get_policy_ddl (Waiting for review/commit)*. > > Options (pretty, owner, tablespace, and a family of *includes_** toggles > for each sub-object class) let callers fine-tune the output. Every clause > is omitted when its value equals what the system would reapply on > round-trip same default-omission convention as the existing _ddl functions. > > I deliberately did *not* add pg_get_index_ddl / _constraint_ddl / > _rule_ddl / _stat_ddl wrappers. The existing C helpers in ruleutils.c > (pg_get_indexdef_string, pg_get_constraintdef_command, etc.) already emit > reproducible statements, so pg_get_table_ddl_internal calls them directly. > Happy to revisit if reviewers prefer separate SQL-level surfaces. > > *Testing: * > pg_regress test at src/test/regress/sql/pg_get_table_ddl.sql covering the > matrix above plus error paths. I kept it as pg_regress rather than TAP > (where the sibling _ddl tests live) since exact-.out diffs are the more > rigorous check for a deparse function, and the pg_regress restriction that > drove the TAP choice for CREATE DATABASE/TABLESPACE doesn't apply to > tables. A manual round-trip across IDENTITY/GENERATED/ > STORAGE/COMPRESSION/PK produces an empty EXCEPT diff against the original. > > *Out of scope:* COMMENT ON, GRANT/REVOKE (matching the existing _ddl > family), and typed tables (CREATE TABLE name OF type). > > Feedback is welcome, particularly on the option naming and the decision to > reuse the C helpers directly rather than add new SQL wrappers. > > ----- > Regards, > Akshay Joshi > Principal Engineer | Engineering Manager | pgAdmin Hacker > EDB (EnterpriseDB) > >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-06-08T19:11:05Z
Em seg., 8 de jun. de 2026 às 08:30, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: > Hi Hackers, > > Attached is the v2 patch, which fixes the Meson build failure, and the > rebased patch. > Would be good to have an option "schema-qualified" boolean, because sometimes I don't want a schema-qualified result. Suppose you are duplicating a schema, the way your did you cannot do something like this to have a complete script to generate a new schema select 'create schema new_schema;' union all select 'set search_path to new_schema, public;' union all select string_agg(pg_get_table_ddl(oid),',') from pg_class where relnamespace::regnamespace::text = 'old_schema' and relkind = 'r'; regards Marcos
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-08T21:12:09Z
Hello! I did some basic testing with the new functions, and found a few bugs: 1. Seems like check constraints on partitions are ignored: CREATE TABLE p (id int, val int) PARTITION BY RANGE (id); CREATE TABLE p_child PARTITION OF p (CONSTRAINT chk_inline CHECK (val > 0)) FOR VALUES FROM (0) TO (100); SELECT * FROM pg_get_table_ddl('p_child', 'owner','false'); 2. inherited stored generated columns can't be replayed: CREATE TABLE par_s ( id int, g int GENERATED ALWAYS AS (id * 2) STORED ); CREATE TABLE ch_s () INHERITS (par_s); SELECT * FROM pg_get_table_ddl('ch_s', 'owner','false'); -- CREATE TABLE public.ch_s () INHERITS (public.par_s); -- ALTER TABLE public.ch_s ALTER COLUMN g SET DEFAULT (id * 2); Dropping ch_s, executing the returned statements: ERROR: column "g" of relation "ch_s" is a generated column HINT: Use ALTER TABLE ... ALTER COLUMN ... SET EXPRESSION instead. 3. named not null constraints can't be replayed: CREATE TABLE t (a int CONSTRAINT my_nn NOT NULL); SELECT * FROM pg_get_table_ddl('t'::regclass,'owner','false'); -- CREATE TABLE public.t ( a integer NOT NULL); -- ALTER TABLE public.t ADD CONSTRAINT my_nn NOT NULL a; Dropping t, executing the statements: ERROR: cannot create not-null constraint "my_nn" on column "a" of table "t" DETAIL: A not-null constraint named "t_a_not_null" already exists for this column. -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-09T08:58:15Z
Thanks to Zsolt and Marcos for the review. Added *schema_qualified (boolean, default true)*. When false, the target table is emitted unqualified everywhere (e.g., CREATE TABLE, ALTER TABLE, INHERITS, PARTITION OF, identity SEQUENCE NAME, etc.); same-schema sibling references follow suit; cross-schema references (e.g. FK targets in a different schema) remain qualified for correctness. Output from the always-qualified ruleutils helpers (pg_get_indexdef_string, pg_get_constraintdef_command, pg_get_ruledef, pg_get_statisticsobjdef_string) is post-processed to strip the base-schema prefix. 1) CHECK constraints on partition children are now emitted as ALTER TABLE … ADD CONSTRAINT … CHECK (…). They had been silently dropped because the PARTITION OF form has no column list to inline them into. 2) Inherited generated columns no longer emit a spurious ALTER COLUMN … SET DEFAULT, which would fail at replay. 3) User-named NOT NULL constraints are now emitted inline as CONSTRAINT <name> NOT NULL. Auto-named NOT NULLs keep the existing inline-NOT NULL + ALTER TABLE dedup behaviour so the common-case output is unchanged. 4) Added regression coverage for the three bug fixes plus the schema_qualified=false paths (same-schema vs cross-schema, INHERITS/PARTITION OF parents, custom identity sequence name, replay into a different target schema) Attached is the v3 patch, ready for review. On Tue, Jun 9, 2026 at 2:42 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > Hello! > > I did some basic testing with the new functions, and found a few bugs: > > 1. Seems like check constraints on partitions are ignored: > > CREATE TABLE p (id int, val int) PARTITION BY RANGE (id); > CREATE TABLE p_child PARTITION OF p (CONSTRAINT chk_inline CHECK (val > 0)) > FOR VALUES FROM (0) TO (100); > SELECT * FROM pg_get_table_ddl('p_child', 'owner','false'); > > 2. inherited stored generated columns can't be replayed: > > CREATE TABLE par_s ( > id int, > g int GENERATED ALWAYS AS (id * 2) STORED > ); > CREATE TABLE ch_s () INHERITS (par_s); > SELECT * FROM pg_get_table_ddl('ch_s', 'owner','false'); > -- CREATE TABLE public.ch_s () INHERITS (public.par_s); > -- ALTER TABLE public.ch_s ALTER COLUMN g SET DEFAULT (id * 2); > > Dropping ch_s, executing the returned statements: > > ERROR: column "g" of relation "ch_s" is a generated column > HINT: Use ALTER TABLE ... ALTER COLUMN ... SET EXPRESSION instead. > > 3. named not null constraints can't be replayed: > > CREATE TABLE t (a int CONSTRAINT my_nn NOT NULL); > SELECT * FROM pg_get_table_ddl('t'::regclass,'owner','false'); > -- CREATE TABLE public.t ( a integer NOT NULL); > -- ALTER TABLE public.t ADD CONSTRAINT my_nn NOT NULL a; > > Dropping t, executing the statements: > > ERROR: cannot create not-null constraint "my_nn" on column "a" of table > "t" > DETAIL: A not-null constraint named "t_a_not_null" already exists for > this column. > > > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-09T19:17:11Z
Thanks, I can confirm that the previous bugs were fixed, however the bugfixes also introduce a new issue, where inherited not null constraints are missing: CREATE TABLE par (a int); CREATE TABLE ch (b int) INHERITS (par); ALTER TABLE ch ADD CONSTRAINT my_nn NOT NULL a; SELECT * FROM pg_get_table_ddl('ch','owner','false'); -- CREATE TABLE public.ch ( b integer) INHERITS (public.par); The schema_qualified => false part also doesn't work as described: + <command>CREATE STATISTICS</command> statement. References to + objects in the same schema as the target table (inheritance + parents, partition parents, identity sequences, and any + same-schema object the deparse helpers happen to mention) are + also emitted unqualified, so the script can be replayed under a + different <varname>search_path</varname> to recreate the table + in another schema. but: CREATE SCHEMA s1; CREATE SEQUENCE s1.myseq; CREATE FUNCTION s1.f(int) RETURNS int LANGUAGE sql IMMUTABLE AS 'SELECT $1'; CREATE TABLE s1.t ( id int DEFAULT nextval('s1.myseq'), val int, CONSTRAINT chk CHECK (s1.f(val) > 0) ); SELECT * FROM pg_get_table_ddl('s1.t', 'owner','false', 'schema_qualified','false'); -- CREATE TABLE t ( id integer DEFAULT nextval('s1.myseq'::regclass), val integer, CONSTRAINT chk CHECK ((s1.f(val) > 0))); s1 appears twice in the output. It also has an issue with strings containing the schema: CREATE SCHEMA myschema; CREATE TABLE myschema.p (id int, note text) PARTITION BY RANGE (id); CREATE TABLE myschema.pc PARTITION OF myschema.p (CONSTRAINT chk CHECK (note <> 'myschema.secret')) FOR VALUES FROM (0) TO (100); SELECT * FROM pg_get_table_ddl('myschema.pc', 'owner','false', 'schema_qualified','false'); -- CREATE TABLE pc PARTITION OF p FOR VALUES FROM (0) TO (100); -- ALTER TABLE pc ADD CONSTRAINT chk CHECK ((note <> 'secret'::text)); -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-10T11:24:22Z
Thanks for the review. I have fixed all the issues you mentioned. Because this is a major restructuring of the DDL function involving multiple child elements and complex syntaxes, some edge cases might still pop up, though I have tried to cover as much as possible. The v4 patch is ready for your review. On Wed, Jun 10, 2026 at 12:47 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > Thanks, I can confirm that the previous bugs were fixed, however the > bugfixes also introduce a new issue, where inherited not null > constraints are missing: > > CREATE TABLE par (a int); > CREATE TABLE ch (b int) INHERITS (par); > ALTER TABLE ch ADD CONSTRAINT my_nn NOT NULL a; > SELECT * FROM pg_get_table_ddl('ch','owner','false'); > -- CREATE TABLE public.ch ( b integer) INHERITS (public.par); > > The schema_qualified => false part also doesn't work as described: > > + <command>CREATE STATISTICS</command> statement. References to > + objects in the same schema as the target table (inheritance > + parents, partition parents, identity sequences, and any > + same-schema object the deparse helpers happen to mention) are > + also emitted unqualified, so the script can be replayed under a > + different <varname>search_path</varname> to recreate the table > + in another schema. > > but: > > CREATE SCHEMA s1; > CREATE SEQUENCE s1.myseq; > CREATE FUNCTION s1.f(int) RETURNS int LANGUAGE sql IMMUTABLE AS 'SELECT > $1'; > CREATE TABLE s1.t ( > id int DEFAULT nextval('s1.myseq'), > val int, > CONSTRAINT chk CHECK (s1.f(val) > 0) > ); > SELECT * FROM pg_get_table_ddl('s1.t', 'owner','false', > 'schema_qualified','false'); > -- CREATE TABLE t ( id integer DEFAULT nextval('s1.myseq'::regclass), > val integer, CONSTRAINT chk CHECK ((s1.f(val) > 0))); > > s1 appears twice in the output. > > It also has an issue with strings containing the schema: > > CREATE SCHEMA myschema; > CREATE TABLE myschema.p (id int, note text) PARTITION BY RANGE (id); > CREATE TABLE myschema.pc PARTITION OF myschema.p > (CONSTRAINT chk CHECK (note <> 'myschema.secret')) FOR VALUES FROM > (0) TO (100); > SELECT * FROM pg_get_table_ddl('myschema.pc', 'owner','false', > 'schema_qualified','false'); > -- CREATE TABLE pc PARTITION OF p FOR VALUES FROM (0) TO (100); > -- ALTER TABLE pc ADD CONSTRAINT chk CHECK ((note <> 'secret'::text)); > > > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-10T20:43:41Z
Thanks for the update! The new version looks mostly good, I only found one corner case that doesn't work, double quoted literals can still get over-stripped: CREATE SCHEMA s; CREATE TABLE s.p (id int, "s.weird" int) PARTITION BY RANGE (id); CREATE TABLE s.pc PARTITION OF s.p (CONSTRAINT chk CHECK ("s.weird" > 0)) FOR VALUES FROM (0) TO (100); SELECT * FROM pg_get_table_ddl('s.pc', 'owner', 'false', 'schema_qualified', 'false'); -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-11T07:48:07Z
On Thu, Jun 11, 2026 at 2:13 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > Thanks for the update! The new version looks mostly good, I only found > one corner case that doesn't work, double quoted literals can still > get over-stripped: > > CREATE SCHEMA s; > CREATE TABLE s.p (id int, "s.weird" int) PARTITION BY RANGE (id); > CREATE TABLE s.pc PARTITION OF s.p > (CONSTRAINT chk CHECK ("s.weird" > 0)) FOR VALUES FROM (0) TO (100); > SELECT * FROM pg_get_table_ddl('s.pc', 'owner', 'false', > 'schema_qualified', 'false'); > Fixed the issue above. The v5 patch is ready for review/testing. -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-06-11T13:07:37Z
Em qui., 11 de jun. de 2026 às 04:48, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: > Fixed the issue above. The v5 patch is ready for review/testing. > One thing I noticed, though I'm not sure if it's the point here, is that it's not possible to extract only the foreign keys or only the triggers from the table. So if we want to extract the objects independently by type, we would need to have all the return types as optional, and we could have more granularity in the return types. Just like you have... if (!ctx->include_indexes) You could have too + if (!ctx->include_create_table) + if (!ctx->include_foreign_keys) + if (!ctx->include_primary_keys) Because only in this way can we more or less execute the dump behavior here, which is to create all the tables beforehand, then primary keys, then foreign keys, then triggers. I repeat, sorry if this is not the function's intended purpose. regards Marcos
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-11T21:00:59Z
The new version seem to work correctly to me, I didn't find any further issues. Now the main blockers seem to be the remaining TODOs related to includes_triggers/includes_policies. I only have some minor comments about code structuring: + * get_inheritance_parents + * Return a List of parent OIDs for relid, ordered by inhseqno. + * + * find_inheritance_children() walks the opposite direction (parent->children), Shouldn't this follow the same naming and parameter pattern and live at the same place in pg_inherits? +static char * +lookup_qualified_relname(Oid relid) +... +static char * +lookup_relname_for_emit(Oid relid, bool schema_qualified, Oid base_namespace) Is lookup_qualified_relname needed? It is only called within lookup_relname_for_emit, and it results in a double syscache lookup, which could be avoided if these were a single function. + /* COMPRESSION clause, only if explicitly set on the column. */ + if (CompressionMethodIsValid(att->attcompression)) + { + const char *cm = NULL; + + switch (att->attcompression) + { + case TOAST_PGLZ_COMPRESSION: + cm = "pglz"; + break; + case TOAST_LZ4_COMPRESSION: + cm = "lz4"; + break; + } + if (cm) + appendStringInfo(buf, " COMPRESSION %s", cm); + } Isn't this basically GetCompressionMethodName(att->attcompression)? + /* STORAGE clause, only if it differs from the type's default. */ + if (att->attstorage != get_typstorage(att->atttypid)) + { + const char *storage = NULL; + + switch (att->attstorage) + { + case TYPSTORAGE_PLAIN: +... And this seems like storage_name(att->attstorage) from tablecmds.c, the only issue is that that's currently static -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-12T01:10:30Z
Hello. At Thu, 11 Jun 2026 13:18:07 +0530, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote in > Fixed the issue above. The v5 patch is ready for review/testing. I have not looked at the patch in detail, but I noticed that some comments in the patch seem to contain non-ASCII characters. > * * re-emitting it as ALTER TABLE … ADD CONSTRAINT ― the column-emit I don't think that is recommended in PostgreSQL source comments, so these should probably be replaced with plain ASCII equivalents. https://www.postgresql.org/message-id/E1pnhhu-003D6z-Ki%40gemulon.postgresql.org For reference, I have attached the result of a quick search below. Regards, -- Kyotaro Horiguchi NTT Open Source Software Center Quick search result: ========= 20 matches in 18 lines for "[^[:ascii:]]" in buffer: v5-0001-Add-pg_get_table_ddl-to-reconstruct-CREATE-TABLE.patch 565:+ * re-emitting it as ALTER TABLE … ADD CONSTRAINT ― the column-emit 877:+ * defaults ― mirroring pg_get_database_ddl's pattern of 934:+ * SEQUENCE NAME ― omit when it matches the implicit 1042:+ * Table-level CHECK constraints ― emitted inline in the CREATE TABLE 1055:+ * applied per-column overrides ― DEFAULT, NOT NULL, and any locally 1155:+ * pg_get_ruledef, pg_get_statisticsobjdef_string) ― when 1383:+ * ALTER TABLE qualname ALTER COLUMN col SET DEFAULT expr ― one per 1422:+ * ALTER TABLE qualname ALTER COLUMN col SET (...) ― one per column 1468:+ * out-of-line by emit_local_constraints (the ALTER TABLE … ADD 1500:+ * ALTER TABLE … ADD CONSTRAINT for each locally-defined constraint 1617:+ * ALTER TABLE qualname REPLICA IDENTITY … ― emitted only when the 1749:+ * (#if 0) ― they will become a single helper call once the standalone 1830:+ * Pre-compute "<schema>." too ― the always-qualified helpers 1884:+ * Triggers and row-level security policies ― disabled until the 2680:+-- quoting (its prefix starts with "). Both forms of the prefix ― 2681:+-- bare-lowercase and quoted ― must be stripped from outer 3197:+-- quoting (its prefix starts with "). Both forms of the prefix ― 3198:+-- bare-lowercase and quoted ― must be stripped from outer ========= -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-15T07:52:03Z
On Thu, Jun 11, 2026 at 6:38 PM Marcos Pegoraro <marcos@f10.com.br> wrote: > Em qui., 11 de jun. de 2026 às 04:48, Akshay Joshi < > akshay.joshi@enterprisedb.com> escreveu: > >> Fixed the issue above. The v5 patch is ready for review/testing. >> > > One thing I noticed, though I'm not sure if it's the point here, is that > it's not possible to extract only the foreign keys or only the triggers > from the table. So if we want to extract the objects independently by type, > we would need to have all the return types as optional, and we could have > more granularity in the return types. > > Just like you have... > if (!ctx->include_indexes) > > You could have too > + if (!ctx->include_create_table) > + if (!ctx->include_foreign_keys) > + if (!ctx->include_primary_keys) > > Because only in this way can we more or less execute the dump behavior > here, which is to create all the tables beforehand, then primary keys, then > foreign keys, then triggers. > > I repeat, sorry if this is not the function's intended purpose. > I don't think per-contype flags are the right shape, though. The existing toggles group by catalog (indexes, constraints, rules, ...); splitting constraints into PK/FK/CHECK/UNIQUE/EXCLUDE/NOT NULL adds six flags on a second axis, and the function already carries nine. Only FKs have the cross-table dependency-ordering problem; the rest only reference the same table, so splitting them unlocks nothing new. On include_create_table, we are reconstructing the DDL for the table itself, so I don't think we should skip the CREATE TABLE statement. I'd rather always emit CREATE TABLE. > regards > Marcos > >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-15T08:55:06Z
Hi Kyotaro and Zsolt, I have incorporated the feedback provided by both of you. The v6 patch is updated and ready for your review. On Fri, Jun 12, 2026 at 6:40 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote: > Hello. > > At Thu, 11 Jun 2026 13:18:07 +0530, Akshay Joshi < > akshay.joshi@enterprisedb.com> wrote in > > Fixed the issue above. The v5 patch is ready for review/testing. > > I have not looked at the patch in detail, but I noticed that some > comments in the patch seem to contain non-ASCII characters. > > > * * re-emitting it as ALTER TABLE … ADD CONSTRAINT ― the column-emit > > I don't think that is recommended in PostgreSQL source comments, so > these should probably be replaced with plain ASCII equivalents. > > > https://www.postgresql.org/message-id/E1pnhhu-003D6z-Ki%40gemulon.postgresql.org > > For reference, I have attached the result of a quick search below. > > Regards, > > -- > Kyotaro Horiguchi > NTT Open Source Software Center > > > Quick search result: > > ========= > 20 matches in 18 lines for "[^[:ascii:]]" in buffer: > v5-0001-Add-pg_get_table_ddl-to-reconstruct-CREATE-TABLE.patch > 565:+ * re-emitting it as ALTER TABLE … ADD CONSTRAINT ― the > column-emit > 877:+ * defaults ― mirroring > pg_get_database_ddl's pattern of > 934:+ * SEQUENCE NAME ― > omit when it matches the implicit > 1042:+ * Table-level CHECK constraints ― emitted inline in the > CREATE TABLE > 1055:+ * applied per-column overrides ― DEFAULT, NOT NULL, > and any locally > 1155:+ * pg_get_ruledef, pg_get_statisticsobjdef_string) ― when > 1383:+ * ALTER TABLE qualname ALTER COLUMN col SET DEFAULT > expr ― one per > 1422:+ * ALTER TABLE qualname ALTER COLUMN col SET (...) ― > one per column > 1468:+ * out-of-line by emit_local_constraints (the ALTER > TABLE … ADD > 1500:+ * ALTER TABLE … ADD CONSTRAINT for each > locally-defined constraint > 1617:+ * ALTER TABLE qualname REPLICA IDENTITY … ― emitted > only when the > 1749:+ * (#if 0) ― they will become a single helper call once the > standalone > 1830:+ * Pre-compute "<schema>." too ― the always-qualified > helpers > 1884:+ * Triggers and row-level security policies ― disabled > until the > 2680:+-- quoting (its prefix starts with "). Both forms of the prefix ― > 2681:+-- bare-lowercase and quoted ― must be stripped from outer > 3197:+-- quoting (its prefix starts with "). Both forms of the prefix ― > 3198:+-- bare-lowercase and quoted ― must be stripped from outer > ========= >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-06-15T21:04:43Z
Em seg., 15 de jun. de 2026 às 04:52, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: > I don't think per-contype flags are the right shape, though. The existing > toggles group by catalog (indexes, constraints, rules, ...); splitting > constraints into PK/FK/CHECK/UNIQUE/EXCLUDE/NOT NULL adds six flags on a > second axis, and the function already carries nine. Only FKs have the > cross-table dependency-ordering problem; the rest only reference the same > table, so splitting them unlocks nothing new. > Ok, I understand your point. Initially, I saw the usefulness of this function for cloning a schema, something very common in a multi-tenant model. But creating the foreign keys along with the create table makes that unfeasible. Options are variadic, so you could split your emit_local_constraints into +emit_local_foreign_keys_constraints(TableDdlContext * ctx) + if (!(ctx->include_constraints || ctx->include_foreign_keys)) then + return +emit_local_primary_keys_constraints(TableDdlContext * ctx) + if (!(ctx->include_constraints || ctx->include_primary_keys)) then + return pg_get_table_ddl('x','includes_constraints','true') -- would print all constraints pg_get_table_ddl('x','include_primary_keys','true') -- would print only primary key constraints regards Marcos -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-19T12:19:44Z
On Tue, Jun 16, 2026 at 2:35 AM Marcos Pegoraro <marcos@f10.com.br> wrote: > Em seg., 15 de jun. de 2026 às 04:52, Akshay Joshi < > akshay.joshi@enterprisedb.com> escreveu: > >> I don't think per-contype flags are the right shape, though. The existing >> toggles group by catalog (indexes, constraints, rules, ...); splitting >> constraints into PK/FK/CHECK/UNIQUE/EXCLUDE/NOT NULL adds six flags on a >> second axis, and the function already carries nine. Only FKs have the >> cross-table dependency-ordering problem; the rest only reference the same >> table, so splitting them unlocks nothing new. >> > > Ok, I understand your point. Initially, I saw the usefulness of this > function for cloning a schema, something very common in a multi-tenant > model. But creating the foreign keys along with the create table makes that > unfeasible. > > Options are variadic, so you could split your emit_local_constraints into > +emit_local_foreign_keys_constraints(TableDdlContext * ctx) > + if (!(ctx->include_constraints || ctx->include_foreign_keys)) then > + return > > +emit_local_primary_keys_constraints(TableDdlContext * ctx) > + if (!(ctx->include_constraints || ctx->include_primary_keys)) then > + return > > pg_get_table_ddl('x','includes_constraints','true') -- would print all > constraints > pg_get_table_ddl('x','include_primary_keys','true') -- would print only > primary key constraints > The schema cloning use case is valid. The v7 patch (attached) adds a single new option, `includes_foreign_keys` (boolean, default true), which acts as an additive gate underneath `includes_constraints`. Calling pg_get_table_ddl(t, 'includes_foreign_keys', 'false') now emits everything except FOREIGN KEY constraints. This covers the multi-tenant clone workflow: create tables first without cross-table references, then re-run with the default to add the constraints once all targets exist. I held off on the broader split into per-contype options (includes_primary_keys, includes_unique, etc.) for two reasons. First, only FOREIGN KEY actually breaks schema cloning. PRIMARY KEY, UNIQUE, CHECK, EXCLUDE, and named NOT NULL constraints are all table local and don't reference anything else, so no workflow currently needs to suppress them independently. Second, the function already exposes thirteen boolean options; adding five more granular ones without a concrete use case expands the surface area unnecessarily. The v7 patch is ready for review. > > regards > Marcos > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-19T19:45:12Z
The previous features all look good to me, I only have one question for the new flag. > Calling > pg_get_table_ddl(t, 'includes_foreign_keys', 'false') now emits everything > except FOREIGN KEY constraints. This covers the multi-tenant clone > workflow: create tables first without cross-table references, then re-run > with the default to add the constraints once all targets exist. I think this feature needs a bit more documentation, an "only_foreign_keys" flag, or both. CREATE TABLE refd (id int PRIMARY KEY); CREATE TABLE cons (a int CHECK(a>0), b int UNIQUE, c int REFERENCES refd(id)); -- pass 1: running without foreign keys SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','false'); -- execute everything -- loading data -- pass 2: running with everything SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','true'); -- ERROR: relation "cons" already exists (and the unique constraint also collides) I could do a "grep FOREIGN KEY" before executing (unless it's a tricky schema where that phrase appears elsewhere), or since psql continues on error, it will simply work if I accept a significant error noise, but then the documentation should be clear about this limitation. Following the documented approach and getting a bunch of unexpected errors could be confusing for users. -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-22T06:26:58Z
Thanks for the review; you're right, `includes_foreign_keys=false` on its own is a half-measure. Re-running with the default to add FKs back collides with the existing CREATE TABLE, UNIQUE indexes, etc. I've added an only_foreign_keys option (boolean, default false) as the natural complement of includes_foreign_keys=false. When set to true, the function emits only the ALTER TABLE ... ADD CONSTRAINT ... FOREIGN KEY statements and suppresses everything else (CREATE TABLE, owner, indexes, non-FK constraints, rules, statistics, replica identity, RLS toggles). Partition-child recursion still runs so child FKs are reached too. Combining `only_foreign_keys=true` with `includes_foreign_keys=false` is rejected upfront since it would produce no output. The documentation paragraph for `includes_foreign_keys` now directs users to `only_foreign_keys` as the intended second pass. Regression coverage adds three cases: the FK-only emission for your cons example, the zero-row result for a table without FKs, and the error path. The v8 patch is ready for review. On Sat, Jun 20, 2026 at 1:15 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > The previous features all look good to me, I only have one question > for the new flag. > > > Calling > > pg_get_table_ddl(t, 'includes_foreign_keys', 'false') now emits > everything > > except FOREIGN KEY constraints. This covers the multi-tenant clone > > workflow: create tables first without cross-table references, then re-run > > with the default to add the constraints once all targets exist. > > I think this feature needs a bit more documentation, an > "only_foreign_keys" flag, or both. > > CREATE TABLE refd (id int PRIMARY KEY); > CREATE TABLE cons (a int CHECK(a>0), b int UNIQUE, c int REFERENCES > refd(id)); > > -- pass 1: running without foreign keys > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','false'); > -- execute everything > > -- loading data > > -- pass 2: running with everything > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','true'); > -- ERROR: relation "cons" already exists (and the unique constraint > also collides) > > I could do a "grep FOREIGN KEY" before executing (unless it's a tricky > schema where that phrase appears elsewhere), or since psql continues > on error, it will simply work if I accept a significant error noise, > but then the documentation should be clear about this limitation. > Following the documented approach and getting a bunch of unexpected > errors could be confusing for users. > > > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Chao Li <li.evan.chao@gmail.com> — 2026-06-22T06:54:35Z
> On Jun 22, 2026, at 14:26, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: > > Thanks for the review; you're right, `includes_foreign_keys=false` on its own is a half-measure. Re-running with the default to add FKs back collides with the existing CREATE TABLE, UNIQUE indexes, etc. > > I've added an only_foreign_keys option (boolean, default false) as the natural complement of includes_foreign_keys=false. When set to true, the function emits only the ALTER TABLE ... ADD CONSTRAINT ... FOREIGN KEY statements and suppresses everything else (CREATE TABLE, owner, indexes, non-FK constraints, rules, statistics, replica identity, RLS toggles). Partition-child recursion still runs so child FKs are reached too. Combining `only_foreign_keys=true` with `includes_foreign_keys=false` is rejected upfront since it would produce no output. > > The documentation paragraph for `includes_foreign_keys` now directs users to `only_foreign_keys` as the intended second pass. Regression coverage adds three cases: the FK-only emission for your cons example, the zero-row result for a table without FKs, and the error path. > > The v8 patch is ready for review. > > On Sat, Jun 20, 2026 at 1:15 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > The previous features all look good to me, I only have one question > for the new flag. > > > Calling > > pg_get_table_ddl(t, 'includes_foreign_keys', 'false') now emits everything > > except FOREIGN KEY constraints. This covers the multi-tenant clone > > workflow: create tables first without cross-table references, then re-run > > with the default to add the constraints once all targets exist. > > I think this feature needs a bit more documentation, an > "only_foreign_keys" flag, or both. > > CREATE TABLE refd (id int PRIMARY KEY); > CREATE TABLE cons (a int CHECK(a>0), b int UNIQUE, c int REFERENCES refd(id)); > > -- pass 1: running without foreign keys > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','false'); > -- execute everything > > -- loading data > > -- pass 2: running with everything > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','true'); > -- ERROR: relation "cons" already exists (and the unique constraint > also collides) > > I could do a "grep FOREIGN KEY" before executing (unless it's a tricky > schema where that phrase appears elsewhere), or since psql continues > on error, it will simply work if I accept a significant error noise, > but then the documentation should be clear about this limitation. > Following the documented approach and getting a bunch of unexpected > errors could be confusing for users. > > > <v8-0001-Add-pg_get_table_ddl-to-reconstruct-CREATE-TABLE.patch> I have a comment, or maybe a question: ``` +{ oid => '8215', descr => 'get DDL to recreate a table', + proname => 'pg_get_table_ddl', prorows => '50', provariadic => 'text', + proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', + pronargdefaults => '1', prorettype => 'text', + proargtypes => 'regclass text', proallargtypes => '{regclass,text}', + proargmodes => '{i,v}', proargdefaults => '{NULL}', + prosrc => 'pg_get_table_ddl' }, ``` Since provariadic is text, I wonder if proallargtypes should be {regclass,_text}, with _text meaning an array of text. I’m asking because I have had this suspicion for some time. I saw a few other procs using the same pattern, for example: ``` { oid => '6501', descr => 'get DDL to recreate a role', proname => 'pg_get_role_ddl', prorows => '10', provariadic => 'text', proisstrict => 'f', proretset => 't', provolatile => 's', pronargdefaults => '1', prorettype => 'text', proargtypes => 'regrole text', proallargtypes => '{regrole,text}', proargmodes => '{i,v}', proargdefaults => '{NULL}', prosrc => 'pg_get_role_ddl' }, ``` But for jsonb_delete etc procs, _text is used: ``` { oid => '3343', proname => 'jsonb_delete', provariadic => 'text', prorettype => 'jsonb', proargtypes => 'jsonb _text', proallargtypes => '{jsonb,_text}', proargmodes => '{i,v}', proargnames => '{from_json,path_elems}', prosrc => 'jsonb_delete_array' }, ``` So I wonder whether “text” rather than “_text" is intentionally used in proallargtypes, or if this was just never noticed. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-22T12:40:53Z
You're right, and thanks for spotting this. The existing pattern in pg_proc.dat for variadic-text functions (e.g., jsonb_delete, json_extract_path) uses _text at the variadic position in both proargtypes and proallargtypes, with provariadic => 'text'. That is the convention documented by the sanity check in src/test/regress/sql/opr_sanity.sql. The same issue applies to *pg_get_role_ddl*, *pg_get_tablespace_ddl* (both variants), and *pg_get_database_ddl*, but that will require a separate patch. The v9 patch is ready for review. On Mon, Jun 22, 2026 at 12:25 PM Chao Li <li.evan.chao@gmail.com> wrote: > > > > On Jun 22, 2026, at 14:26, Akshay Joshi <akshay.joshi@enterprisedb.com> > wrote: > > > > Thanks for the review; you're right, `includes_foreign_keys=false` on > its own is a half-measure. Re-running with the default to add FKs back > collides with the existing CREATE TABLE, UNIQUE indexes, etc. > > > > I've added an only_foreign_keys option (boolean, default false) as the > natural complement of includes_foreign_keys=false. When set to true, the > function emits only the ALTER TABLE ... ADD CONSTRAINT ... FOREIGN KEY > statements and suppresses everything else (CREATE TABLE, owner, indexes, > non-FK constraints, rules, statistics, replica identity, RLS toggles). > Partition-child recursion still runs so child FKs are reached too. > Combining `only_foreign_keys=true` with `includes_foreign_keys=false` is > rejected upfront since it would produce no output. > > > > The documentation paragraph for `includes_foreign_keys` now directs > users to `only_foreign_keys` as the intended second pass. Regression > coverage adds three cases: the FK-only emission for your cons example, the > zero-row result for a table without FKs, and the error path. > > > > The v8 patch is ready for review. > > > > On Sat, Jun 20, 2026 at 1:15 AM Zsolt Parragi <zsolt.parragi@percona.com> > wrote: > > The previous features all look good to me, I only have one question > > for the new flag. > > > > > Calling > > > pg_get_table_ddl(t, 'includes_foreign_keys', 'false') now emits > everything > > > except FOREIGN KEY constraints. This covers the multi-tenant clone > > > workflow: create tables first without cross-table references, then > re-run > > > with the default to add the constraints once all targets exist. > > > > I think this feature needs a bit more documentation, an > > "only_foreign_keys" flag, or both. > > > > CREATE TABLE refd (id int PRIMARY KEY); > > CREATE TABLE cons (a int CHECK(a>0), b int UNIQUE, c int REFERENCES > refd(id)); > > > > -- pass 1: running without foreign keys > > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','false'); > > -- execute everything > > > > -- loading data > > > > -- pass 2: running with everything > > SELECT * FROM pg_get_table_ddl('cons','includes_foreign_keys','true'); > > -- ERROR: relation "cons" already exists (and the unique constraint > > also collides) > > > > I could do a "grep FOREIGN KEY" before executing (unless it's a tricky > > schema where that phrase appears elsewhere), or since psql continues > > on error, it will simply work if I accept a significant error noise, > > but then the documentation should be clear about this limitation. > > Following the documented approach and getting a bunch of unexpected > > errors could be confusing for users. > > > > > > <v8-0001-Add-pg_get_table_ddl-to-reconstruct-CREATE-TABLE.patch> > > I have a comment, or maybe a question: > ``` > +{ oid => '8215', descr => 'get DDL to recreate a table', > + proname => 'pg_get_table_ddl', prorows => '50', provariadic => 'text', > + proisstrict => 'f', proretset => 't', provolatile => 's', proparallel > => 'r', > + pronargdefaults => '1', prorettype => 'text', > + proargtypes => 'regclass text', proallargtypes => '{regclass,text}', > + proargmodes => '{i,v}', proargdefaults => '{NULL}', > + prosrc => 'pg_get_table_ddl' }, > ``` > > Since provariadic is text, I wonder if proallargtypes should be > {regclass,_text}, with _text meaning an array of text. > > I’m asking because I have had this suspicion for some time. I saw a few > other procs using the same pattern, for example: > ``` > { oid => '6501', descr => 'get DDL to recreate a role', > proname => 'pg_get_role_ddl', prorows => '10', provariadic => 'text', > proisstrict => 'f', proretset => 't', provolatile => 's', > pronargdefaults => '1', prorettype => 'text', proargtypes => 'regrole > text', > proallargtypes => '{regrole,text}', proargmodes => '{i,v}', > proargdefaults => '{NULL}', prosrc => 'pg_get_role_ddl' }, > ``` > > But for jsonb_delete etc procs, _text is used: > ``` > { oid => '3343', > proname => 'jsonb_delete', provariadic => 'text', prorettype => 'jsonb', > proargtypes => 'jsonb _text', proallargtypes => '{jsonb,_text}', > proargmodes => '{i,v}', proargnames => '{from_json,path_elems}', > prosrc => 'jsonb_delete_array' }, > ``` > > So I wonder whether “text” rather than “_text" is intentionally used in > proallargtypes, or if this was just never noticed. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Chao Li <li.evan.chao@gmail.com> — 2026-06-22T14:00:20Z
> On Jun 22, 2026, at 20:40, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: > > You're right, and thanks for spotting this. The existing pattern in pg_proc.dat for variadic-text functions (e.g., jsonb_delete, json_extract_path) uses _text at the variadic position in both proargtypes and proallargtypes, with provariadic => 'text'. That is the convention documented by the sanity check in src/test/regress/sql/opr_sanity.sql. > > The same issue applies to pg_get_role_ddl, pg_get_tablespace_ddl (both variants), and pg_get_database_ddl, but that will require a separate patch. > Thanks for confirming. Then I will file a patch tomorrow to fix those. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-22T14:11:18Z
On Mon, Jun 22, 2026 at 7:30 PM Chao Li <li.evan.chao@gmail.com> wrote: > > > > On Jun 22, 2026, at 20:40, Akshay Joshi <akshay.joshi@enterprisedb.com> > wrote: > > > > You're right, and thanks for spotting this. The existing pattern in > pg_proc.dat for variadic-text functions (e.g., jsonb_delete, > json_extract_path) uses _text at the variadic position in both proargtypes > and proallargtypes, with provariadic => 'text'. That is the convention > documented by the sanity check in src/test/regress/sql/opr_sanity.sql. > > > > The same issue applies to pg_get_role_ddl, pg_get_tablespace_ddl (both > variants), and pg_get_database_ddl, but that will require a separate patch. > > > > Thanks for confirming. Then I will file a patch tomorrow to fix those. > I started working on it, but if you want to take the lead, just let me know and I won't send my version over > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-06-22T14:54:38Z
Em seg., 22 de jun. de 2026 às 03:27, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: > The documentation paragraph for `includes_foreign_keys` now directs users > to `only_foreign_keys` as the intended second pass. Regression coverage > adds three cases: the FK-only emission for your cons example, the zero-row > result for a table without FKs, and the error path. > >> I still think this model of only having options for foreign keys is incomplete, maybe wrong. Imagine then cloning a schema from a publication server to be executed on a subscription server. So I don't want any other constraints besides the primary key, for example. The way you implemented it is not possible. Furthermore having only_foreign_keys and includes_foreign_keys seems confuse. regards Marcos
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-22T18:56:37Z
Thanks, I can confirm that only_foreign_keys works properly. > Combining `only_foreign_keys=true` with `includes_foreign_keys=false` is > rejected upfront since it would produce no output. Shouldn't only_foreign_keys=true with include_constraints=false also error out?
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Chao Li <li.evan.chao@gmail.com> — 2026-06-23T02:18:11Z
> On Jun 22, 2026, at 22:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: > > > > On Mon, Jun 22, 2026 at 7:30 PM Chao Li <li.evan.chao@gmail.com> wrote: > > > > On Jun 22, 2026, at 20:40, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: > > > > You're right, and thanks for spotting this. The existing pattern in pg_proc.dat for variadic-text functions (e.g., jsonb_delete, json_extract_path) uses _text at the variadic position in both proargtypes and proallargtypes, with provariadic => 'text'. That is the convention documented by the sanity check in src/test/regress/sql/opr_sanity.sql. > > > > The same issue applies to pg_get_role_ddl, pg_get_tablespace_ddl (both variants), and pg_get_database_ddl, but that will require a separate patch. > > > > Thanks for confirming. Then I will file a patch tomorrow to fix those. > > I started working on it, but if you want to take the lead, just let me know and I won't send my version over > The changes to pg_proc.dat have been in my local tree for some time. Today I also fixed the sanity check in opr_sanity so that it can now report this mismatch. Could you please take a look at my patch [1]? [1] https://www.postgresql.org/message-id/D41A334E-ED9E-42EE-830D-28D4D36E9317%40gmail.com Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-23T06:15:57Z
At Mon, 15 Jun 2026 14:25:06 +0530, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote in > Hi Kyotaro and Zsolt, > > I have incorporated the feedback provided by both of you. > The v6 patch is updated and ready for your review. > > On Fri, Jun 12, 2026 at 6:40 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> > wrote: > > > > I have not looked at the patch in detail, but I noticed that some > > comments in the patch seem to contain non-ASCII characters. Thanks for the update. That issue appears to be resolved in this version. Regards, -- Kyotaro Horiguchi NTT Open Source Software Center
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-23T07:21:04Z
At Mon, 22 Jun 2026 18:10:53 +0530, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote in > The v9 patch is ready for review. I have not looked closely at the DDL generation logic itself, but I have a few comments on how pg_get_table_ddl handles its options. Since pg_get_table_ddl_internal() appears to copy these values into TableDdlContext almost immediately, I wonder whether TableDdlContext could be initialized by the caller instead. Using positional boolean arguments is probably fine when there are only a handful of options, but with around fifteen of them the current approach seems somewhat error-prone. It might also be clearer to initialize the default values first, and then override only the fields corresponding to explicitly specified options, rather than folding the default handling and option lookup into the same expression. Regards, -- Kyotaro Horiguchi NTT Open Source Software Center
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-23T08:57:54Z
On Mon, Jun 22, 2026 at 8:25 PM Marcos Pegoraro <marcos@f10.com.br> wrote: > Em seg., 22 de jun. de 2026 às 03:27, Akshay Joshi < > akshay.joshi@enterprisedb.com> escreveu: > >> The documentation paragraph for `includes_foreign_keys` now directs users >> to `only_foreign_keys` as the intended second pass. Regression coverage >> adds three cases: the FK-only emission for your cons example, the zero-row >> result for a table without FKs, and the error path. >> >>> > I still think this model of only having options for foreign keys is > incomplete, maybe wrong. > Imagine then cloning a schema from a publication server to be executed on > a subscription server. So I don't want any other constraints besides the > primary key, for example. The way you implemented it is not possible. > > Furthermore having only_foreign_keys and includes_foreign_keys seems > confuse. > OK. I'd like to change the model, not just the flag names. Drop the entire *includes_** family and *only_foreign_keys*, replace them with two mutually-exclusive variadic keys: - include => 'kind1,kind2,...' — emit only these kinds - exclude => 'kind1,kind2,...' — emit everything except these Setting both is an error. Setting neither emits everything (today's default behavior, which is preserved). *Vocabulary*: indexes, primary_key, unique, check, foreign_keys, exclusion, rules, statistics, triggers, policies, rls, replica_identity, partitions. Unknown kind → parse-time error, which also catches typos that the boolean version silently accepted. If everyone approves the model above, I'll try implementing it. regards > Marcos > >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-23T09:22:37Z
On Tue, Jun 23, 2026 at 12:51 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote: > At Mon, 22 Jun 2026 18:10:53 +0530, Akshay Joshi < > akshay.joshi@enterprisedb.com> wrote in > > The v9 patch is ready for review. > > I have not looked closely at the DDL generation logic itself, but I > have a few comments on how pg_get_table_ddl handles its options. > > Since pg_get_table_ddl_internal() appears to copy these values into > TableDdlContext almost immediately, I wonder whether TableDdlContext > could be initialized by the caller instead. > > Using positional boolean arguments is probably fine when there are > only a handful of options, but with around fifteen of them the current > approach seems somewhat error-prone. > > It might also be clearer to initialize the default values first, and > then override only the fields corresponding to explicitly specified > options, rather than folding the default handling and option lookup > into the same expression. > I assume that changing the implementation model, as I mentioned in my other email, will solve this problem as well. We can drop the entire includes_* family and only_foreign_keys, and replace them with two mutually exclusive variadic keys: include => 'kind1,kind2,...' — emit only these kinds exclude => 'kind1,kind2,...' — emit everything except these kinds > > > Regards, > > -- > Kyotaro Horiguchi > NTT Open Source Software Center >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-23T13:34:25Z
All, A quick note on a design change I made in the *pg_get_table_ddl* option surface. The earlier draft exposed one boolean option per sub-object kind: include_indexes, include_primary_key, include_check, include_foreign_keys, include_rules, include_statistics, include_rls, include_replica_identity, include_partitions, and so on. Every new kind we wanted to gate (triggers, policies, exclusion constraints…) meant another option name. Callers wanting "everything except FKs" had to flip nine flags, and the include/exclude polarity was not symmetric: to drop one item, you toggled one flag, but to keep only one item, you toggled all the others. *I have replaced those with two options:* - include — comma-separated list of kinds; emit only the listed ones. - exclude — comma-separated list of kinds; emit everything except the listed ones. *Vocabulary*: table, indexes, primary_key, unique, check, foreign_keys, exclusion, rules, statistics, triggers, policies, rls, replica_identity, partitions. NOT NULL is intentionally omitted from the vocabulary; it's always emitted to avoid silently producing schemas that accept NULLs when the source would have rejected them. The v10 patch is ready for review. On Tue, Jun 23, 2026 at 2:52 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: > > > On Tue, Jun 23, 2026 at 12:51 PM Kyotaro Horiguchi < > horikyota.ntt@gmail.com> wrote: > >> At Mon, 22 Jun 2026 18:10:53 +0530, Akshay Joshi < >> akshay.joshi@enterprisedb.com> wrote in >> > The v9 patch is ready for review. >> >> I have not looked closely at the DDL generation logic itself, but I >> have a few comments on how pg_get_table_ddl handles its options. >> >> Since pg_get_table_ddl_internal() appears to copy these values into >> TableDdlContext almost immediately, I wonder whether TableDdlContext >> could be initialized by the caller instead. >> >> Using positional boolean arguments is probably fine when there are >> only a handful of options, but with around fifteen of them the current >> approach seems somewhat error-prone. >> >> It might also be clearer to initialize the default values first, and >> then override only the fields corresponding to explicitly specified >> options, rather than folding the default handling and option lookup >> into the same expression. >> > > I assume that changing the implementation model, as I mentioned in my > other email, will solve this problem as well. We can drop the entire > includes_* family and only_foreign_keys, and replace them with two mutually > exclusive variadic keys: > include => 'kind1,kind2,...' — emit only these kinds > exclude => 'kind1,kind2,...' — emit everything except these kinds >> >> >> Regards, >> >> -- >> Kyotaro Horiguchi >> NTT Open Source Software Center >> >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-06-23T14:03:08Z
Em ter., 23 de jun. de 2026 às 10:34, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: > A quick note on a design change I made in the *pg_get_table_ddl* option > surface > Much better now. All options can be present or omitted, so why couldn't the owner also be part of the include/exclude options too ? Just one addition, some options are plural and others are not. We have indexes, policies, triggers, but we also have check, unique, exclusion. You don't know how many indices or checks you have, so wouldn't it be better to have them in singular form ? regards Marcos
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-23T20:05:20Z
The new design is definitely an improvement as a more generic interface. I'm unsure about the "include" wording, to me that suggest "also includes", while the actual behavior is "only includes". To me it would be a bit surprising pg_get_table_ddl including something not including the basic create table statement. I also found a few issues with include in v10. Include check doesn't seem to work: CREATE TABLE t ( id int PRIMARY KEY, qty int CHECK (qty > 0), CONSTRAINT t_id_pos CHECK (id > 0) ); SELECT * FROM pg_get_table_ddl('t', 'include', 'check'); -- empty? The include partitions clause also doesn't work: CREATE TABLE p (id int, val text) PARTITION BY RANGE (id); CREATE TABLE p_a PARTITION OF p FOR VALUES FROM (0) TO (100); CREATE TABLE p_b PARTITION OF p FOR VALUES FROM (100) TO (200); SELECT * FROM pg_get_table_ddl('p','include','partitions'); -- empty There's also an issue with replica identity non primary key unique indexes: CREATE TABLE t2 (a int NOT NULL UNIQUE, b int); ALTER TABLE t2 REPLICA IDENTITY USING INDEX t2_a_key; SELECT * FROM pg_get_table_ddl('t2','exclude','unique'); -- ALTER TABLE public.t2 REPLICA IDENTITY USING INDEX t2_a_key; -- but there's no such index -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-24T08:23:13Z
On Tue, Jun 23, 2026 at 7:33 PM Marcos Pegoraro <marcos@f10.com.br> wrote: > Em ter., 23 de jun. de 2026 às 10:34, Akshay Joshi < > akshay.joshi@enterprisedb.com> escreveu: > >> A quick note on a design change I made in the *pg_get_table_ddl* option >> surface >> > > Much better now. > > All options can be present or omitted, so why couldn't the owner also be > part of the include/exclude options too ? > > Just one addition, some options are plural and others are not. > We have indexes, policies, triggers, but we also have check, unique, > exclusion. > You don't know how many indices or checks you have, so wouldn't it be > better to have them in singular form ? > Regarding moving owner into the include/exclude syntax, I prefer keeping it as a top-level boolean for now: 1) It aligns with existing sibling functions (pg_get_role_ddl, pg_get_tablespace_ddl, and pg_get_database_ddl), which all use a boolean owner. 2) Existing regression tests and early callers pass owner => false for output stability. Moving this into the exclude list would create unnecessary churn that I'd prefer to avoid bundling here. As for the naming inconsistency (singular vs. plural), you're entirely right mixing indexes/policies/triggers with checks/uniques/exclusions was an oversight. Note that I left statistics alone to match the CREATE STATISTICS keyword. I'll address the naming consistency in the next patch. > > regards > Marcos >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-24T09:16:03Z
On Wed, Jun 24, 2026 at 1:35 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > The new design is definitely an improvement as a more generic > interface. I'm unsure about the "include" wording, to me that suggest > "also includes", while the actual behavior is "only includes". To me > it would be a bit surprising pg_get_table_ddl including something not > including the basic create table statement. > > I also found a few issues with include in v10. > > Include check doesn't seem to work: > > CREATE TABLE t ( > id int PRIMARY KEY, > qty int CHECK (qty > 0), > CONSTRAINT t_id_pos CHECK (id > 0) > ); > > SELECT * FROM pg_get_table_ddl('t', 'include', 'check'); -- empty? > > The include partitions clause also doesn't work: > > CREATE TABLE p (id int, val text) PARTITION BY RANGE (id); > CREATE TABLE p_a PARTITION OF p FOR VALUES FROM (0) TO (100); > CREATE TABLE p_b PARTITION OF p FOR VALUES FROM (100) TO (200); > > SELECT * FROM pg_get_table_ddl('p','include','partitions'); -- empty > > There's also an issue with replica identity non primary key unique indexes: > > CREATE TABLE t2 (a int NOT NULL UNIQUE, b int); > ALTER TABLE t2 REPLICA IDENTITY USING INDEX t2_a_key; > SELECT * FROM pg_get_table_ddl('t2','exclude','unique'); > -- ALTER TABLE public.t2 REPLICA IDENTITY USING INDEX t2_a_key; > -- but there's no such index > > Thanks for the review. In the attached patch, I’ve renamed *include/exclude* to *only* and *except*. If you have any other suggestions or a better name, please let me know. Fixed all the above issues as well. The semantics remain unchanged: 1) only => 'kind1,kind2,...' — emits only the listed kinds. 2) except => 'kind1,kind2,...' — emits everything except the listed kinds. 3) Setting both raises an error; setting neither emits all kinds (the default). I also dropped the trailing "s" from the multi-item kind names so the vocabulary reads naturally with only/except (e.g., index, foreign_key, rule, trigger, policy, partition). statistics and rls retain their existing spellings. The documentation, regression tests, and func-info.sgml have all been updated accordingly. The v11 patch is now ready for review and testing. -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Rui Zhao <zhaorui126@gmail.com> — 2026-06-28T16:44:31Z
Hi Akshay, Nice patch -- server-side CREATE TABLE reconstruction is something we want in production. I tested v11 on current master and it round-trips correctly across the documented coverage, including the recent grammar (VIRTUAL columns, NOT ENFORCED, temporal WITHOUT OVERLAPS / PERIOD) and state set later via ALTER. A few comments: 1. Real bug in schema_qualified => false. strip_schema_prefix() strips the base prefix anywhere in code position with no token-boundary check, so a cross-schema name whose schema ends with the base schema's name gets mangled: postgres=# CREATE SCHEMA xs; CREATE SCHEMA postgres=# CREATE TABLE xs.reftbl(id int PRIMARY KEY); CREATE TABLE postgres=# CREATE SCHEMA s; CREATE SCHEMA postgres=# CREATE TABLE s.orders(id int PRIMARY KEY, ref int REFERENCES xs.reftbl(id)); CREATE TABLE postgres=# SELECT d FROM pg_get_table_ddl('s.orders'::regclass,'schema_qualified','false') d; d --------------------------------------------------------------------------------------------- CREATE TABLE orders (id integer NOT NULL, ref integer); ALTER TABLE orders OWNER TO postgres; ALTER TABLE orders ADD CONSTRAINT orders_pkey PRIMARY KEY (id); ALTER TABLE orders ADD CONSTRAINT orders_ref_fkey FOREIGN KEY (ref) REFERENCES xreftbl(id); (4 rows) The last FK line says REFERENCES xreftbl(id); it should be xs.reftbl(id), and the result doesn't replay. Gating the match on a token boundary fixes it -- though see (2), which would remove this code path (and the bug) entirely. 2. Bigger picture: is schema_qualified needed at all? None of the existing pg_get_*def / pg_get_*ddl functions have such a knob, so this would be the lone exception. The established convention is to let search_path decide (generate_relation_name): pg_get_viewdef and pg_get_constraintdef already work that way, and pg_get_indexdef supports it too via pg_get_indexdef(idx, 0, true) (the PRETTYFLAG_SCHEMA code path). It is genuinely what we want in production -- the caller controls qualification through search_path. Set it to the table's schema for unqualified output, or to pg_catalog (or '') for fully-qualified schema.table; pg_dump itself dumps under an empty search_path (ALWAYS_SECURE_SEARCH_PATH_SQL) for exactly this reason. Following the convention would also drop the option and the strip_schema_prefix code (and this bug). That last point matters: there's no robust way to strip a schema prefix out of already-generated SQL by text processing. Doing it safely means re-tokenizing arbitrary SQL (string literals, quoted identifiers, dollar quotes, comments, casts, operators, ...), and strip_schema_prefix is a hand-rolled partial scanner of exactly that. It shows -- it has already needed several over-stripping fixes during review (the base name appearing inside a string literal, and inside a quoted identifier), and the token-boundary case in (1) is yet another. Deciding qualification at generation time (generate_relation_name) avoids the whole class: the backend's real deparser already gets this right, rather than a post-hoc string pass trying to re-derive it. 3. typedefs.list is missing TableDdlContext and LocalNotNullEntry, so pgindent leaves the "Type * var" pointer spacing in ~30 places -- for example the forward declarations at ddlutils.c:226-237: static void append_stmt(TableDdlContext * ctx); There is also a stale comment at ddlutils.c:1830 in append_column_defs(): inherited columns are described as "emitted by the INHERITS clause (once implemented)", but INHERITS is implemented now. 4. Related to (2): a temporary table's default output qualifies it with the session-private pg_temp_NN schema (e.g. CREATE TEMPORARY TABLE pg_temp_3.t ...), which won't replay anywhere else. A reconstructed temp table should just be CREATE TEMPORARY TABLE t (...) -- the TEMPORARY keyword already puts it in pg_temp, so the schema name should never be emitted. This also falls out for free under the search_path convention in (2): pg_temp is always in the effective search_path, so the table is visible and wouldn't be qualified in the first place. 5. The commit message is out of sync with the code and func-info.sgml on the option interface: it still says "include and exclude" and lists plural kind names (indexes, foreign_keys, triggers, policies, partitions), whereas the code and docs use only/except and singular names (index, foreign_key, trigger, policy, partition). Looks like a leftover from the include/exclude -> only/except rename. (FWIW on the naming itself, include/exclude is the more common convention for this kind of list parameter -- pg_dump has --exclude-table/-schema/-extension, etc. -- while only/except reads more like the SQL keywords.) Otherwise it looks good. Thanks, Rui -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-29T10:40:47Z
On Sun, Jun 28, 2026 at 10:14 PM Rui Zhao <zhaorui126@gmail.com> wrote: > Hi Akshay, > > Nice patch -- server-side CREATE TABLE reconstruction is something we want > in > production. I tested v11 on current master and it round-trips correctly > across the documented coverage, including the recent grammar (VIRTUAL > columns, NOT ENFORCED, temporal WITHOUT OVERLAPS / PERIOD) and state set > later via ALTER. A few comments: > > 1. Real bug in schema_qualified => false. strip_schema_prefix() strips the > base prefix anywhere in code position with no token-boundary check, so a > cross-schema name whose schema ends with the base schema's name gets > mangled: > > postgres=# CREATE SCHEMA xs; > CREATE SCHEMA > postgres=# CREATE TABLE xs.reftbl(id int PRIMARY KEY); > CREATE TABLE > postgres=# CREATE SCHEMA s; > CREATE SCHEMA > postgres=# CREATE TABLE s.orders(id int PRIMARY KEY, ref int > REFERENCES xs.reftbl(id)); > CREATE TABLE > postgres=# SELECT d FROM > pg_get_table_ddl('s.orders'::regclass,'schema_qualified','false') d; > d > > --------------------------------------------------------------------------------------------- > CREATE TABLE orders (id integer NOT NULL, ref integer); > ALTER TABLE orders OWNER TO postgres; > ALTER TABLE orders ADD CONSTRAINT orders_pkey PRIMARY KEY (id); > ALTER TABLE orders ADD CONSTRAINT orders_ref_fkey FOREIGN KEY (ref) > REFERENCES xreftbl(id); > (4 rows) > > The last FK line says REFERENCES xreftbl(id); it should be xs.reftbl(id), > and the result doesn't replay. Gating the match on a token boundary fixes > it -- though see (2), which would remove this code path (and the bug) > entirely. > Fixed. > > 2. Bigger picture: is schema_qualified needed at all? None of the existing > pg_get_*def / pg_get_*ddl functions have such a knob, so this would be the > lone exception. The established convention is to let search_path decide > (generate_relation_name): pg_get_viewdef and pg_get_constraintdef already > work that way, and pg_get_indexdef supports it too via > pg_get_indexdef(idx, 0, true) (the PRETTYFLAG_SCHEMA code path). It is > genuinely what we want in production -- the caller > controls qualification through search_path. Set it to the table's schema > for > unqualified output, or to pg_catalog (or '') for fully-qualified > schema.table; pg_dump itself dumps under an empty search_path > (ALWAYS_SECURE_SEARCH_PATH_SQL) for exactly this reason. Following the > convention would also drop the option and the strip_schema_prefix code (and > this bug). > > That last point matters: there's no robust way to strip a schema prefix out > of already-generated SQL by text processing. Doing it safely means > re-tokenizing arbitrary SQL (string literals, quoted identifiers, dollar > quotes, comments, casts, operators, ...), and strip_schema_prefix is a > hand-rolled partial scanner of exactly that. It shows -- it has already > needed several over-stripping fixes during review (the base name appearing > inside a string literal, and inside a quoted identifier), and the > token-boundary case in (1) is yet another. Deciding qualification at > generation time (generate_relation_name) avoids the whole class: the > backend's real deparser already gets this right, rather than a post-hoc > string pass trying to re-derive it. > Done. strip_schema_prefix and append_stripped_stmt are gone. Instead of post-processing, I added four thin wrappers in ruleutils.c. Let the deparser decide qualification at generation time: - pg_get_indexdef_ddl passes PRETTYFLAG_SCHEMA to the worker, enabling generate_relation_name instead of generate_qualified_relation_name. - pg_get_ruledef_ddl — same flag. - pg_get_constraintdef_body — returns body only (FK targets already use generate_relation_name); emit_local_constraints now builds the ALTER TABLE ctx->qualname ADD CONSTRAINT prefix itself. - pg_get_statisticsobjdef_ddl — uses StatisticsObjIsVisible() to qualify the statistics object name *The schema_qualified=false path still narrows search_path to the base schema, which is what makes all four helpers emit unqualified names for same-schema objects. The option is kept for per-call convenience.* > 3. typedefs.list is missing TableDdlContext and LocalNotNullEntry, so > pgindent leaves the "Type * var" pointer spacing in ~30 places -- for > example the forward declarations at ddlutils.c:226-237: > > static void append_stmt(TableDdlContext * ctx); > > There is also a stale comment at ddlutils.c:1830 in append_column_defs(): > inherited columns are described as "emitted by the INHERITS clause (once > implemented)", but INHERITS is implemented now. > > 4. Related to (2): a temporary table's default output qualifies it with the > session-private pg_temp_NN schema (e.g. CREATE TEMPORARY TABLE pg_temp_3.t > ...), which won't replay anywhere else. A reconstructed temp table should > just be CREATE TEMPORARY TABLE t (...) -- the TEMPORARY keyword already > puts > it in pg_temp, so the schema name should never be emitted. This also falls > out for free under the search_path convention in (2): pg_temp is always in > the effective search_path, so the table is visible and wouldn't be > qualified > in the first place. > > 5. The commit message is out of sync with the code and func-info.sgml on > the > option interface: it still says "include and exclude" and lists plural kind > names (indexes, foreign_keys, triggers, policies, partitions), whereas the > code and docs use only/except and singular names (index, foreign_key, > trigger, policy, partition). Looks like a leftover from the > include/exclude -> only/except rename. (FWIW on the naming itself, > include/exclude is the more common convention for this kind of list > parameter -- pg_dump has --exclude-table/-schema/-extension, etc. -- while > only/except reads more like the SQL keywords.) > Fixed 3, 4 and 5. The v12 patch is ready for review/test. > > Otherwise it looks good. > > Thanks, > Rui > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-06-29T13:38:11Z
Fixed some compiler warnings. The *v13* patch is ready for review/test. On Mon, Jun 29, 2026 at 4:10 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: > > > On Sun, Jun 28, 2026 at 10:14 PM Rui Zhao <zhaorui126@gmail.com> wrote: > >> Hi Akshay, >> >> Nice patch -- server-side CREATE TABLE reconstruction is something we >> want in >> production. I tested v11 on current master and it round-trips correctly >> across the documented coverage, including the recent grammar (VIRTUAL >> columns, NOT ENFORCED, temporal WITHOUT OVERLAPS / PERIOD) and state set >> later via ALTER. A few comments: >> >> 1. Real bug in schema_qualified => false. strip_schema_prefix() strips the >> base prefix anywhere in code position with no token-boundary check, so a >> cross-schema name whose schema ends with the base schema's name gets >> mangled: >> >> postgres=# CREATE SCHEMA xs; >> CREATE SCHEMA >> postgres=# CREATE TABLE xs.reftbl(id int PRIMARY KEY); >> CREATE TABLE >> postgres=# CREATE SCHEMA s; >> CREATE SCHEMA >> postgres=# CREATE TABLE s.orders(id int PRIMARY KEY, ref int >> REFERENCES xs.reftbl(id)); >> CREATE TABLE >> postgres=# SELECT d FROM >> pg_get_table_ddl('s.orders'::regclass,'schema_qualified','false') d; >> d >> >> --------------------------------------------------------------------------------------------- >> CREATE TABLE orders (id integer NOT NULL, ref integer); >> ALTER TABLE orders OWNER TO postgres; >> ALTER TABLE orders ADD CONSTRAINT orders_pkey PRIMARY KEY (id); >> ALTER TABLE orders ADD CONSTRAINT orders_ref_fkey FOREIGN KEY (ref) >> REFERENCES xreftbl(id); >> (4 rows) >> >> The last FK line says REFERENCES xreftbl(id); it should be xs.reftbl(id), >> and the result doesn't replay. Gating the match on a token boundary fixes >> it -- though see (2), which would remove this code path (and the bug) >> entirely. >> > > Fixed. > >> >> 2. Bigger picture: is schema_qualified needed at all? None of the existing >> pg_get_*def / pg_get_*ddl functions have such a knob, so this would be the >> lone exception. The established convention is to let search_path decide >> (generate_relation_name): pg_get_viewdef and pg_get_constraintdef already >> work that way, and pg_get_indexdef supports it too via >> pg_get_indexdef(idx, 0, true) (the PRETTYFLAG_SCHEMA code path). It is >> genuinely what we want in production -- the caller >> controls qualification through search_path. Set it to the table's schema >> for >> unqualified output, or to pg_catalog (or '') for fully-qualified >> schema.table; pg_dump itself dumps under an empty search_path >> (ALWAYS_SECURE_SEARCH_PATH_SQL) for exactly this reason. Following the >> convention would also drop the option and the strip_schema_prefix code >> (and >> this bug). >> >> That last point matters: there's no robust way to strip a schema prefix >> out >> of already-generated SQL by text processing. Doing it safely means >> re-tokenizing arbitrary SQL (string literals, quoted identifiers, dollar >> quotes, comments, casts, operators, ...), and strip_schema_prefix is a >> hand-rolled partial scanner of exactly that. It shows -- it has already >> needed several over-stripping fixes during review (the base name appearing >> inside a string literal, and inside a quoted identifier), and the >> token-boundary case in (1) is yet another. Deciding qualification at >> generation time (generate_relation_name) avoids the whole class: the >> backend's real deparser already gets this right, rather than a post-hoc >> string pass trying to re-derive it. >> > > Done. strip_schema_prefix and append_stripped_stmt are gone. Instead of > post-processing, I added four thin wrappers in ruleutils.c. Let the > deparser decide qualification at generation time: > - pg_get_indexdef_ddl passes PRETTYFLAG_SCHEMA to the worker, enabling > generate_relation_name instead of generate_qualified_relation_name. > - pg_get_ruledef_ddl — same flag. > - pg_get_constraintdef_body — returns body only (FK targets already use > generate_relation_name); emit_local_constraints now builds the ALTER TABLE > ctx->qualname ADD CONSTRAINT prefix itself. > - pg_get_statisticsobjdef_ddl — uses StatisticsObjIsVisible() to qualify > the statistics object name > *The schema_qualified=false path still narrows search_path to the base > schema, which is what makes all four helpers emit unqualified names for > same-schema objects. The option is kept for per-call convenience.* > >> 3. typedefs.list is missing TableDdlContext and LocalNotNullEntry, so >> pgindent leaves the "Type * var" pointer spacing in ~30 places -- for >> example the forward declarations at ddlutils.c:226-237: >> >> static void append_stmt(TableDdlContext * ctx); >> >> There is also a stale comment at ddlutils.c:1830 in append_column_defs(): >> inherited columns are described as "emitted by the INHERITS clause (once >> implemented)", but INHERITS is implemented now. >> >> 4. Related to (2): a temporary table's default output qualifies it with >> the >> session-private pg_temp_NN schema (e.g. CREATE TEMPORARY TABLE pg_temp_3.t >> ...), which won't replay anywhere else. A reconstructed temp table should >> just be CREATE TEMPORARY TABLE t (...) -- the TEMPORARY keyword already >> puts >> it in pg_temp, so the schema name should never be emitted. This also falls >> out for free under the search_path convention in (2): pg_temp is always in >> the effective search_path, so the table is visible and wouldn't be >> qualified >> in the first place. >> >> 5. The commit message is out of sync with the code and func-info.sgml on >> the >> option interface: it still says "include and exclude" and lists plural >> kind >> names (indexes, foreign_keys, triggers, policies, partitions), whereas the >> code and docs use only/except and singular names (index, foreign_key, >> trigger, policy, partition). Looks like a leftover from the >> include/exclude -> only/except rename. (FWIW on the naming itself, >> include/exclude is the more common convention for this kind of list >> parameter -- pg_dump has --exclude-table/-schema/-extension, etc. -- while >> only/except reads more like the SQL keywords.) >> > > Fixed 3, 4 and 5. > > The v12 patch is ready for review/test. > >> >> Otherwise it looks good. >> >> Thanks, >> Rui >> > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-06-29T21:31:41Z
Hello I noticed that there's an inconsistency with schema qualification, if the schema is in the search path: CREATE SCHEMA s; CREATE TABLE s.parent (id int PRIMARY KEY); CREATE TABLE s.t (id int PRIMARY KEY, pid int REFERENCES s.parent(id), name text); CREATE INDEX t_name_idx ON s.t (name); CREATE STATISTICS s.t_stat ON id, pid FROM s.t; SET search_path = s, public; SELECT pg_get_table_ddl('s.t', 'owner', 'false'); outputs: CREATE TABLE s.t (id integer NOT NULL, pid integer, name text); CREATE INDEX t_name_idx ON t USING btree (name); -- should be s.t ALTER TABLE s.t ADD CONSTRAINT t_pid_fkey FOREIGN KEY (pid) REFERENCES parent(id); -- should be s.parent ALTER TABLE s.t ADD CONSTRAINT t_pkey PRIMARY KEY (id); CREATE STATISTICS t_stat ON id, pid FROM t; -- should be s.t In the included testcase: +drop cascades to view v +drop cascades to sequence s +ERROR: relation "parted_range_1" already exists +CONTEXT: SQL statement "CREATE TABLE pgtbl_ddl_test.parted_range_1 PARTITION OF pgtbl_ddl_test.parted_range FOR VALUES FROM (0) TO (100);" +PL/pgSQL function inline_code_block line 22 at EXECUTE is this error expected, doesn't it break the test? -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-07-01T14:12:31Z
Thanks for the continued review. I've rebased and updated the patch. Andrew's commit replaced the VARIADIC text[] option interface on pg_get_role_ddl(), pg_get_tablespace_ddl(), and pg_get_database_ddl() with typed named boolean parameters. pg_get_table_ddl() now follows the exact same convention: pretty, owner, tablespace, and schema_qualified are plain boolean parameters with DEFAULT values, using PG_GETARG_BOOL() directly rather than parse_ddl_options(). The shared DdlOptType / DdlOption / parse_ddl_options infrastructure is gone entirely. *Filtering parameters:* *only_kinds* / *except_kinds* as text[] Based on review feedback the filtering parameters(*include/exclude*) have been revised: The two mutually-exclusive filter arguments are now named *only_kinds* and *except_kinds* (matching the SQL set-operation vocabulary) and take text[] rather than a comma-separated text value (e.g. only_kinds => ARRAY['index','foreign_key']). This provides type safety and allows named-argument syntax. *Note*: only and except are the keywords, and I couldn't find any better name. Suggestions are welcome. The *v14* patch is ready for review/test. On Tue, Jun 30, 2026 at 3:01 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > Hello > > I noticed that there's an inconsistency with schema qualification, if > the schema is in the search path: > > CREATE SCHEMA s; > CREATE TABLE s.parent (id int PRIMARY KEY); > CREATE TABLE s.t (id int PRIMARY KEY, pid int REFERENCES s.parent(id), > name text); > CREATE INDEX t_name_idx ON s.t (name); > CREATE STATISTICS s.t_stat ON id, pid FROM s.t; > SET search_path = s, public; > SELECT pg_get_table_ddl('s.t', 'owner', 'false'); > > outputs: > > CREATE TABLE s.t (id integer NOT NULL, pid integer, name text); > CREATE INDEX t_name_idx ON t USING btree (name); -- should be s.t > ALTER TABLE s.t ADD CONSTRAINT t_pid_fkey FOREIGN KEY (pid) REFERENCES > parent(id); -- should be s.parent > ALTER TABLE s.t ADD CONSTRAINT t_pkey PRIMARY KEY (id); > CREATE STATISTICS t_stat ON id, pid FROM t; -- should be s.t > > > In the included testcase: > > +drop cascades to view v > +drop cascades to sequence s > +ERROR: relation "parted_range_1" already exists > +CONTEXT: SQL statement "CREATE TABLE pgtbl_ddl_test.parted_range_1 > PARTITION OF pgtbl_ddl_test.parted_range FOR VALUES FROM (0) TO > (100);" > +PL/pgSQL function inline_code_block line 22 at EXECUTE > > is this error expected, doesn't it break the test? > > > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-07-01T15:10:49Z
Em qua., 1 de jul. de 2026 às 11:12, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: > pretty, owner, tablespace, and schema_qualified are plain boolean > parameters with DEFAULT values > *Filtering parameters:* *only_kinds* / *except_kinds* as text[] > >> If all parameters are optional, and all parameters are boolean, perhaps you could also make pretty, owner, tablespace, and schema_qualified as optional parts of only_kinds and except_kinds. Therefore, we could call these two ways and the result would be the same. pg_get_table_ddl('idxd'::regclass, owner => false, tablespace => false, except_kinds => '{primary_key}'); pg_get_table_ddl('idxd'::regclass, except_kinds => '{primary_key,tablespace, owner}'); Obviously this way you have to know if owner param is false or it exists on except_kinds. What do you think ? regards Marcos -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-07-01T22:19:51Z
I did some more testing, I noticed one more issue with self referencing foreign keys: CREATE TABLE t (id int PRIMARY KEY, parent_id int REFERENCES t(id)); SELECT pg_get_table_ddl('t'::regclass); -- CREATE TABLE public.t (id integer NOT NULL, parent_id integer); -- ALTER TABLE public.t OWNER TO postgres; -- ALTER TABLE public.t ADD CONSTRAINT t_parent_id_fkey FOREIGN KEY (parent_id) REFERENCES public.t(id); -- ALTER TABLE public.t ADD CONSTRAINT t_pkey PRIMARY KEY (id); It tries to add the foreign key before the primary, and fails with `ERROR: there is no unique constraint matching given keys for referenced table "t"` There's also another issue in schema_qualified false, with partitions in different schemas: CREATE SCHEMA s; CREATE SCHEMA other; CREATE TABLE s.pt (id int, val int) PARTITION BY RANGE (id); CREATE TABLE other.pt_c PARTITION OF s.pt FOR VALUES FROM (0) TO (100); SELECT pg_get_table_ddl('s.pt'::regclass, schema_qualified => false); -- CREATE TABLE pt (id integer, val integer) PARTITION BY RANGE (id); -- ALTER TABLE pt OWNER TO postgres; -- CREATE TABLE pt_c PARTITION OF s.pt FOR VALUES FROM (0) TO (100); -- ALTER TABLE pt_c OWNER TO postgres; The second create table statement references pt as s.pt, which seems incorrect. It is also missing its own schema qualification, which I'm unsure if it is wrong or not. If I interpret the documentation strictly, it isn't the target table, so it should appear with its schema qualification? -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-07-02T02:17:15Z
Hello, Looking at this, one thing that concerns me is the large amount of overlap with dumpTableSchema() in pg_dump. I wonder if it would make sense to separate the SQL generation logic into frontend/backend-shared code so that it could also be used by pg_dump. The catalog lookup would naturally remain separate, but sharing the DDL generation itself would significantly reduce the duplication. By the way, a couple of comments use a Unicode RIGHTWARDS ARROW (U+2192). Please use an ASCII equivalent instead. Regards, -- Kyotaro Horiguchi NTT Open Source Software Center
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-07-02T07:48:51Z
On Wed, Jul 1, 2026 at 8:41 PM Marcos Pegoraro <marcos@f10.com.br> wrote: > Em qua., 1 de jul. de 2026 às 11:12, Akshay Joshi < > akshay.joshi@enterprisedb.com> escreveu: > >> pretty, owner, tablespace, and schema_qualified are plain boolean >> parameters with DEFAULT values >> *Filtering parameters:* *only_kinds* / *except_kinds* as text[] >> >>> If all parameters are optional, and all parameters are boolean, perhaps > you could also make pretty, owner, tablespace, and schema_qualified as > optional parts of only_kinds and except_kinds. > > Therefore, we could call these two ways and the result would be the same. > pg_get_table_ddl('idxd'::regclass, owner => false, tablespace => false, except_kinds > => '{primary_key}'); > pg_get_table_ddl('idxd'::regclass, except_kinds => '{primary_key, > tablespace,owner}'); > > Obviously this way you have to know if owner param is false or it exists > on except_kinds. > What do you think ? > *owner* is the one case where it could work, but to make it consistent with how owner behaves in pg_get_tablespace_ddl and pg_get_database_ddl, we should not add it. *tablespace* doesn't map to a kind at all. It controls the inline TABLESPACE clause within the CREATE TABLE statement body it's a sub-clause, not a separate statement. If we added tablespace as a kind, except_kinds => '{table,tablespace}' would be wrong (if you're skipping the table statement, there's no inline clause to suppress), and except_kinds => '{tablespace}' would imply skipping a standalone statement that doesn't exist. *pretty* and *schema_qualified* are rendering/formatting options, not statement filters. They affect how every statement is rendered — indentation, name qualification — not which statements are emitted. Putting them in except_kinds conflates two orthogonal axes: filtering (what to emit) and formatting (how to emit it). The current design intentionally keeps these separate: only_kinds/except_kinds for statement-level filtering, booleans for rendering and inline-clause control. Merging them would make except_kinds overloaded and harder to document clearly. > > regards > Marcos > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-07-02T08:48:06Z
On Thu, Jul 2, 2026 at 7:47 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote: > Hello, > > Looking at this, one thing that concerns me is the large amount of > overlap with dumpTableSchema() in pg_dump. > > I wonder if it would make sense to separate the SQL generation logic > into frontend/backend-shared code so that it could also be used by > pg_dump. The catalog lookup would naturally remain separate, but > sharing the DDL generation itself would significantly reduce the > duplication. > Thanks for the suggestion. The overlap is real, but sharing the SQL generation logic at the C level runs into a few structural mismatches: dumpTableSchema drives column rendering off pre-populated TableInfo arrays that pg_dump bulk-loaded at startup, while the backend function uses live syscache lookups - a shared builder would need an adapter layer roughly as large as the code it replaces. The pattern PostgreSQL already uses for this kind of sharing is to call backend pg_get_*def functions via SQL from pg_dump — it already does this 20 times for indexes, constraints, triggers, rules, and statistics. The natural long-term path would be for pg_dump to call pg_get_table_ddl() the same way and retire dumpTableSchema, but that is a substantial refactor in its own right and feels out of scope here. By the way, a couple of comments use a Unicode RIGHTWARDS ARROW > (U+2192). Please use an ASCII equivalent instead. > Will fix it in the next v15 patch. > > Regards, > > -- > Kyotaro Horiguchi > NTT Open Source Software Center >
-
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-07-02T10:39:50Z
Thanks for the review, I have fixed the mentioned issue. The v15 patch is ready for review. On Thu, Jul 2, 2026 at 3:50 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > I did some more testing, I noticed one more issue with self > referencing foreign keys: > > CREATE TABLE t (id int PRIMARY KEY, parent_id int REFERENCES t(id)); > SELECT pg_get_table_ddl('t'::regclass); > -- CREATE TABLE public.t (id integer NOT NULL, parent_id integer); > -- ALTER TABLE public.t OWNER TO postgres; > -- ALTER TABLE public.t ADD CONSTRAINT t_parent_id_fkey FOREIGN KEY > (parent_id) REFERENCES public.t(id); > -- ALTER TABLE public.t ADD CONSTRAINT t_pkey PRIMARY KEY (id); > > It tries to add the foreign key before the primary, and fails with > `ERROR: there is no unique constraint matching given keys for > referenced table "t"` > > There's also another issue in schema_qualified false, with partitions > in different schemas: > > CREATE SCHEMA s; > CREATE SCHEMA other; > CREATE TABLE s.pt (id int, val int) PARTITION BY RANGE (id); > CREATE TABLE other.pt_c PARTITION OF s.pt FOR VALUES FROM (0) TO (100); > SELECT pg_get_table_ddl('s.pt'::regclass, schema_qualified => false); > -- CREATE TABLE pt (id integer, val integer) PARTITION BY RANGE (id); > -- ALTER TABLE pt OWNER TO postgres; > -- CREATE TABLE pt_c PARTITION OF s.pt FOR VALUES FROM (0) TO (100); > -- ALTER TABLE pt_c OWNER TO postgres; > > The second create table statement references pt as s.pt, which seems > incorrect. > It is also missing its own schema qualification, which I'm unsure if > it is wrong or not. If I interpret the documentation strictly, it > isn't the target table, so it should appear with its schema > qualification? > > > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-07-02T16:35:41Z
Em qui., 2 de jul. de 2026 às 04:49, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: > *owner* is the one case where it could work, but to make it consistent > with how owner behaves in pg_get_tablespace_ddl and pg_get_database_ddl, we > should not add it. > > *tablespace* doesn't map to a kind at all. It controls the inline > TABLESPACE clause within the CREATE TABLE statement body it's a sub-clause, > not a separate statement. If we added tablespace as a kind, except_kinds => > '{table,tablespace}' would be wrong (if you're skipping the table > statement, there's no inline clause to suppress), and except_kinds => > '{tablespace}' would imply skipping a standalone statement that doesn't > exist. > > *pretty* and *schema_qualified* are rendering/formatting options, not > statement filters. They affect how every statement is rendered — > indentation, name qualification — not which statements are emitted. Putting > them in except_kinds conflates two orthogonal axes: filtering (what to > emit) and formatting (how to emit it). > >> Well, I thought you could continue with the same variable structure, just assigning them beforehand if those values were used in the kind list. So, TableDdlContext continues the same, you just add TABLE_DDL_KIND_NO_OWNER and others to TableDdlKind And before anything you see if TableDdlKind->TABLE_DDL_KIND_NO_OWNER is set then TableDdlContext->no_owner receives that value, just that. regards Marcos -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Zsolt Parragi <zsolt.parragi@percona.com> — 2026-07-02T22:34:00Z
Hello! I can confirm the previous issues fixed, however I also found one more with unique indexes on partitioned tables: CREATE SCHEMA s; CREATE TABLE s.p (id int, region text) PARTITION BY LIST (region); CREATE TABLE s.p_a PARTITION OF s.p FOR VALUES IN ('a'); CREATE UNIQUE INDEX p_uidx ON s.p (id, region); SELECT pg_get_table_ddl('s.p', owner => false); -- CREATE TABLE s.p (id integer, region text) PARTITION BY LIST (region); -- CREATE UNIQUE INDEX p_uidx ON s.p USING btree (id, region); -- CREATE TABLE s.p_a PARTITION OF s.p FOR VALUES IN ('a'); -- CREATE UNIQUE INDEX p_a_id_region_idx ON s.p_a USING btree (id, region); -- fails because index already exists -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-07-03T08:47:25Z
Thanks Zsolt for the review. I have tried to fix most of the *edge cases. *I added 12 new test cases. The v16 patch is ready for review. *Note: Windows-MinGW-Meson *builds are failing but not because of this patch. On Fri, Jul 3, 2026 at 4:04 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote: > Hello! > > I can confirm the previous issues fixed, however I also found one more > with unique indexes on partitioned tables: > > CREATE SCHEMA s; > CREATE TABLE s.p (id int, region text) PARTITION BY LIST (region); > CREATE TABLE s.p_a PARTITION OF s.p FOR VALUES IN ('a'); > CREATE UNIQUE INDEX p_uidx ON s.p (id, region); > > SELECT pg_get_table_ddl('s.p', owner => false); > -- CREATE TABLE s.p (id integer, region text) PARTITION BY LIST (region); > -- CREATE UNIQUE INDEX p_uidx ON s.p USING btree (id, region); > -- CREATE TABLE s.p_a PARTITION OF s.p FOR VALUES IN ('a'); > -- CREATE UNIQUE INDEX p_a_id_region_idx ON s.p_a USING btree (id, > region); -- fails because index already exists > > > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Rui Zhao <zhaorui126@gmail.com> — 2026-07-04T16:35:44Z
Hi Akshay, Re-tested v16 on current master -- builds clean and the recent fixes hold up (self-ref FK after PK, no duplicate partition-child index, cross-schema partition child qualified, schema_qualified => true consistent). A few issues, in severity order; several are places where pg_dump already does the right thing. 1. Clause order: TABLESPACE is emitted before ON COMMIT (ddlutils.c:2042 vs 2054), but the grammar is "... OptWith OnCommitOption OptTableSpace" -- ON COMMIT must come first, so a temp table with both clauses is non-replayable: CREATE TABLESPACE ts1 LOCATION '/path/to/dir'; CREATE TEMP TABLE tt (a int) ON COMMIT DROP TABLESPACE ts1; SELECT d FROM pg_get_table_ddl('tt'::regclass, owner => false) d; -- CREATE TEMPORARY TABLE tt (a integer) TABLESPACE ts1 ON COMMIT DROP; CREATE TEMPORARY TABLE tt (a integer) TABLESPACE ts1 ON COMMIT DROP; -- ERROR: syntax error at or near "ON" Swapping the two blocks so ON COMMIT precedes TABLESPACE fixes it. 2. Child-default override recurses (missing ONLY). emit_child_default_overrides emits the inherited-column default without ONLY (ddlutils.c:2118), so SET DEFAULT recurses into the table's children and reconstructing one table silently rewrites another: CREATE TABLE dpar (x int); CREATE TABLE dch () INHERITS (dpar); CREATE TABLE dgc () INHERITS (dch); ALTER TABLE ONLY dch ALTER COLUMN x SET DEFAULT 5; ALTER TABLE ONLY dgc ALTER COLUMN x SET DEFAULT 10; SELECT d FROM pg_get_table_ddl('dch'::regclass, owner => false) d; -- ALTER TABLE dch ALTER COLUMN x SET DEFAULT 5; -- no ONLY ALTER TABLE dch ALTER COLUMN x SET DEFAULT 5; -- replay this line -- => dgc's default is now 5, not 10 pg_dump uses ALTER TABLE ONLY here for exactly this reason. Partitioned tables hit this especially easily -- any partitioned table with a column default emits a redundant, ONLY-less SET DEFAULT for every partition that merely inherits it: CREATE TABLE p (id int, amt int DEFAULT 5) PARTITION BY LIST (id); CREATE TABLE p_a PARTITION OF p FOR VALUES IN (1); SELECT d FROM pg_get_table_ddl('p'::regclass, owner => false) d; -- CREATE TABLE public.p (id integer, amt integer DEFAULT 5) PARTITION BY LIST (id); -- CREATE TABLE public.p_a PARTITION OF public.p FOR VALUES IN (1); -- ALTER TABLE public.p_a ALTER COLUMN amt SET DEFAULT 5; -- redundant, no ONLY p_a already inherits amt's default from the PARTITION OF, so the third line is redundant; pg_dump instead keeps the default inline on the child and attaches with ALTER TABLE ONLY ... ATTACH PARTITION. A full-hierarchy replay self-corrects (each child's own SET DEFAULT runs last), but a partial replay or a lone emitted statement does not. The commit message lists "child-local DEFAULT overrides on inheritance/partition children" as supported, so this is in scope. (More generally the patch never emits ONLY anywhere; ADD CONSTRAINT, where CHECK / NOT NULL also recurse to children, is worth the same audit.) 3. Inherited-only NOT NULL emitted as local. When a child redeclares an inherited column (attislocal) without restating NOT NULL, the constraint is inherited-only (conislocal = false); collect_local_not_null skips it, but append_column_defs keys off att->attnotnull and emits a bare NOT NULL anyway: CREATE TABLE par (a int NOT NULL); CREATE TABLE chld (a int) INHERITS (par); -- 'a' redeclared, no NOT NULL SELECT d FROM pg_get_table_ddl('chld'::regclass, owner => false) d; -- CREATE TABLE public.chld (a integer NOT NULL) INHERITS (public.par); On replay the child now owns the constraint (conislocal flips false -> true, name regenerates par_a_not_null -> chld_a_not_null), so a later "ALTER TABLE par ALTER a DROP NOT NULL" cascades to the original child but not the reconstructed one. pg_dump emits "a integer" with no NOT NULL here, suppressing it via notnull_islocal (pg_dump.c ~9916). The docs describe this as the intended behavior -- "Inherited columns and constraints ... are not duplicated on inheritance children or partitions" -- so it reads as a documented contract the code doesn't quite meet. The inline CHECK path in this patch already filters on conislocal; the NOT NULL path could do the same. 4. Typed-table STORAGE / COMPRESSION not emitted -- intended? append_column_defs emits per-column STORAGE for ordinary tables, but the typed-table path (append_typed_column_overrides) only handles DEFAULT / NOT NULL / CHECK, so a storage override on a typed table is not reproduced: CREATE TYPE mytype AS (a int, b text); CREATE TABLE typed_t OF mytype; ALTER TABLE typed_t ALTER COLUMN b SET STORAGE external; SELECT d FROM pg_get_table_ddl('typed_t'::regclass, owner => false) d; -- CREATE TABLE public.typed_t OF public.mytype; -- STORAGE not emitted The docs scope the typed-table form to overrides for "defaults, NOT NULL, and CHECK", so this may well be deliberate. But STORAGE is listed in the general per-column coverage, and pg_dump does emit it (ALTER TABLE ONLY ... ALTER COLUMN b SET STORAGE EXTERNAL) -- so it seems worth confirming the omission is intentional rather than an oversight. 5. Minor: is_auto (ddlutils.c:1381) and the identity SEQUENCE NAME check (1681) rebuild the expected auto-name with snprintf, but the backend uses makeObjectName(), which truncates name1/name2 to fit NAMEDATALEN and never the label -- so for long names the "_not_null" / "_seq" suffix is dropped and the checks misfire, emitting a name a short-named table would omit: CREATE TABLE aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (bbbbbbbbbbbbbbbbbbbb int GENERATED ALWAYS AS IDENTITY); SELECT d FROM pg_get_table_ddl( 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'::regclass, owner => false) d; -- CREATE TABLE public.aaaa...(50) (bbbb...(20) integer -- GENERATED ALWAYS AS IDENTITY (SEQUENCE NAME public.aaaa..._seq) -- CONSTRAINT aaaa..._not_null NOT NULL); The default-omission convention in the commit message lists "the auto-generated identity sequence name" among the clauses meant to be dropped, which is exactly what misfires here for long names. It still replays (the names are real), so this one is cosmetic; comparing against makeObjectName(relname, colname, "not_null" / "seq") makes both sides agree. Everything else looks good. Thanks, Rui -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-07-06T10:57:56Z
Thanks for the review, Rui. I’ve addressed all the issues you raised. The v17 patch is now ready for your review. On Sat, Jul 4, 2026 at 10:05 PM Rui Zhao <zhaorui126@gmail.com> wrote: > Hi Akshay, > > Re-tested v16 on current master -- builds clean and the recent fixes hold > up > (self-ref FK after PK, no duplicate partition-child index, cross-schema > partition child qualified, schema_qualified => true consistent). A few > issues, in severity order; several are places where pg_dump already does > the > right thing. > > 1. Clause order: TABLESPACE is emitted before ON COMMIT (ddlutils.c:2042 vs > 2054), but the grammar is "... OptWith OnCommitOption OptTableSpace" -- ON > COMMIT must come first, so a temp table with both clauses is > non-replayable: > > CREATE TABLESPACE ts1 LOCATION '/path/to/dir'; > CREATE TEMP TABLE tt (a int) ON COMMIT DROP TABLESPACE ts1; > SELECT d FROM pg_get_table_ddl('tt'::regclass, owner => false) d; > -- CREATE TEMPORARY TABLE tt (a integer) TABLESPACE ts1 ON COMMIT > DROP; > > CREATE TEMPORARY TABLE tt (a integer) TABLESPACE ts1 ON COMMIT DROP; > -- ERROR: syntax error at or near "ON" > > Swapping the two blocks so ON COMMIT precedes TABLESPACE fixes it. > > 2. Child-default override recurses (missing ONLY). > emit_child_default_overrides > emits the inherited-column default without ONLY (ddlutils.c:2118), so SET > DEFAULT recurses into the table's children and reconstructing one table > silently rewrites another: > > CREATE TABLE dpar (x int); > CREATE TABLE dch () INHERITS (dpar); > CREATE TABLE dgc () INHERITS (dch); > ALTER TABLE ONLY dch ALTER COLUMN x SET DEFAULT 5; > ALTER TABLE ONLY dgc ALTER COLUMN x SET DEFAULT 10; > > SELECT d FROM pg_get_table_ddl('dch'::regclass, owner => false) d; > -- ALTER TABLE dch ALTER COLUMN x SET DEFAULT 5; -- no ONLY > > ALTER TABLE dch ALTER COLUMN x SET DEFAULT 5; -- replay this > line > -- => dgc's default is now 5, not 10 > > pg_dump uses ALTER TABLE ONLY here for exactly this reason. > > Partitioned tables hit this especially easily -- any partitioned table > with a > column default emits a redundant, ONLY-less SET DEFAULT for every partition > that merely inherits it: > > CREATE TABLE p (id int, amt int DEFAULT 5) PARTITION BY LIST (id); > CREATE TABLE p_a PARTITION OF p FOR VALUES IN (1); > > SELECT d FROM pg_get_table_ddl('p'::regclass, owner => false) d; > -- CREATE TABLE public.p (id integer, amt integer DEFAULT 5) > PARTITION BY LIST (id); > -- CREATE TABLE public.p_a PARTITION OF public.p FOR VALUES IN (1); > -- ALTER TABLE public.p_a ALTER COLUMN amt SET DEFAULT 5; -- > redundant, no ONLY > > p_a already inherits amt's default from the PARTITION OF, so the third > line is > redundant; pg_dump instead keeps the default inline on the child and > attaches > with ALTER TABLE ONLY ... ATTACH PARTITION. A full-hierarchy replay > self-corrects (each child's own SET DEFAULT runs last), but a partial > replay or > a lone emitted statement does not. The commit message lists "child-local > DEFAULT overrides on inheritance/partition children" as supported, so this > is > in scope. (More generally the patch never emits ONLY anywhere; ADD > CONSTRAINT, > where CHECK / NOT NULL also recurse to children, is worth the same audit.) > > 3. Inherited-only NOT NULL emitted as local. When a child redeclares an > inherited column (attislocal) without restating NOT NULL, the constraint is > inherited-only (conislocal = false); collect_local_not_null skips it, but > append_column_defs keys off att->attnotnull and emits a bare NOT NULL > anyway: > > CREATE TABLE par (a int NOT NULL); > CREATE TABLE chld (a int) INHERITS (par); -- 'a' redeclared, no NOT > NULL > > SELECT d FROM pg_get_table_ddl('chld'::regclass, owner => false) d; > -- CREATE TABLE public.chld (a integer NOT NULL) INHERITS > (public.par); > > On replay the child now owns the constraint (conislocal flips false -> > true, > name regenerates par_a_not_null -> chld_a_not_null), so a later > "ALTER TABLE par ALTER a DROP NOT NULL" cascades to the original child but > not > the reconstructed one. pg_dump emits "a integer" with no NOT NULL here, > suppressing it via notnull_islocal (pg_dump.c ~9916). The docs describe > this as > the intended behavior -- "Inherited columns and constraints ... are not > duplicated on inheritance children or partitions" -- so it reads as a > documented contract the code doesn't quite meet. The inline CHECK path in > this > patch already filters on conislocal; the NOT NULL path could do the same. > > 4. Typed-table STORAGE / COMPRESSION not emitted -- intended? > append_column_defs > emits per-column STORAGE for ordinary tables, but the typed-table path > (append_typed_column_overrides) only handles DEFAULT / NOT NULL / CHECK, > so a > storage override on a typed table is not reproduced: > > CREATE TYPE mytype AS (a int, b text); > CREATE TABLE typed_t OF mytype; > ALTER TABLE typed_t ALTER COLUMN b SET STORAGE external; > > SELECT d FROM pg_get_table_ddl('typed_t'::regclass, owner => false) d; > -- CREATE TABLE public.typed_t OF public.mytype; -- STORAGE > not emitted > > The docs scope the typed-table form to overrides for "defaults, NOT NULL, > and > CHECK", so this may well be deliberate. But STORAGE is listed in the > general > per-column coverage, and pg_dump does emit it (ALTER TABLE ONLY ... ALTER > COLUMN > b SET STORAGE EXTERNAL) -- so it seems worth confirming the omission is > intentional rather than an oversight. > > 5. Minor: is_auto (ddlutils.c:1381) and the identity SEQUENCE NAME check > (1681) > rebuild the expected auto-name with snprintf, but the backend uses > makeObjectName(), which truncates name1/name2 to fit NAMEDATALEN and never > the > label -- so for long names the "_not_null" / "_seq" suffix is dropped and > the > checks misfire, emitting a name a short-named table would omit: > > CREATE TABLE aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > (bbbbbbbbbbbbbbbbbbbb int GENERATED ALWAYS AS IDENTITY); > > SELECT d FROM pg_get_table_ddl( > 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'::regclass, > owner => false) d; > -- CREATE TABLE public.aaaa...(50) (bbbb...(20) integer > -- GENERATED ALWAYS AS IDENTITY (SEQUENCE NAME public.aaaa..._seq) > -- CONSTRAINT aaaa..._not_null NOT NULL); > > The default-omission convention in the commit message lists "the > auto-generated > identity sequence name" among the clauses meant to be dropped, which is > exactly > what misfires here for long names. It still replays (the names are real), > so > this one is cosmetic; comparing against makeObjectName(relname, colname, > "not_null" / "seq") makes both sides agree. > > Everything else looks good. > > Thanks, > Rui > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Rui Zhao <zhaorui126@gmail.com> — 2026-07-07T16:53:04Z
Hi Akshay, Re-tested v17 on current master (73dfe79fd6) -- all five issues are fixed, and make check passes here. I then ran a round-trip check against pg_dump: for each table, replay the pg_get_table_ddl() output into a clone of the database and diff "pg_dump -t" of both sides (both sides being pg_dump output, any surviving diff is semantic, not formatting). Corpus: the create_sql scenarios from 002_pg_dump.pl, then the whole regression database. 483 of 560 tables round-trip identically; setting aside the documented no-output kinds (trigger/policy), the rest reduce to: 1) serial columns produce non-replayable DDL -- the output references the sequence but never creates it: CREATE TABLE ser (id serial, v text); SELECT d FROM pg_get_table_ddl('ser'::regclass, owner => false) d; -- CREATE TABLE public.ser (id integer DEFAULT nextval('public.ser_id_seq'::regclass) NOT NULL, v text); CREATE TABLE public.ser (id integer DEFAULT nextval('public.ser_id_seq'::regclass) NOT NULL, v text); -- ERROR: relation "public.ser_id_seq" does not exist (on an empty database) pg_dump emits CREATE SEQUENCE + OWNED BY + SET DEFAULT. This accounts for 14 regression failures, including whole partition trees. 2) Partition/inheritance children that diverged from their parent don't survive the PARTITION OF / INHERITS rebuild. Three variants: - a dropped default silently comes back: CREATE TABLE dp (a int DEFAULT 99) PARTITION BY LIST (a); CREATE TABLE dp1 PARTITION OF dp FOR VALUES IN (1); ALTER TABLE ONLY dp1 ALTER COLUMN a DROP DEFAULT; SELECT d FROM pg_get_table_ddl('dp'::regclass, owner => false) d; -- CREATE TABLE public.dp (a integer DEFAULT 99) PARTITION BY LIST (a); -- CREATE TABLE public.dp1 PARTITION OF public.dp FOR VALUES IN (1); SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef WHERE adrelid = 'dp1'::regclass; -- source: (0 rows) -- replayed: 99 => INSERTs into dp1 now get 99, not NULL - a child's own NOT NULL constraint name is lost: CREATE TABLE np (a int NOT NULL) PARTITION BY LIST (a); CREATE TABLE np1 (a int CONSTRAINT np1_nn NOT NULL); ALTER TABLE np ATTACH PARTITION np1 FOR VALUES IN (1); SELECT d FROM pg_get_table_ddl('np'::regclass, owner => false) d; -- CREATE TABLE public.np (a integer NOT NULL) PARTITION BY LIST (a); -- CREATE TABLE public.np1 PARTITION OF public.np FOR VALUES IN (1); SELECT conname FROM pg_constraint WHERE conrelid = 'np1'::regclass AND contype = 'n'; -- source: np1_nn -- replayed: np_a_not_null and when the column itself is inherited, the emitted constraint errors instead of merging: CREATE TABLE p5 (a int); CREATE TABLE c5 () INHERITS (p5); ALTER TABLE c5 ADD CONSTRAINT c5_nn NOT NULL a; ALTER TABLE p5 ADD CONSTRAINT p5_nn NOT NULL a; SELECT d FROM pg_get_table_ddl('c5'::regclass, owner => false) d; -- CREATE TABLE public.c5 () INHERITS (public.p5); -- ALTER TABLE public.c5 ADD CONSTRAINT c5_nn NOT NULL a; ALTER TABLE public.c5 ADD CONSTRAINT c5_nn NOT NULL a; -- replay, p5 recreated first -- ERROR: cannot create not-null constraint "c5_nn" on column "a" of table "c5" -- DETAIL: A not-null constraint named "p5_nn" already exists for this column. - a partition attached from a table with different column order is rebuilt in the parent's order: CREATE TABLE p (a int, b int, c int) PARTITION BY LIST (a); CREATE TABLE c1 (c int, b int, a int); ALTER TABLE p ATTACH PARTITION c1 FOR VALUES IN (1); SELECT d FROM pg_get_table_ddl('p'::regclass, owner => false) d; -- CREATE TABLE public.p (a integer, b integer, c integer) PARTITION BY LIST (a); -- CREATE TABLE public.c1 PARTITION OF public.p FOR VALUES IN (1); -- => replayed c1 columns are a, b, c (source: c, b, a), so -- SELECT * / COPY / positional INSERT all shift pg_dump handles all three: it emits the named constraint inline in the child's CREATE body (the merge with the inherited constraint at CREATE time keeps the local name, and attislocal / conislocal / coninhcount all survive), and falls back to standalone CREATE + ATTACH PARTITION for shapes a PARTITION OF / INHERITS clause can't express. The first two variants have lightweight fixes that keep the PARTITION OF / INHERITS shape -- CREATE TABLE c5 (CONSTRAINT c5_nn NOT NULL a) INHERITS (p5); CREATE TABLE np1 PARTITION OF np (CONSTRAINT np1_nn NOT NULL a) FOR VALUES IN (1); plus a counter-statement for the divergent-default case (ALTER TABLE ONLY dp1 ALTER COLUMN a DROP DEFAULT). Only the reordered-column case really needs the standalone shape. The commit message lists child-local DEFAULT overrides and named NOT NULL constraints as supported, so I'm treating these as bugs rather than scope cuts. 3) emit_typed_column_storage() (ddlutils.c:2331, 2342) emits ALTER TABLE without ONLY; SET STORAGE / SET COMPRESSION recurse: CREATE TYPE mytype AS (a int, b text); CREATE TABLE typed_t OF mytype; ALTER TABLE typed_t ALTER COLUMN b SET STORAGE external; CREATE TABLE tchild () INHERITS (typed_t); ALTER TABLE ONLY tchild ALTER COLUMN b SET STORAGE main; ALTER TABLE public.typed_t ALTER COLUMN b SET STORAGE EXTERNAL; -- replay v17's output -- => tchild's attstorage flips m -> e pg_dump uses ALTER TABLE ONLY here. (The other emitted ALTER TABLEs are fine: OWNER / REPLICA IDENTITY / RLS / SET (options) don't recurse, and ADD CONSTRAINT must stay ONLY-less since the dump relies on its recursion to rebuild the children's suppressed inherited copies.) 4) Some per-table state pg_dump preserves is missing. Index statistics targets: CREATE TABLE ist (c1 int); CREATE INDEX ist_idx ON ist ((c1 + 1)); ALTER INDEX ist_idx ALTER COLUMN 1 SET STATISTICS 400; SELECT d FROM pg_get_table_ddl('ist'::regclass, owner => false) d; -- CREATE TABLE public.ist (c1 integer); -- CREATE INDEX ist_idx ON public.ist USING btree (((c1 + 1))); -- no SET STATISTICS Same story for ALTER TABLE ... CLUSTER ON (the index comes back without the indisclustered marker) and for ALTER TABLE ... DISABLE RULE (the rule is emitted but comes back enabled, which changes behavior -- and rule is a covered kind). 5) emit_indexes doesn't check indisvalid, so an invalid index is emitted as a normal one: CREATE TABLE inv (x int); INSERT INTO inv VALUES (1), (1); CREATE UNIQUE INDEX CONCURRENTLY inv_uidx ON inv (x); -- ERROR: could not create unique index "inv_uidx" (leaves indisvalid = false) SELECT d FROM pg_get_table_ddl('inv'::regclass, owner => false) d; -- CREATE TABLE public.inv (x integer); -- CREATE UNIQUE INDEX inv_uidx ON public.inv USING btree (x); pg_dump skips those (getIndexes: "i.indisvalid OR t2.relkind = 'p'"). 6) Minor: COMMENT ON and GRANT/REVOKE are not emitted. If that's intentional -- like trigger/policy -- worth saying so in the doc. Thanks, Rui -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Akshay Joshi <akshay.joshi@enterprisedb.com> — 2026-07-08T11:56:30Z
Thank you for the thorough round-trip analysis; this caught real bugs. I have fixed several of the issues you highlighted, though a few fall outside this function's current scope. *Out of Scope (Documented in the Function Description)* Issue 1 (Serial Columns): Owned sequences are independent catalog objects. This is the same reason *pg_get_table_ddl* does not emit CREATE TYPE for composite types used in columns. While the nextval() default is correctly emitted, the sequence itself requires separate capture. I have added an explicit note to the documentation to call this out. Issue 2c (Reordered Partition Columns): You correctly noted that this scenario requires a standalone CREATE + ATTACH PARTITION fallback. Detecting column-order divergence and falling back to the non-PARTITION OF syntax is a larger architecture change, so I have deferred it to a follow-up task. I have documented this as a known limitation for now. Issue 6 (COMMENT/GRANT): These are intentionally omitted, consistent with how we handle triggers and policies. I have added them to the documented list of exclusions. The v18 patch is now ready for your review. On Tue, Jul 7, 2026 at 10:23 PM Rui Zhao <zhaorui126@gmail.com> wrote: > Hi Akshay, > > Re-tested v17 on current master (73dfe79fd6) -- all five issues are fixed, > and make check passes here. > > I then ran a round-trip check against pg_dump: for each table, replay the > pg_get_table_ddl() output into a clone of the database and diff > "pg_dump -t" of both sides (both sides being pg_dump output, any surviving > diff is semantic, not formatting). Corpus: the create_sql scenarios from > 002_pg_dump.pl, then the whole regression database. 483 of 560 tables > round-trip identically; setting aside the documented no-output kinds > (trigger/policy), the rest reduce to: > > 1) serial columns produce non-replayable DDL -- the output references the > sequence but never creates it: > > CREATE TABLE ser (id serial, v text); > > SELECT d FROM pg_get_table_ddl('ser'::regclass, owner => false) d; > -- CREATE TABLE public.ser (id integer DEFAULT > nextval('public.ser_id_seq'::regclass) NOT NULL, v text); > > CREATE TABLE public.ser (id integer DEFAULT > nextval('public.ser_id_seq'::regclass) NOT NULL, v text); > -- ERROR: relation "public.ser_id_seq" does not exist (on an > empty database) > > pg_dump emits CREATE SEQUENCE + OWNED BY + SET DEFAULT. This accounts for > 14 regression failures, including whole partition trees. > > 2) Partition/inheritance children that diverged from their parent don't > survive the PARTITION OF / INHERITS rebuild. Three variants: > > - a dropped default silently comes back: > > CREATE TABLE dp (a int DEFAULT 99) PARTITION BY LIST (a); > CREATE TABLE dp1 PARTITION OF dp FOR VALUES IN (1); > ALTER TABLE ONLY dp1 ALTER COLUMN a DROP DEFAULT; > > SELECT d FROM pg_get_table_ddl('dp'::regclass, owner => false) d; > -- CREATE TABLE public.dp (a integer DEFAULT 99) PARTITION BY > LIST (a); > -- CREATE TABLE public.dp1 PARTITION OF public.dp FOR VALUES IN > (1); > > SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef > WHERE adrelid = 'dp1'::regclass; > -- source: (0 rows) > -- replayed: 99 => INSERTs into dp1 now get 99, not NULL > > - a child's own NOT NULL constraint name is lost: > > CREATE TABLE np (a int NOT NULL) PARTITION BY LIST (a); > CREATE TABLE np1 (a int CONSTRAINT np1_nn NOT NULL); > ALTER TABLE np ATTACH PARTITION np1 FOR VALUES IN (1); > > SELECT d FROM pg_get_table_ddl('np'::regclass, owner => false) d; > -- CREATE TABLE public.np (a integer NOT NULL) PARTITION BY LIST > (a); > -- CREATE TABLE public.np1 PARTITION OF public.np FOR VALUES IN > (1); > > SELECT conname FROM pg_constraint > WHERE conrelid = 'np1'::regclass AND contype = 'n'; > -- source: np1_nn > -- replayed: np_a_not_null > > and when the column itself is inherited, the emitted constraint > errors instead of merging: > > CREATE TABLE p5 (a int); > CREATE TABLE c5 () INHERITS (p5); > ALTER TABLE c5 ADD CONSTRAINT c5_nn NOT NULL a; > ALTER TABLE p5 ADD CONSTRAINT p5_nn NOT NULL a; > > SELECT d FROM pg_get_table_ddl('c5'::regclass, owner => false) d; > -- CREATE TABLE public.c5 () INHERITS (public.p5); > -- ALTER TABLE public.c5 ADD CONSTRAINT c5_nn NOT NULL a; > > ALTER TABLE public.c5 ADD CONSTRAINT c5_nn NOT NULL a; -- > replay, p5 recreated first > -- ERROR: cannot create not-null constraint "c5_nn" on > column "a" of table "c5" > -- DETAIL: A not-null constraint named "p5_nn" already > exists for this column. > > - a partition attached from a table with different column order is > rebuilt in the parent's order: > > CREATE TABLE p (a int, b int, c int) PARTITION BY LIST (a); > CREATE TABLE c1 (c int, b int, a int); > ALTER TABLE p ATTACH PARTITION c1 FOR VALUES IN (1); > > SELECT d FROM pg_get_table_ddl('p'::regclass, owner => false) d; > -- CREATE TABLE public.p (a integer, b integer, c integer) > PARTITION BY LIST (a); > -- CREATE TABLE public.c1 PARTITION OF public.p FOR VALUES IN (1); > -- => replayed c1 columns are a, b, c (source: c, b, a), so > -- SELECT * / COPY / positional INSERT all shift > > pg_dump handles all three: it emits the named constraint inline in the > child's CREATE body (the merge with the inherited constraint at CREATE > time keeps the local name, and attislocal / conislocal / coninhcount all > survive), and falls back to standalone CREATE + ATTACH PARTITION for > shapes a PARTITION OF / INHERITS clause can't express. The first two > variants have lightweight fixes that keep the PARTITION OF / INHERITS > shape -- > > CREATE TABLE c5 (CONSTRAINT c5_nn NOT NULL a) INHERITS (p5); > CREATE TABLE np1 PARTITION OF np (CONSTRAINT np1_nn NOT NULL a) > FOR VALUES IN (1); > > plus a counter-statement for the divergent-default case (ALTER TABLE > ONLY dp1 ALTER COLUMN a DROP DEFAULT). Only the reordered-column case > really needs the standalone shape. The commit message lists child-local > DEFAULT overrides and named NOT NULL constraints as supported, so I'm > treating these as bugs rather than scope cuts. > > 3) emit_typed_column_storage() (ddlutils.c:2331, 2342) emits ALTER TABLE > without ONLY; SET STORAGE / SET COMPRESSION recurse: > > CREATE TYPE mytype AS (a int, b text); > CREATE TABLE typed_t OF mytype; > ALTER TABLE typed_t ALTER COLUMN b SET STORAGE external; > CREATE TABLE tchild () INHERITS (typed_t); > ALTER TABLE ONLY tchild ALTER COLUMN b SET STORAGE main; > > ALTER TABLE public.typed_t ALTER COLUMN b SET STORAGE EXTERNAL; > -- replay v17's output > -- => tchild's attstorage flips m -> e > > pg_dump uses ALTER TABLE ONLY here. (The other emitted ALTER TABLEs are > fine: OWNER / REPLICA IDENTITY / RLS / SET (options) don't recurse, and > ADD CONSTRAINT must stay ONLY-less since the dump relies on its recursion > to rebuild the children's suppressed inherited copies.) > > 4) Some per-table state pg_dump preserves is missing. Index statistics > targets: > > CREATE TABLE ist (c1 int); > CREATE INDEX ist_idx ON ist ((c1 + 1)); > ALTER INDEX ist_idx ALTER COLUMN 1 SET STATISTICS 400; > > SELECT d FROM pg_get_table_ddl('ist'::regclass, owner => false) d; > -- CREATE TABLE public.ist (c1 integer); > -- CREATE INDEX ist_idx ON public.ist USING btree (((c1 + 1))); > -- no SET STATISTICS > > Same story for ALTER TABLE ... CLUSTER ON (the index comes back without > the indisclustered marker) and for ALTER TABLE ... DISABLE RULE (the rule > is emitted but comes back enabled, which changes behavior -- and rule is > a covered kind). > > 5) emit_indexes doesn't check indisvalid, so an invalid index is emitted > as a normal one: > > CREATE TABLE inv (x int); > INSERT INTO inv VALUES (1), (1); > CREATE UNIQUE INDEX CONCURRENTLY inv_uidx ON inv (x); > -- ERROR: could not create unique index "inv_uidx" (leaves > indisvalid = false) > > SELECT d FROM pg_get_table_ddl('inv'::regclass, owner => false) d; > -- CREATE TABLE public.inv (x integer); > -- CREATE UNIQUE INDEX inv_uidx ON public.inv USING btree (x); > > pg_dump skips those (getIndexes: "i.indisvalid OR t2.relkind = 'p'"). > > 6) Minor: COMMENT ON and GRANT/REVOKE are not emitted. If that's > intentional -- like trigger/policy -- worth saying so in the doc. > > Thanks, > Rui > -
Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Marcos Pegoraro <marcos@f10.com.br> — 2026-07-08T13:58:26Z
Em qua., 8 de jul. de 2026 às 08:56, Akshay Joshi < akshay.joshi@enterprisedb.com> escreveu: The v18 patch is now ready for your review. On SGML part, schema_qualified is a param which comes before only_kinds. But the <para> explaining schema_qualified is the latest to be explained, why ? Shouldn't it be placed right after tablespace ? regards Marcos