Refactor dumping of pg_type_encoding attributes.

This commit drops the GPDB specific TypeStorageOptions
struct and supporting code in favor of adding
pg_type_encoding.typoptions to the getTypes query.

The usage of CatalogId as the hash index caused collisions
with the TypeInfo indexes when TypeStorageOptions was a
unique dumpable object, since the only unique aspect was the
typstorage field. Now, when dumpType is called, we check the
typstorage field and dump the ALTER TABLE...SET DEFAULT ENCODING
TOC entry if needed.

Authored-by: Brent Doil <[email protected]>

Author: bmdoil

src/bin/pg_dump/common.c

@@ -125,7 +125,6 @@ getSchemaData(Archive *fout, int *numTablesPtr)
 	int			numForeignServers;
 	int			numDefaultACLs;
 	int			numEventTriggers;
-	int  		numTypeStorageOptions;
 
 	/* GPDB specific variables */
 	int			numExtProtocols;
@@ -171,10 +170,6 @@ getSchemaData(Archive *fout, int *numTablesPtr)
 	pg_log_info("reading user-defined types");
 	(void) getTypes(fout, &numTypes);
 
-	/* this must be after getFuncs */
-	pg_log_info("reading type storage options");
-	getTypeStorageOptions(fout, &numTypeStorageOptions);
-
 	/* this must be after getFuncs, too */
 	pg_log_info("reading procedural languages");
 	getProcLangs(fout, &numProcLangs);

src/bin/pg_dump/pg_dump.c

@@ -266,7 +266,6 @@ static void dumpForeignServer(Archive *fout, const ForeignServerInfo *srvinfo);
 
 /* GPDB follow upstream style */
 static void dumpExtProtocol(Archive *fout, const ExtProtInfo *ptcinfo);
-static void dumpTypeStorageOptions(Archive *fout, const TypeStorageOptions *tstorageoptions);
 
 static void dumpUserMappings(Archive *fout,
 							 const char *servername, const char *namespace,
@@ -368,6 +367,7 @@ static bool forcePartitionRootLoad(const TableInfo *tbinfo);
 
 
 /* START MPP ADDITION */
+static void dumpTypeStorageOptions(Archive *fout, const TypeInfo *tyinfo);
 static void setExtPartDependency(TableInfo *tblinfo, int numTables);
 static char *format_table_function_columns(Archive *fout, const FuncInfo *finfo, int nallargs,
 							  char **allargtypes,
@@ -5948,6 +5948,7 @@ getTypes(Archive *fout, int *numTypes)
 	int			i_typtype;
 	int			i_typisdefined;
 	int			i_isarray;
+	int			i_typstorage;
 
 	/*
 	 * we include even the built-in types because those may be used as array
@@ -5988,8 +5989,10 @@ getTypes(Archive *fout, int *numTypes)
 						  "ELSE (SELECT relkind FROM pg_class WHERE oid = t.typrelid) END AS typrelkind, "
 						  "t.typtype, t.typisdefined, "
 						  "t.typname[0] = '_' AND t.typelem != 0 AND "
-						  "(SELECT typarray FROM pg_type te WHERE oid = t.typelem) = t.oid AS isarray "
+						  "(SELECT typarray FROM pg_type te WHERE oid = t.typelem) = t.oid AS isarray, "
+							"coalesce(array_to_string(e.typoptions, ', '), '') AS typstorage "
 						  "FROM pg_type t "
+							"LEFT JOIN pg_type_encoding e ON t.oid = e.typid "
 						  "LEFT JOIN pg_init_privs pip ON "
 						  "(t.oid = pip.objoid "
 						  "AND pip.classoid = 'pg_type'::regclass "
@@ -6057,6 +6060,7 @@ getTypes(Archive *fout, int *numTypes)
 	i_typtype = PQfnumber(res, "typtype");
 	i_typisdefined = PQfnumber(res, "typisdefined");
 	i_isarray = PQfnumber(res, "isarray");
+	i_typstorage = PQfnumber(res, "typstorage");
 
 	for (i = 0; i < ntups; i++)
 	{
@@ -6089,6 +6093,8 @@ getTypes(Archive *fout, int *numTypes)
 		else
 			tyinfo[i].isArray = false;
 
+		tyinfo[i].typstorage = pg_strdup(PQgetvalue(res, i, i_typstorage));
+
 		if (tyinfo[i].typtype == 'm')
 			tyinfo[i].isMultirange = true;
 		else
@@ -6157,91 +6163,6 @@ getTypes(Archive *fout, int *numTypes)
 	return tyinfo;
 }
 
-
-
-/*
- * getTypeStorageOptions:
- *	  read all types with storage options in the system catalogs and return them in the
- * TypeStorageOptions* structure
- *
- *	numTypes is set to the number of types with storage options read in
- *
- */
-TypeStorageOptions *
-getTypeStorageOptions(Archive *fout, int *numTypes)
-{
-	PGresult   *res;
-	int			ntups;
-	int			i;
-	PQExpBuffer query = createPQExpBuffer();
-	TypeStorageOptions   *tstorageoptions;
-	int			i_tableoid;
-	int			i_oid;
-	int			i_typname;
-	int			i_typnamespace;
-	int			i_typoptions;
-	int			i_rolname;
-
-	/*
-	 * The following statement used format_type to resolve an internal name to its equivalent sql name.
-	 * The format_type seems to do two things, it translates an internal type name (e.g. bpchar) into its
-	 * sql equivalent (e.g. character), and it puts trailing "[]" on a type if it is an array.
-	 * For any user defined type (ie. oid > 10000) or any type that might be an array (ie. starts with '_'),
-	 * then we will call quote_ident. If the type is a system defined type (i.e. oid <= 10000)
-	 * and can not possibly be an array (i.e. does not start with '_'), then call format_type to get the name. The
-	 * reason we do not call format_type for arrays is that it will return a '[]' on the end, which can not be used
-	 * when dumping the type.
-	 */
-	appendPQExpBuffer(query, "SELECT "
-					  "CASE WHEN t.oid > 10000 OR substring(t.typname from 1 for 1) = '_' "
-					  "THEN quote_ident(t.typname) "
-					  "ELSE pg_catalog.format_type(t.oid, NULL) "
-					  "END  as typname, "
-					  "t.tableoid as tableoid, "
-					  "t.oid AS oid, "
-					  "t.typnamespace AS typnamespace, "
-					  "(%s typowner) as rolname, "
-					  "array_to_string(a.typoptions, ', ') AS typoptions "
-					  "FROM pg_type t "
-					  "JOIN pg_catalog.pg_type_encoding a ON a.typid = t.oid "
-					  "WHERE t.typisdefined = 't'", username_subquery);
-
-	res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
-
-	ntups = PQntuples(res);
-
-	tstorageoptions = (TypeStorageOptions *) pg_malloc(ntups * sizeof(TypeStorageOptions));
-
-	i_tableoid = PQfnumber(res, "tableoid");
-	i_oid = PQfnumber(res, "oid");
-	i_typname = PQfnumber(res, "typname");
-	i_typnamespace = PQfnumber(res, "typnamespace");
-	i_typoptions = PQfnumber(res, "typoptions");
-	i_rolname = PQfnumber(res, "rolname");
-
-	for (i = 0; i < ntups; i++)
-	{
-		tstorageoptions[i].dobj.objType = DO_TYPE_STORAGE_OPTIONS;
-		tstorageoptions[i].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
-		tstorageoptions[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
-		AssignDumpId(&tstorageoptions[i].dobj);
-		tstorageoptions[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_typname));
-		tstorageoptions[i].dobj.namespace = findNamespace(atooid(PQgetvalue(res, i, i_typnamespace)));
-		tstorageoptions[i].typoptions = pg_strdup(PQgetvalue(res, i, i_typoptions));
-		tstorageoptions[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
-	}
-
-	*numTypes = ntups;
-
-	PQclear(res);
-
-	destroyPQExpBuffer(query);
-
-	return tstorageoptions;
-}
-
-
-
 /*
  * getOperators:
  *	  read all operators in the system catalogs and return them in the
@@ -11488,9 +11409,6 @@ dumpDumpableObject(Archive *fout, const DumpableObject *dobj)
 		case DO_TYPE:
 			dumpType(fout, (const TypeInfo *) dobj);
 			break;
-		case DO_TYPE_STORAGE_OPTIONS:
-			dumpTypeStorageOptions(fout, (const TypeStorageOptions *) dobj);
-			break;
 		case DO_SHELL_TYPE:
 			dumpShellType(fout, (const ShellTypeInfo *) dobj);
 			break;
@@ -11871,6 +11789,10 @@ dumpType(Archive *fout, const TypeInfo *tyinfo)
 	else
 		pg_log_warning("typtype of data type \"%s\" appears to be invalid",
 					   tyinfo->dobj.name);
+
+	if (tyinfo->typstorage && *tyinfo->typstorage != '\0')
+		dumpTypeStorageOptions(fout, tyinfo);
+
 }
 
 /*
@@ -12486,36 +12408,28 @@ dumpBaseType(Archive *fout, const TypeInfo *tyinfo)
  *     writes out to fout the ALTER TYPE queries to set default storage options for type
  */
 static void
-dumpTypeStorageOptions(Archive *fout, const TypeStorageOptions *tstorageoptions)
+dumpTypeStorageOptions(Archive *fout, const TypeInfo *tyinfo)
 {
 	PQExpBuffer q;
-	PQExpBuffer delq;
-
 	q = createPQExpBuffer();
-	delq = createPQExpBuffer();
 
-	/*
-	 * Type name is already quoted by caller using quote_ident, hence used
-	 * directly here.
-	 */
 	appendPQExpBuffer(q, "ALTER TYPE %s.",
-					  fmtId(tstorageoptions->dobj.namespace->dobj.name));
+					  fmtId(tyinfo->dobj.namespace->dobj.name));
 	appendPQExpBuffer(q, "%s SET DEFAULT ENCODING (%s);\n",
-					  tstorageoptions->dobj.name,
-					  tstorageoptions->typoptions);
+					  fmtId(tyinfo->dobj.name),
+					  tyinfo->typstorage);
 
 	ArchiveEntry(fout,
-				 tstorageoptions->dobj.catId,
-				 tstorageoptions->dobj.dumpId,
-				 ARCHIVE_OPTS(.tag = tstorageoptions->dobj.name,
-				 			  .namespace = tstorageoptions->dobj.namespace->dobj.name,
-							  .owner = tstorageoptions->rolname,
+				 tyinfo->dobj.catId,
+				 tyinfo->dobj.dumpId,
+				 ARCHIVE_OPTS(.tag = tyinfo->dobj.name,
+							  .namespace = tyinfo->dobj.namespace->dobj.name,
+							  .owner = tyinfo->rolname,
 							  .description = "TYPE STORAGE OPTIONS",
 							  .section = SECTION_PRE_DATA,
 							  .createStmt = q->data));
 
 	destroyPQExpBuffer(q);
-	destroyPQExpBuffer(delq);
 }
 
 /*
@@ -21030,7 +20944,6 @@ addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
 			case DO_TRANSFORM:
 			case DO_BLOB:
 			case DO_EXTPROTOCOL:
-			case DO_TYPE_STORAGE_OPTIONS:
 			case DO_BINARY_UPGRADE:
 				/* Pre-data objects: must come before the pre-data boundary */
 				addObjectDependency(preDataBound, dobj->dumpId);

src/bin/pg_dump/pg_dump.h

@@ -77,7 +77,6 @@ typedef enum
 	DO_BLOB,
 	DO_BLOB_DATA,
 	DO_EXTPROTOCOL,
-	DO_TYPE_STORAGE_OPTIONS,
 	DO_PRE_DATA_BOUNDARY,
 	DO_POST_DATA_BOUNDARY,
 	DO_EVENT_TRIGGER,
@@ -178,12 +177,12 @@ typedef struct _typeInfo
 	 * result of format_type(), which will be quoted if needed, and might be
 	 * schema-qualified too.
 	 */
-	char	   *ftypname;
-	char	   *rolname;		/* name of owner, or empty string */
-	char	   *typacl;
-	char	   *rtypacl;
-	char	   *inittypacl;
-	char	   *initrtypacl;
+	char		*ftypname;
+	char		*rolname;		/* name of owner, or empty string */
+	char		*typacl;
+	char		*rtypacl;
+	char		*inittypacl;
+	char		*initrtypacl;
 	Oid			typelem;
 	Oid			typrelid;
 	char		typrelkind;		/* 'r', 'v', 'c', etc */
@@ -196,24 +195,10 @@ typedef struct _typeInfo
 	/* If it's a domain, we store links to its constraints here: */
 	int			nDomChecks;
 	struct _constraintInfo *domChecks;
+	char		*typstorage; /* GPDB: store the type's encoding clause */
 } TypeInfo;
 
 
-typedef struct _typeStorageOptions
-{
-	DumpableObject dobj;
-
-	/*
-	 * Note: dobj.name is the pg_type.typname entry.  format_type() might
-	 * produce something different than typname
-	 */
-	char     *typnamespace;
-	char     *typoptions; /* storage options */
-	char     *rolname;		/* name of owner, or empty string */
-} TypeStorageOptions;
-
-
-
 typedef struct _shellTypeInfo
 {
 	DumpableObject dobj;
@@ -779,7 +764,6 @@ extern void getPublicationTables(Archive *fout, TableInfo tblinfo[],
 extern void getSubscriptions(Archive *fout);
 
 /* START MPP ADDITION */
-extern TypeStorageOptions *getTypeStorageOptions(Archive *fout, int *numTypes);
 extern ExtProtInfo *getExtProtocols(Archive *fout, int *numExtProtocols);
 extern BinaryUpgradeInfo *getBinaryUpgradeObjects(void);
 

src/bin/pg_dump/pg_dump_sort.c

@@ -138,7 +138,6 @@ static const int dbObjectTypePriority[] =
 	21,							/* DO_BLOB */
 	25,							/* DO_BLOB_DATA */
 	8,							/* DO_EXTPROTOCOL */
-	21,							/* DO_TYPE_STORAGE_OPTIONS */
 	1,							/* DO_BINARY_UPGRADE */
 	22,							/* DO_PRE_DATA_BOUNDARY */
 	26,							/* DO_POST_DATA_BOUNDARY */
@@ -1287,11 +1286,6 @@ describeDumpableObject(DumpableObject *obj, char *buf, int bufsize)
 					 "TYPE %s  (ID %d OID %u)",
 					 obj->name, obj->dumpId, obj->catId.oid);
 			return;
-		case DO_TYPE_STORAGE_OPTIONS:
-			snprintf(buf, bufsize,
-					 "TYPE STORAGE OPTIONS FOR TYPE %s.%s  (ID %d OID %u) OPTIONS %s",
-					 ((TypeStorageOptions *)obj)->typnamespace, obj->name, obj->dumpId, obj->catId.oid, ((TypeStorageOptions *)obj)->typoptions);
-			return;
 		case DO_SHELL_TYPE:
 			snprintf(buf, bufsize,
 					 "SHELL TYPE %s  (ID %d OID %u)",

src/bin/pg_dump/t/002_pg_dump.pl

@@ -2022,6 +2022,16 @@
 		unlike => { exclude_dump_test_schema => 1, },
 	},
 
+	'ALTER TYPE dump_test.int42 SET DEFAULT ENCODING' => {
+		create_order => 43,
+		create_sql => 'ALTER TYPE dump_test.int42 SET DEFAULT ENCODING
+		(compresstype=rle_type, blocksize=8192, compresslevel=4);',
+		regexp => qr/^\QALTER TYPE dump_test.int42 SET DEFAULT ENCODING (compresstype=rle_type, blocksize=8192, compresslevel=4);\E/m,
+		like =>
+			{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => { exclude_dump_test_schema => 1, },
+	},
+
 	'CREATE FOREIGN DATA WRAPPER dummy' => {
 		create_order => 35,
 		create_sql   => 'CREATE FOREIGN DATA WRAPPER dummy;',