Date: Wed, 10 Apr 2019 18:11:21 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: David Rowley <david.rowley@2ndquadrant.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_dump is broken for partition tablespaces
Message-ID: <20190410221121.GA23248@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="y0ulUmNC+osPPQO6"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190410202152.GA10240@alvherre.pgsql>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--y0ulUmNC+osPPQO6
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2019-Apr-10, Alvaro Herrera wrote:

> but the test immediately does this:
> 
> alter table at_partitioned alter column b type numeric using b::numeric;
> 
> and watch what happens!  (1663 is pg_default)
> 
> alvherre=# select relname, reltablespace from pg_class where relname like 'at_partitioned%';
>        relname        | reltablespace 
> ----------------------+---------------
>  at_partitioned       |             0
>  at_partitioned_a_idx |             0
>  at_partitioned_b_idx |          1663
> (3 filas)
> 
> Outrageous!

This is because ruleutils.c attaches a TABLESPACE clause when asked to
dump an index definition; and tablecmds.c uses ruleutils to deparse the
index definition into something that can be replayed via CREATE INDEX
commands (or ALTER TABLE ADD CONSTRAINT UNIQUE/PRIMARY KEY, if that's
the case.)

This patch (PoC quality) fixes that behavior, but I'm looking to see
what else it breaks.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--y0ulUmNC+osPPQO6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="tblsp.patch"

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 83f72470b10..acb6b09a576 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10567,7 +10567,13 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 							tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
 																foundObject.objectId);
 							tab->changedIndexDefs = lappend(tab->changedIndexDefs,
-															pg_get_indexdef_string(foundObject.objectId));
+															/*
+															 * we include a tablespace clause
+															 * only if it's a plain index.
+															 * This likely breaks some stuff.
+															 */
+															pg_get_indexdef_string(foundObject.objectId,
+																				   relKind == RELKIND_INDEX));
 						}
 					}
 					else if (relKind == RELKIND_SEQUENCE)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 0c7a533e697..41144d84a76 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1142,11 +1142,11 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
  * Returns a palloc'd C string; no pretty-printing.
  */
 char *
-pg_get_indexdef_string(Oid indexrelid)
+pg_get_indexdef_string(Oid indexrelid, bool tablespace)
 {
 	return pg_get_indexdef_worker(indexrelid, 0, NULL,
 								  false, false,
-								  true, true,
+								  tablespace, true,
 								  0, false);
 }
 
@@ -2170,6 +2170,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
 						pfree(options);
 					}
 
+					/* XXX this tablespace works even if it's 0 */
 					tblspc = get_rel_tablespace(indexId);
 					if (OidIsValid(tblspc))
 						appendStringInfo(&buf, " USING INDEX TABLESPACE %s",
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index 7c49e9d0a83..6ff3b841057 100644
--- a/src/include/utils/ruleutils.h
+++ b/src/include/utils/ruleutils.h
@@ -18,7 +18,7 @@
 #include "nodes/pg_list.h"
 
 
-extern char *pg_get_indexdef_string(Oid indexrelid);
+extern char *pg_get_indexdef_string(Oid indexrelid, bool tablespace);
 extern char *pg_get_indexdef_columns(Oid indexrelid, bool pretty);
 
 extern char *pg_get_partkeydef_columns(Oid relid, bool pretty);

--y0ulUmNC+osPPQO6--