On 2020-Jan-11, Tomas Vondra wrote: > Hi, > > On Thu, Sep 26, 2019 at 01:47:28PM +0000, Luis Carril wrote: > > > > I don't disagree with adding FOREIGN, though. > > > > Your patch is failing the pg_dump TAP tests. Please use > > configure --enable-tap-tests, fix the problems, then resubmit. > > > > Fixed, I've attached a new version. > > This seems like a fairly small and non-controversial patch (I agree with > Alvaro that having the optional FOREIGN seems won't hurt). So barring > objections I'll polish it a bit and push sometime next week.
If we're messing with that code, we may as well reduce cognitive load a little bit and unify all those multiple consecutive appendStringInfo calls into one. (My guess is that this was previously not possible because there were multiple fmtId() calls in the argument list, but that's no longer the case.) -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From e369a559b2f96e3e73cf1bc4a1fefc9890243a7c Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Mon, 13 Jan 2020 12:33:21 -0300 Subject: [PATCH v3] pg_dump: Add FOREIGN to ALTER statements, if appropriate Author: Luis Carril Discussion: https://postgr.es/m/lejpr01mb0185a19b2e7c98e5e2a031f5e7...@lejpr01mb0185.deuprd01.prod.outlook.de --- src/bin/pg_dump/pg_dump.c | 90 ++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 799b6988b7..2866c55e96 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -15583,6 +15583,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) { char *ftoptions = NULL; char *srvname = NULL; + char *foreign = ""; switch (tbinfo->relkind) { @@ -15616,6 +15617,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) ftoptions = pg_strdup(PQgetvalue(res, 0, i_ftoptions)); PQclear(res); destroyPQExpBuffer(query); + + foreign = "FOREIGN "; break; } case RELKIND_MATVIEW: @@ -15957,11 +15960,10 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) continue; appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inherited constraint.\n"); - appendPQExpBuffer(q, "ALTER TABLE ONLY %s ", - qualrelname); - appendPQExpBuffer(q, " ADD CONSTRAINT %s ", - fmtId(constr->dobj.name)); - appendPQExpBuffer(q, "%s;\n", constr->condef); + appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ADD CONSTRAINT %s %s;\n", + foreign, qualrelname, + fmtId(constr->dobj.name), + constr->condef); appendPQExpBufferStr(q, "UPDATE pg_catalog.pg_constraint\n" "SET conislocal = false\n" "WHERE contype = 'c' AND conname = "); @@ -15978,7 +15980,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) { TableInfo *parentRel = parents[k]; - appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT %s;\n", + appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s INHERIT %s;\n", foreign, qualrelname, fmtQualifiedDumpable(parentRel)); } @@ -16084,9 +16086,9 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (!shouldPrintColumn(dopt, tbinfo, j) && tbinfo->notnull[j] && !tbinfo->inhNotNull[j]) { - appendPQExpBuffer(q, "ALTER TABLE ONLY %s ", - qualrelname); - appendPQExpBuffer(q, "ALTER COLUMN %s SET NOT NULL;\n", + appendPQExpBuffer(q, + "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET NOT NULL;\n", + foreign, qualrelname, fmtId(tbinfo->attnames[j])); } @@ -16097,11 +16099,9 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) */ if (tbinfo->attstattarget[j] >= 0) { - appendPQExpBuffer(q, "ALTER TABLE ONLY %s ", - qualrelname); - appendPQExpBuffer(q, "ALTER COLUMN %s ", - fmtId(tbinfo->attnames[j])); - appendPQExpBuffer(q, "SET STATISTICS %d;\n", + appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET STATISTICS %d;\n", + foreign, qualrelname, + fmtId(tbinfo->attnames[j]), tbinfo->attstattarget[j]); } @@ -16134,11 +16134,9 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) */ if (storage != NULL) { - appendPQExpBuffer(q, "ALTER TABLE ONLY %s ", - qualrelname); - appendPQExpBuffer(q, "ALTER COLUMN %s ", - fmtId(tbinfo->attnames[j])); - appendPQExpBuffer(q, "SET STORAGE %s;\n", + appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET STORAGE %s;\n", + foreign, qualrelname, + fmtId(tbinfo->attnames[j]), storage); } } @@ -16148,11 +16146,9 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) */ if (tbinfo->attoptions[j][0] != '\0') { - appendPQExpBuffer(q, "ALTER TABLE ONLY %s ", - qualrelname); - appendPQExpBuffer(q, "ALTER COLUMN %s ", - fmtId(tbinfo->attnames[j])); - appendPQExpBuffer(q, "SET (%s);\n", + appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET (%s);\n", + foreign, qualrelname, + fmtId(tbinfo->attnames[j]), tbinfo->attoptions[j]); } @@ -16162,11 +16158,12 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (tbinfo->relkind == RELKIND_FOREIGN_TABLE && tbinfo->attfdwoptions[j][0] != '\0') { - appendPQExpBuffer(q, "ALTER FOREIGN TABLE %s ", - qualrelname); - appendPQExpBuffer(q, "ALTER COLUMN %s ", - fmtId(tbinfo->attnames[j])); - appendPQExpBuffer(q, "OPTIONS (\n %s\n);\n", + appendPQExpBuffer(q, + "ALTER FOREIGN TABLE %s ALTER COLUMN %s OPTIONS (\n" + " %s\n" + ");\n", + qualrelname, + fmtId(tbinfo->attnames[j]), tbinfo->attfdwoptions[j]); } } @@ -16271,6 +16268,7 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo) PQExpBuffer delq; char *qualrelname; char *tag; + char *foreign; /* Skip if table definition not to be dumped */ if (!tbinfo->dobj.dump || dopt->dataOnly) @@ -16285,15 +16283,15 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo) qualrelname = pg_strdup(fmtQualifiedDumpable(tbinfo)); - appendPQExpBuffer(q, "ALTER TABLE ONLY %s ", - qualrelname); - appendPQExpBuffer(q, "ALTER COLUMN %s SET DEFAULT %s;\n", - fmtId(tbinfo->attnames[adnum - 1]), + foreign = tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : ""; + + appendPQExpBuffer(q, + "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET DEFAULT %s;\n", + foreign, qualrelname, fmtId(tbinfo->attnames[adnum - 1]), adinfo->adef_expr); - appendPQExpBuffer(delq, "ALTER TABLE %s ", - qualrelname); - appendPQExpBuffer(delq, "ALTER COLUMN %s DROP DEFAULT;\n", + appendPQExpBuffer(delq, "ALTER %sTABLE %s ALTER COLUMN %s DROP DEFAULT;\n", + foreign, qualrelname, fmtId(tbinfo->attnames[adnum - 1])); tag = psprintf("%s %s", tbinfo->dobj.name, tbinfo->attnames[adnum - 1]); @@ -16597,6 +16595,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) PQExpBuffer q; PQExpBuffer delq; char *tag = NULL; + char *foreign; /* Skip if not to be dumped */ if (!coninfo->dobj.dump || dopt->dataOnly) @@ -16605,6 +16604,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) q = createPQExpBuffer(); delq = createPQExpBuffer(); + foreign = tbinfo && tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : ""; + if (coninfo->contype == 'p' || coninfo->contype == 'u' || coninfo->contype == 'x') @@ -16623,7 +16624,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) binary_upgrade_set_pg_class_oids(fout, q, indxinfo->dobj.catId.oid, true); - appendPQExpBuffer(q, "ALTER TABLE ONLY %s\n", + appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s\n", foreign, fmtQualifiedDumpable(tbinfo)); appendPQExpBuffer(q, " ADD CONSTRAINT %s ", fmtId(coninfo->dobj.name)); @@ -16713,7 +16714,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) fmtId(indxinfo->dobj.name)); } - appendPQExpBuffer(delq, "ALTER TABLE ONLY %s ", + appendPQExpBuffer(delq, "ALTER %sTABLE ONLY %s ", foreign, fmtQualifiedDumpable(tbinfo)); appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n", fmtId(coninfo->dobj.name)); @@ -16747,13 +16748,13 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) * XXX Potentially wrap in a 'SET CONSTRAINTS OFF' block so that the * current table data is not processed */ - appendPQExpBuffer(q, "ALTER TABLE %s%s\n", + appendPQExpBuffer(q, "ALTER %sTABLE %s%s\n", foreign, only, fmtQualifiedDumpable(tbinfo)); appendPQExpBuffer(q, " ADD CONSTRAINT %s %s;\n", fmtId(coninfo->dobj.name), coninfo->condef); - appendPQExpBuffer(delq, "ALTER TABLE %s%s ", + appendPQExpBuffer(delq, "ALTER %sTABLE %s%s ", foreign, only, fmtQualifiedDumpable(tbinfo)); appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n", fmtId(coninfo->dobj.name)); @@ -16778,13 +16779,13 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) if (coninfo->separate && coninfo->conislocal) { /* not ONLY since we want it to propagate to children */ - appendPQExpBuffer(q, "ALTER TABLE %s\n", + appendPQExpBuffer(q, "ALTER %sTABLE %s\n", foreign, fmtQualifiedDumpable(tbinfo)); appendPQExpBuffer(q, " ADD CONSTRAINT %s %s;\n", fmtId(coninfo->dobj.name), coninfo->condef); - appendPQExpBuffer(delq, "ALTER TABLE %s ", + appendPQExpBuffer(delq, "ALTER %sTABLE %s ", foreign, fmtQualifiedDumpable(tbinfo)); appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n", fmtId(coninfo->dobj.name)); @@ -17376,7 +17377,10 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo) if (tginfo->tgenabled != 't' && tginfo->tgenabled != 'O') { - appendPQExpBuffer(query, "\nALTER TABLE %s ", + char *foreign; + + foreign = tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : ""; + appendPQExpBuffer(query, "\nALTER %sTABLE %s ", foreign, fmtQualifiedDumpable(tbinfo)); switch (tginfo->tgenabled) { -- 2.20.1
