public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v24 7/7] Add support for PRESERVE
3+ messages / 3 participants
[nested] [flat]

* [PATCH v24 7/7] Add support for PRESERVE
@ 2021-02-10 06:19  Dilip Kumar <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Dilip Kumar @ 2021-02-10 06:19 UTC (permalink / raw)

Now the compression method can be changed without forcing a table
rewrite, by including the old method in the PRESERVE list. �For
supporting this the column will maintain the dependency with all
the supported compression methods. �So whenever the compression
method is altered the dependency is added with the new compression
method and the dependency is removed for all the old compression
methods which are not given in the preserve list. �If PRESERVE ALL
is given then all the dependency is maintained.

Dilip Kumar based on the patches from Ildus Kurbangaliev.
Design input from Tomas Vondra and Robert Haas
Reviewed by Robert Haas, Tomas Vondra, Alexander Korotkov and Justin Pryzby
---
 doc/src/sgml/ref/alter_table.sgml           |  10 +-
 src/backend/catalog/pg_depend.c             |   7 +
 src/backend/commands/Makefile               |   1 +
 src/backend/commands/compressioncmds.c      | 300 ++++++++++++++++++++
 src/backend/commands/tablecmds.c            | 126 ++++----
 src/backend/executor/nodeModifyTable.c      |  12 +-
 src/backend/nodes/copyfuncs.c               |  17 +-
 src/backend/nodes/equalfuncs.c              |  15 +-
 src/backend/nodes/outfuncs.c                |  15 +-
 src/backend/parser/gram.y                   |  52 +++-
 src/backend/parser/parse_utilcmd.c          |   2 +-
 src/bin/pg_dump/pg_dump.c                   | 101 +++++++
 src/bin/pg_dump/pg_dump.h                   |  15 +-
 src/bin/psql/tab-complete.c                 |   7 +
 src/include/commands/defrem.h               |   7 +
 src/include/commands/tablecmds.h            |   2 +
 src/include/nodes/nodes.h                   |   1 +
 src/include/nodes/parsenodes.h              |  16 +-
 src/test/regress/expected/compression.out   |  37 ++-
 src/test/regress/expected/compression_1.out |  39 ++-
 src/test/regress/expected/create_index.out  |  56 ++--
 src/test/regress/sql/compression.sql        |   9 +
 22 files changed, 739 insertions(+), 108 deletions(-)
 create mode 100644 src/backend/commands/compressioncmds.c

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 0bd0c1a503..c9f443a59c 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -54,7 +54,7 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET ( <replaceable class="parameter">attribute_option</replaceable> = <replaceable class="parameter">value</replaceable> [, ... ] )
     ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> RESET ( <replaceable class="parameter">attribute_option</replaceable> [, ... ] )
     ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
-    ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET COMPRESSION <replaceable class="parameter">compression_method</replaceable>
+    ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET COMPRESSION <replaceable class="parameter">compression_method</replaceable> [ PRESERVE (<replaceable class="parameter">compression_preserve_list</replaceable>) | PRESERVE ALL ]
     ADD <replaceable class="parameter">table_constraint</replaceable> [ NOT VALID ]
     ADD <replaceable class="parameter">table_constraint_using_index</replaceable>
     ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
@@ -387,7 +387,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
 
    <varlistentry>
     <term>
-     <literal>SET COMPRESSION <replaceable class="parameter">compression_method</replaceable></literal>
+     <literal>SET COMPRESSION <replaceable class="parameter">compression_method</replaceable> [ PRESERVE (<replaceable class="parameter">compression_preserve_list</replaceable>) | PRESERVE ALL ]</literal>
     </term>
     <listitem>
      <para>
@@ -395,6 +395,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       methods are <literal>pglz</literal> and <literal>lz4</literal>.
       <literal>lz4</literal> is available only if <literal>--with-lz4</literal>
       was used when building <productname>PostgreSQL</productname>.
+      The <literal>PRESERVE</literal> list contains a list of compression
+      methods used in the column and determines which of them may be kept.
+      Without <literal>PRESERVE</literal> or if any of the pre-existing
+      compression methods are not preserved, the table will be rewritten.  If
+      <literal>PRESERVE ALL</literal> is specified, then all of the existing
+      methods will be preserved and the table will not be rewritten.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 63da24322d..dd376484b7 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -17,6 +17,7 @@
 #include "access/genam.h"
 #include "access/htup_details.h"
 #include "access/table.h"
+#include "catalog/pg_am.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
 #include "catalog/pg_collation.h"
@@ -125,6 +126,12 @@ recordMultipleDependencies(const ObjectAddress *depender,
 				if (referenced->objectId == DEFAULT_COLLATION_OID)
 					ignore_systempin = true;
 			}
+			/*
+			 * Record the dependency on compression access method for handling
+			 * preserve.
+			 */
+			if (referenced->classId == AccessMethodRelationId)
+				ignore_systempin = true;
 		}
 		else
 			Assert(!version);
diff --git a/src/backend/commands/Makefile b/src/backend/commands/Makefile
index e8504f0ae4..a7395ad77d 100644
--- a/src/backend/commands/Makefile
+++ b/src/backend/commands/Makefile
@@ -21,6 +21,7 @@ OBJS = \
 	cluster.o \
 	collationcmds.o \
 	comment.o \
+	compressioncmds.o \
 	constraint.o \
 	conversioncmds.o \
 	copy.o \
diff --git a/src/backend/commands/compressioncmds.c b/src/backend/commands/compressioncmds.c
new file mode 100644
index 0000000000..fd6db24e7f
--- /dev/null
+++ b/src/backend/commands/compressioncmds.c
@@ -0,0 +1,300 @@
+/*-------------------------------------------------------------------------
+ *
+ * compressioncmds.c
+ *	  Routines for SQL commands for attribute compression methods
+ *
+ * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/commands/compressioncmds.c
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "access/compressamapi.h"
+#include "access/heapam.h"
+#include "access/htup_details.h"
+#include "access/reloptions.h"
+#include "catalog/catalog.h"
+#include "catalog/dependency.h"
+#include "catalog/indexing.h"
+#include "catalog/pg_attribute.h"
+#include "catalog/pg_depend.h"
+#include "commands/defrem.h"
+#include "commands/tablecmds.h"
+#include "miscadmin.h"
+#include "nodes/parsenodes.h"
+#include "utils/builtins.h"
+#include "utils/fmgroids.h"
+#include "utils/lsyscache.h"
+
+/*
+ * Get list of all supported compression methods for the given attribute.
+ *
+ * We maintain dependency of the attribute on the pg_am row for the current
+ * compression AM and all the preserved compression AM.  So scan pg_depend and
+ * find the column dependency on the pg_am.  Collect the list of access method
+ * oids on which this attribute has a dependency.
+ */
+static List *
+lookup_attribute_compression(Oid attrelid, AttrNumber attnum, List *oldcmoids)
+{
+	LOCKMODE	lock = AccessShareLock;
+	HeapTuple	tup;
+	Relation	rel;
+	SysScanDesc scan;
+	ScanKeyData key[3];
+	List	   *cmoids = NIL;
+
+	rel = table_open(DependRelationId, lock);
+
+	ScanKeyInit(&key[0],
+				Anum_pg_depend_classid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(RelationRelationId));
+	ScanKeyInit(&key[1],
+				Anum_pg_depend_objid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(attrelid));
+	ScanKeyInit(&key[2],
+				Anum_pg_depend_objsubid,
+				BTEqualStrategyNumber, F_INT4EQ,
+				Int32GetDatum((int32) attnum));
+
+	scan = systable_beginscan(rel, DependDependerIndexId, true,
+							  NULL, 3, key);
+
+	while (HeapTupleIsValid(tup = systable_getnext(scan)))
+	{
+		Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
+
+		if (depform->refclassid == AccessMethodRelationId)
+			cmoids = list_append_unique_oid(cmoids, depform->refobjid);
+	}
+
+	systable_endscan(scan);
+	table_close(rel, lock);
+
+	return cmoids;
+}
+
+/*
+ * Remove the attribute dependency on the old compression methods
+ *
+ * Scan the pg_depend and search the attribute dependency on the pg_am.  Remove
+ * dependency on previous am which is not preserved.  The list of non-preserved
+ * AMs is given in cmoids.
+ */
+static void
+remove_old_dependencies(Oid attrelid, AttrNumber attnum, List *cmoids)
+{
+	LOCKMODE	lock = AccessShareLock;
+	HeapTuple	tup;
+	Relation	rel;
+	SysScanDesc scan;
+	ScanKeyData key[3];
+
+	rel = table_open(DependRelationId, lock);
+
+	ScanKeyInit(&key[0],
+				Anum_pg_depend_classid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(RelationRelationId));
+	ScanKeyInit(&key[1],
+				Anum_pg_depend_objid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(attrelid));
+	ScanKeyInit(&key[2],
+				Anum_pg_depend_objsubid,
+				BTEqualStrategyNumber, F_INT4EQ,
+				Int32GetDatum((int32) attnum));
+
+	scan = systable_beginscan(rel, DependDependerIndexId, true,
+							  NULL, 3, key);
+
+	while (HeapTupleIsValid(tup = systable_getnext(scan)))
+	{
+		Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
+
+		if (list_member_oid(cmoids, depform->refobjid))
+		{
+			Assert(depform->refclassid == AccessMethodRelationId);
+			CatalogTupleDelete(rel, &tup->t_self);
+		}
+	}
+
+	systable_endscan(scan);
+	table_close(rel, lock);
+}
+
+/*
+ * Check whether the given compression method oid is supported by
+ * the target attribute.
+ */
+bool
+IsCompressionSupported(Form_pg_attribute att, Oid cmoid)
+{
+	List	   *cmoids = NIL;
+
+	/* Check whether it is same as the current compression oid */
+	if (cmoid == att->attcompression)
+		return true;
+
+	/* Check the oid in all preserved compresion methods */
+	cmoids = lookup_attribute_compression(att->attrelid, att->attnum, NULL);
+	if (list_member_oid(cmoids, cmoid))
+		return true;
+	else
+		return false;
+}
+
+/*
+ * In binary upgrade mode add the dependencies for all the preserved compression
+ * method.
+ */
+static void
+BinaryUpgradeAddPreserve(Form_pg_attribute att, List *preserve)
+{
+	ListCell   *cell;
+
+	foreach(cell, preserve)
+	{
+		char   *cmname_p = strVal(lfirst(cell));
+		Oid		cmoid_p = get_compression_am_oid(cmname_p, false);
+
+		add_column_compression_dependency(att->attrelid, att->attnum, cmoid_p);
+	}
+}
+
+/*
+ * Get the compression method oid based on the compression method name.  When
+ * compression is not specified returns default attribute compression.  It is
+ * possible case for CREATE TABLE and ADD COLUMN commands where COMPRESSION
+ * syntax is optional.
+ *
+ * For ALTER command, check all the supported compression methods for the
+ * attribute and if the preserve list is not passed or some of the old
+ * compression methods are not given in the preserved list then delete
+ * dependency from the old compression methods and force the table rewrite.
+ */
+Oid
+GetAttributeCompression(Form_pg_attribute att, ColumnCompression *compression,
+						bool *need_rewrite)
+{
+	Oid			cmoid;
+	char		typstorage = get_typstorage(att->atttypid);
+	ListCell   *cell;
+
+	/*
+	 * No compression for the plain/external storage, refer comments atop
+	 * attcompression parameter in pg_attribute.h
+	 */
+	if (!IsStorageCompressible(typstorage))
+	{
+		if (compression == NULL)
+			return InvalidOid;
+
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("column data type %s does not support compression",
+						format_type_be(att->atttypid))));
+	}
+
+	/* fallback to default compression if it's not specified */
+	if (compression == NULL)
+		return GetDefaultToastCompression();
+
+	cmoid = get_compression_am_oid(compression->cmname, false);
+
+#ifndef HAVE_LIBLZ4
+	if (cmoid == LZ4_COMPRESSION_AM_OID)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("not built with lz4 support")));
+#endif
+
+	/*
+	 * Determine if the column needs rewrite or not. Rewrite conditions: SET
+	 * COMPRESSION without PRESERVE - SET COMPRESSION with PRESERVE but not
+	 * with full list of previous access methods.
+	 */
+	if (need_rewrite != NULL)
+	{
+		List	   *previous_cmoids = NIL;
+
+		*need_rewrite = false;
+
+		/*
+		 * In binary upgrade mode, just create a dependency on all preserved
+		 * methods.
+		 */
+		if (IsBinaryUpgrade)
+		{
+			BinaryUpgradeAddPreserve(att, compression->preserve);
+			return cmoid;
+		}
+
+		/* If we have preserved all then rewrite is not required */
+		if (compression->preserve_all)
+			return cmoid;
+
+		previous_cmoids =
+			lookup_attribute_compression(att->attrelid, att->attnum, NULL);
+
+		foreach(cell, compression->preserve)
+		{
+			char   *cmname_p = strVal(lfirst(cell));
+			Oid		cmoid_p = get_compression_am_oid(cmname_p, false);
+
+			if (!list_member_oid(previous_cmoids, cmoid_p))
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							errmsg("\"%s\" compression access method cannot be preserved", cmname_p)));
+
+			/*
+			 * Remove from previous list, also protect from duplicate
+			 * entries in the PRESERVE list
+			 */
+			previous_cmoids = list_delete_oid(previous_cmoids, cmoid_p);
+		}
+
+		/* delete the current cmoid from the list */
+		previous_cmoids = list_delete_oid(previous_cmoids, cmoid);
+
+		/*
+		 * If the list of previous Oids is not empty after deletions then
+		 * we need to rewrite tuples in the table.  Also remove the dependency
+		 * on the old compression methods which are no longer preserved.
+		 */
+		if (list_length(previous_cmoids) != 0)
+		{
+			remove_old_dependencies(att->attrelid, att->attnum,
+									previous_cmoids);
+			*need_rewrite = true;
+		}
+
+		/* Cleanup */
+		list_free(previous_cmoids);
+	}
+
+	return cmoid;
+}
+
+/*
+ * Construct ColumnCompression node from the compression method oid.
+ */
+ColumnCompression *
+MakeColumnCompression(Oid attcompression)
+{
+	ColumnCompression *node;
+
+	if (!OidIsValid(attcompression))
+		return NULL;
+
+	node = makeNode(ColumnCompression);
+	node->cmname = get_am_name(attcompression);
+
+	return node;
+}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 586a92f0c1..2a1841c353 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -530,7 +530,9 @@ static void ATExecEnableRowSecurity(Relation rel);
 static void ATExecDisableRowSecurity(Relation rel);
 static void ATExecForceNoForceRowSecurity(Relation rel, bool force_rls);
 static ObjectAddress ATExecSetCompression(AlteredTableInfo *tab, Relation rel,
-					 const char *column, Node *newValue, LOCKMODE lockmode);
+										  const char *column,
+										  ColumnCompression *compression,
+										  LOCKMODE lockmode);
 
 static void index_copy_data(Relation rel, RelFileNode newrnode);
 static const char *storage_name(char c);
@@ -562,7 +564,6 @@ static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx,
 static List *GetParentedForeignKeyRefs(Relation partition);
 static void ATDetachCheckNoForeignKeyRefs(Relation partition);
 static void ATExecAlterCollationRefreshVersion(Relation rel, List *coll);
-static Oid GetAttributeCompression(Form_pg_attribute att, char *compression);
 
 /* ----------------------------------------------------------------
  *		DefineRelation
@@ -587,6 +588,7 @@ ObjectAddress
 DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 			   ObjectAddress *typaddress, const char *queryString)
 {
+	int			i;
 	char		relname[NAMEDATALEN];
 	Oid			namespaceId;
 	Oid			relationId;
@@ -865,7 +867,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 			relkind == RELKIND_PARTITIONED_TABLE ||
 			relkind == RELKIND_MATVIEW)
 			attr->attcompression =
-				GetAttributeCompression(attr, colDef->compression);
+				GetAttributeCompression(attr, colDef->compression, NULL);
 		else
 			attr->attcompression = InvalidOid;
 	}
@@ -935,6 +937,20 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	 */
 	rel = relation_open(relationId, AccessExclusiveLock);
 
+	/*
+	 * Add the dependency on the respective compression AM for the relation
+	 * attributes.
+	 */
+	for (i = 0; i < (RelationGetDescr(rel))->natts; i++)
+	{
+		Form_pg_attribute attr;
+
+		attr = TupleDescAttr(RelationGetDescr(rel), i);
+		if (OidIsValid(attr->attcompression))
+			add_column_compression_dependency(attr->attrelid, attr->attnum,
+											  attr->attcompression);
+	}
+
 	/*
 	 * Now add any newly specified column default and generation expressions
 	 * to the new relation.  These are passed to us in the form of raw
@@ -2415,16 +2431,17 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 				/* Copy/check compression parameter */
 				if (OidIsValid(attribute->attcompression))
 				{
-					char *compression = get_am_name(attribute->attcompression);
+					ColumnCompression *compression =
+							MakeColumnCompression(attribute->attcompression);
 
 					if (!def->compression)
 						def->compression = compression;
-					else if (strcmp(def->compression, compression) != 0)
+					else if (strcmp(def->compression->cmname, compression->cmname))
 						ereport(ERROR,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("column \"%s\" has a compression method conflict",
 										attributeName),
-								 errdetail("%s versus %s", def->compression, compression)));
+								 errdetail("%s versus %s", def->compression->cmname, compression->cmname)));
 				}
 
 				def->inhcount++;
@@ -2461,7 +2478,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 				def->collOid = attribute->attcollation;
 				def->constraints = NIL;
 				def->location = -1;
-				def->compression = get_am_name(attribute->attcompression);
+				def->compression = MakeColumnCompression(
+											attribute->attcompression);
 				inhSchema = lappend(inhSchema, def);
 				newattmap->attnums[parent_attno - 1] = ++child_attno;
 			}
@@ -2712,12 +2730,12 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 					def->compression = newdef->compression;
 				else if (newdef->compression)
 				{
-					if (strcmp(def->compression, newdef->compression))
+					if (strcmp(def->compression->cmname, newdef->compression->cmname))
 						ereport(ERROR,
 								(errcode(ERRCODE_DATATYPE_MISMATCH),
 								 errmsg("column \"%s\" has a compression method conflict",
 										attributeName),
-								 errdetail("%s versus %s", def->compression, newdef->compression)));
+								 errdetail("%s versus %s", def->compression->cmname, newdef->compression->cmname)));
 				}
 
 				/* Mark the column as locally defined */
@@ -4908,7 +4926,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			ATExecAlterCollationRefreshVersion(rel, cmd->object);
 			break;
 		case AT_SetCompression:
-			address = ATExecSetCompression(tab, rel, cmd->name, cmd->def,
+			address = ATExecSetCompression(tab, rel, cmd->name,
+										   (ColumnCompression *) cmd->def,
 										   lockmode);
 			break;
 		default:				/* oops */
@@ -6414,7 +6433,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	if (rel->rd_rel->relkind == RELKIND_RELATION ||
 		rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 		attribute.attcompression = GetAttributeCompression(&attribute,
-														   colDef->compression);
+														   colDef->compression,
+														   NULL);
 	else
 		attribute.attcompression = InvalidOid;
 
@@ -6589,6 +6609,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 */
 	add_column_datatype_dependency(myrelid, newattnum, attribute.atttypid);
 	add_column_collation_dependency(myrelid, newattnum, attribute.attcollation);
+	add_column_compression_dependency(myrelid, newattnum, attribute.attcompression);
 
 	/*
 	 * Propagate to children as appropriate.  Unlike most other ALTER
@@ -6736,6 +6757,28 @@ add_column_collation_dependency(Oid relid, int32 attnum, Oid collid)
 	}
 }
 
+/*
+ * Install a dependency for compression on its column.
+ *
+ * This is used for identifying all the supported compression methods
+ * (current and preserved) for a attribute.
+ *
+ * If dependency is already there the whole thing is skipped.
+ */
+void
+add_column_compression_dependency(Oid relid, int32 attnum, Oid cmoid)
+{
+	ObjectAddress acref,
+		attref;
+
+	Assert(relid > 0 && attnum > 0);
+
+	ObjectAddressSet(acref, AccessMethodRelationId, cmoid);
+	ObjectAddressSubSet(attref, RelationRelationId, relid, attnum);
+
+	recordMultipleDependencies(&attref, &acref, 1, DEPENDENCY_NORMAL, true);
+}
+
 /*
  * ALTER TABLE ALTER COLUMN DROP NOT NULL
  */
@@ -11867,7 +11910,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 			  foundDep->refobjid == attTup->attcollation) &&
 			!(foundDep->refclassid == RelationRelationId &&
 			  foundDep->refobjid == RelationGetRelid(rel) &&
-			  foundDep->refobjsubid != 0)
+			  foundDep->refobjsubid != 0) &&
+			  foundDep->refclassid != AccessMethodRelationId
 			)
 			elog(ERROR, "found unexpected dependency for column: %s",
 				 getObjectDescription(&foundObject, false));
@@ -11982,6 +12026,11 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 	add_column_datatype_dependency(RelationGetRelid(rel), attnum, targettype);
 	add_column_collation_dependency(RelationGetRelid(rel), attnum, targetcollid);
 
+	/* Create dependency for new attribute compression */
+	if (OidIsValid(attTup->attcompression))
+		add_column_compression_dependency(RelationGetRelid(rel), attnum,
+										  attTup->attcompression);
+
 	/*
 	 * Drop any pg_statistic entry for the column, since it's now wrong type
 	 */
@@ -15086,24 +15135,21 @@ static ObjectAddress
 ATExecSetCompression(AlteredTableInfo *tab,
 					 Relation rel,
 					 const char *column,
-					 Node *newValue,
+					 ColumnCompression *compression,
 					 LOCKMODE lockmode)
 {
 	Relation	attrel;
 	HeapTuple	tuple;
 	Form_pg_attribute atttableform;
 	AttrNumber	attnum;
-	char	   *compression;
 	char		typstorage;
 	Oid			cmoid;
+	bool		need_rewrite;
 	Datum		values[Natts_pg_attribute];
 	bool		nulls[Natts_pg_attribute];
 	bool		replace[Natts_pg_attribute];
 	ObjectAddress address;
 
-	Assert(IsA(newValue, String));
-	compression = strVal(newValue);
-
 	attrel = table_open(AttributeRelationId, RowExclusiveLock);
 
 	tuple = SearchSysCacheAttName(RelationGetRelid(rel), column);
@@ -15136,11 +15182,16 @@ ATExecSetCompression(AlteredTableInfo *tab,
 	memset(replace, false, sizeof(replace));
 
 	/* Get the attribute compression method. */
-	cmoid = GetAttributeCompression(atttableform, compression);
+	cmoid = GetAttributeCompression(atttableform, compression, &need_rewrite);
 
 	if (atttableform->attcompression != cmoid)
+		add_column_compression_dependency(atttableform->attrelid,
+										  atttableform->attnum, cmoid);
+	if (need_rewrite)
 		tab->rewrite |= AT_REWRITE_ALTER_COMPRESSION;
 
+	atttableform->attcompression = cmoid;
+
 	atttableform->attcompression = cmoid;
 	CatalogTupleUpdate(attrel, &tuple->t_self, tuple);
 
@@ -17865,42 +17916,3 @@ ATExecAlterCollationRefreshVersion(Relation rel, List *coll)
 	index_update_collation_versions(rel->rd_id, get_collation_oid(coll, false));
 	CacheInvalidateRelcache(rel);
 }
-
-/*
- * resolve column compression specification to an OID.
- */
-static Oid
-GetAttributeCompression(Form_pg_attribute att, char *compression)
-{
-	char		typstorage = get_typstorage(att->atttypid);
-	Oid			amoid;
-
-	/*
-	 * No compression for the plain/external storage, refer comments atop
-	 * attcompression parameter in pg_attribute.h
-	 */
-	if (!IsStorageCompressible(typstorage))
-	{
-		if (compression == NULL)
-			return InvalidOid;
-
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("column data type %s does not support compression",
-						format_type_be(att->atttypid))));
-	}
-
-	/* fallback to default compression if it's not specified */
-	if (compression == NULL)
-		return GetDefaultToastCompression();
-
-	amoid = get_compression_am_oid(compression, false);
-
-#ifndef HAVE_LIBLZ4
-	if (amoid == LZ4_COMPRESSION_AM_OID)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("not built with lz4 support")));
-#endif
-	return amoid;
-}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index ea82a05591..90d092671e 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -44,6 +44,7 @@
 #include "access/tableam.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
+#include "commands/defrem.h"
 #include "commands/trigger.h"
 #include "executor/execPartition.h"
 #include "executor/executor.h"
@@ -2068,8 +2069,8 @@ CompareCompressionMethodAndDecompress(TupleTableSlot *slot,
 
 	/*
 	 * Loop over all the attributes in the tuple and check if any attribute is
-	 * compressed and its compression method is not same as the target
-	 * atrribute's compression method then decompress it.
+	 * compressed and its compression method is not is not supported by the
+	 * target attribute then we need to decompress
 	 */
 	for (i = 0; i < natts; i++)
 	{
@@ -2094,12 +2095,13 @@ CompareCompressionMethodAndDecompress(TupleTableSlot *slot,
 				DatumGetPointer(slot->tts_values[attnum - 1]);
 
 			/*
-			 * Get the compression method Oid stored in the toast header and
-			 * compare it with the compression method of the target.
+			 * Get the compression method stored in the toast header and if the
+			 * compression method is not supported by the target attribute then
+			 * we need to decompress it.
 			 */
 			cmoid = toast_get_compression_oid(new_value);
 			if (OidIsValid(cmoid) &&
-				targetTupDesc->attrs[i].attcompression != cmoid)
+				!IsCompressionSupported(&targetTupDesc->attrs[i], cmoid))
 			{
 				new_value = detoast_attr(new_value);
 				slot->tts_values[attnum - 1] = PointerGetDatum(new_value);
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 1338e04409..6a11f8eb60 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2966,7 +2966,7 @@ _copyColumnDef(const ColumnDef *from)
 
 	COPY_STRING_FIELD(colname);
 	COPY_NODE_FIELD(typeName);
-	COPY_STRING_FIELD(compression);
+	COPY_NODE_FIELD(compression);
 	COPY_SCALAR_FIELD(inhcount);
 	COPY_SCALAR_FIELD(is_local);
 	COPY_SCALAR_FIELD(is_not_null);
@@ -2986,6 +2986,18 @@ _copyColumnDef(const ColumnDef *from)
 	return newnode;
 }
 
+static ColumnCompression *
+_copyColumnCompression(const ColumnCompression *from)
+{
+	ColumnCompression *newnode = makeNode(ColumnCompression);
+
+	COPY_STRING_FIELD(cmname);
+	COPY_SCALAR_FIELD(preserve_all);
+	COPY_NODE_FIELD(preserve);
+
+	return newnode;
+}
+
 static Constraint *
 _copyConstraint(const Constraint *from)
 {
@@ -5675,6 +5687,9 @@ copyObjectImpl(const void *from)
 		case T_ColumnDef:
 			retval = _copyColumnDef(from);
 			break;
+		case T_ColumnCompression:
+			retval = _copyColumnCompression(from);
+			break;
 		case T_Constraint:
 			retval = _copyConstraint(from);
 			break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index f3592003da..26a9b85974 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2599,7 +2599,7 @@ _equalColumnDef(const ColumnDef *a, const ColumnDef *b)
 {
 	COMPARE_STRING_FIELD(colname);
 	COMPARE_NODE_FIELD(typeName);
-	COMPARE_STRING_FIELD(compression);
+	COMPARE_NODE_FIELD(compression);
 	COMPARE_SCALAR_FIELD(inhcount);
 	COMPARE_SCALAR_FIELD(is_local);
 	COMPARE_SCALAR_FIELD(is_not_null);
@@ -2619,6 +2619,16 @@ _equalColumnDef(const ColumnDef *a, const ColumnDef *b)
 	return true;
 }
 
+static bool
+_equalColumnCompression(const ColumnCompression *a, const ColumnCompression *b)
+{
+	COMPARE_STRING_FIELD(cmname);
+	COMPARE_SCALAR_FIELD(preserve_all);
+	COMPARE_NODE_FIELD(preserve);
+
+	return true;
+}
+
 static bool
 _equalConstraint(const Constraint *a, const Constraint *b)
 {
@@ -3724,6 +3734,9 @@ equal(const void *a, const void *b)
 		case T_ColumnDef:
 			retval = _equalColumnDef(a, b);
 			break;
+		case T_ColumnCompression:
+			retval = _equalColumnCompression(a, b);
+			break;
 		case T_Constraint:
 			retval = _equalConstraint(a, b);
 			break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 0605ef3f84..b584a58ba3 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2863,7 +2863,7 @@ _outColumnDef(StringInfo str, const ColumnDef *node)
 
 	WRITE_STRING_FIELD(colname);
 	WRITE_NODE_FIELD(typeName);
-	WRITE_STRING_FIELD(compression);
+	WRITE_NODE_FIELD(compression);
 	WRITE_INT_FIELD(inhcount);
 	WRITE_BOOL_FIELD(is_local);
 	WRITE_BOOL_FIELD(is_not_null);
@@ -2881,6 +2881,16 @@ _outColumnDef(StringInfo str, const ColumnDef *node)
 	WRITE_LOCATION_FIELD(location);
 }
 
+static void
+_outColumnCompression(StringInfo str, const ColumnCompression *node)
+{
+	WRITE_NODE_TYPE("COLUMNCOMPRESSION");
+
+	WRITE_STRING_FIELD(cmname);
+	WRITE_BOOL_FIELD(preserve_all);
+	WRITE_NODE_FIELD(preserve);
+}
+
 static void
 _outTypeName(StringInfo str, const TypeName *node)
 {
@@ -4258,6 +4268,9 @@ outNode(StringInfo str, const void *obj)
 			case T_ColumnDef:
 				_outColumnDef(str, obj);
 				break;
+			case T_ColumnCompression:
+				_outColumnCompression(str, obj);
+				break;
 			case T_TypeName:
 				_outTypeName(str, obj);
 				break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 30acfe615d..9eb2b04d58 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -596,7 +596,9 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>		hash_partbound
 %type <defelt>		hash_partbound_elem
 
-%type <str>	optColumnCompression
+%type <node>	optColumnCompression alterColumnCompression
+%type <str>		compressionClause
+%type <list>	optCompressionPreserve
 
 /*
  * Non-keyword token types.  These are hard-wired into the "flex" lexer.
@@ -2309,12 +2311,12 @@ alter_table_cmd:
 					$$ = (Node *)n;
 				}
 			/* ALTER TABLE <name> ALTER [COLUMN] <colname> SET (COMPRESSION <cm>) */
-			| ALTER opt_column ColId SET optColumnCompression
+			| ALTER opt_column ColId SET alterColumnCompression
 				{
 					AlterTableCmd *n = makeNode(AlterTableCmd);
 					n->subtype = AT_SetCompression;
 					n->name = $3;
-					n->def = (Node *) makeString($5);
+					n->def = $5;
 					$$ = (Node *)n;
 				}
 			/* ALTER TABLE <name> DROP [COLUMN] IF EXISTS <colname> [RESTRICT|CASCADE] */
@@ -3437,7 +3439,7 @@ columnDef:	ColId Typename optColumnCompression create_generic_options ColQualLis
 					ColumnDef *n = makeNode(ColumnDef);
 					n->colname = $1;
 					n->typeName = $2;
-					n->compression = $3;
+					n->compression = (ColumnCompression *) $3;
 					n->inhcount = 0;
 					n->is_local = true;
 					n->is_not_null = false;
@@ -3492,13 +3494,43 @@ columnOptions:	ColId ColQualList
 				}
 		;
 
+optCompressionPreserve:
+			PRESERVE '(' name_list ')' { $$ = $3; }
+			| /*EMPTY*/ { $$ = NULL; }
+		;
+
+compressionClause:
+			COMPRESSION name { $$ = pstrdup($2); }
+		;
+
 optColumnCompression:
-					COMPRESSION name
-					{
-						$$ = $2;
-					}
-					| /*EMPTY*/	{ $$ = NULL; }
-				;
+			compressionClause
+				{
+					ColumnCompression *n = makeNode(ColumnCompression);
+					n->cmname = $1;
+					n->preserve = NIL;
+					$$ = (Node *) n;
+				}
+			| /*EMPTY*/	{ $$ = NULL; }
+		;
+
+alterColumnCompression:
+			compressionClause optCompressionPreserve
+				{
+					ColumnCompression *n = makeNode(ColumnCompression);
+					n->cmname = $1;
+					n->preserve = (List *) $2;
+					$$ = (Node *) n;
+				}
+			|	compressionClause PRESERVE ALL
+				{
+					ColumnCompression *n = makeNode(ColumnCompression);
+					n->cmname = $1;
+					n->preserve_all = true;
+					n->preserve = NIL;
+					$$ = (Node *) n;
+				}
+		;
 
 ColQualList:
 			ColQualList ColConstraint				{ $$ = lappend($1, $2); }
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index cf4413da64..45f4724a13 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1086,7 +1086,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		/* Likewise, copy compression if requested */
 		if ((table_like_clause->options & CREATE_TABLE_LIKE_COMPRESSION) != 0
 			&& OidIsValid(attribute->attcompression))
-			def->compression = get_am_name(attribute->attcompression);
+			def->compression = MakeColumnCompression(attribute->attcompression);
 		else
 			def->compression = NULL;
 
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 46044cb92a..7bf345a4ac 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -9037,6 +9037,80 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			}
 			PQclear(res);
 		}
+
+		/*
+		 * Get compression info
+		 */
+		if (fout->remoteVersion >= 140000 && dopt->binary_upgrade)
+		{
+			int			i_amname;
+			int			i_amoid;
+			int			i_curattnum;
+			int			start;
+
+			pg_log_info("finding compression info for table \"%s.%s\"",
+						tbinfo->dobj.namespace->dobj.name,
+						tbinfo->dobj.name);
+
+			tbinfo->attcompression = pg_malloc0(tbinfo->numatts * sizeof(AttrCompressionInfo *));
+
+			resetPQExpBuffer(q);
+			appendPQExpBuffer(q,
+				" SELECT attrelid::pg_catalog.regclass AS relname, attname,"
+				" amname, am.oid as amoid, d.objsubid AS curattnum"
+				" FROM pg_depend d"
+				" JOIN pg_attribute a ON"
+				"	(classid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid AND a.attrelid = d.objid"
+				"		AND a.attnum = d.objsubid AND d.deptype = 'n'"
+				"		AND d.refclassid = 'pg_am'::pg_catalog.regclass::pg_catalog.oid)"
+				" JOIN pg_am am ON"
+				"	(d.deptype = 'n' AND d.refobjid = am.oid)"
+				" WHERE (deptype = 'n' AND d.objid = %d AND a.attcompression != am.oid);",
+				tbinfo->dobj.catId.oid);
+
+			res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK);
+			ntups = PQntuples(res);
+
+			if (ntups > 0)
+			{
+				int		j;
+				int		k;
+
+				i_amname = PQfnumber(res, "amname");
+				i_amoid = PQfnumber(res, "amoid");
+				i_curattnum = PQfnumber(res, "curattnum");
+
+				start = 0;
+
+				for (j = 0; j < ntups; j++)
+				{
+					int		attnum = atoi(PQgetvalue(res, j, i_curattnum));
+
+					if ((j == ntups - 1) || atoi(PQgetvalue(res, j + 1, i_curattnum)) != attnum)
+					{
+						AttrCompressionInfo *cminfo = pg_malloc(sizeof(AttrCompressionInfo));
+
+						cminfo->nitems = j - start + 1;
+						cminfo->items = pg_malloc(sizeof(AttrCompressionItem *) * cminfo->nitems);
+
+						for (k = start; k < start + cminfo->nitems; k++)
+						{
+							AttrCompressionItem	*cmitem = pg_malloc0(sizeof(AttrCompressionItem));
+
+							cmitem->amname = pg_strdup(PQgetvalue(res, k, i_amname));
+							cmitem->amoid = atooid(PQgetvalue(res, k, i_amoid));
+
+							cminfo->items[k - start] = cmitem;
+						}
+
+						tbinfo->attcompression[attnum - 1] = cminfo;
+						start = j + 1;	/* start from next */
+					}
+				}
+			}
+
+			PQclear(res);
+		}
 	}
 
 	destroyPQExpBuffer(q);
@@ -16343,6 +16417,33 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 								  qualrelname,
 								  fmtId(tbinfo->attnames[j]),
 								  tbinfo->attfdwoptions[j]);
+
+			/*
+			 * Dump per-column compression options
+			 */
+			if (tbinfo->attcompression && tbinfo->attcompression[j])
+			{
+				AttrCompressionInfo *cminfo = tbinfo->attcompression[j];
+
+				appendPQExpBuffer(q, "ALTER TABLE %s ALTER COLUMN %s\nSET COMPRESSION %s",
+									qualrelname, fmtId(tbinfo->attnames[j]), tbinfo->attcmnames[j]);
+
+				if (cminfo->nitems > 0)
+				{
+					appendPQExpBuffer(q, "\nPRESERVE (");
+					for (int i = 0; i < cminfo->nitems; i++)
+					{
+						AttrCompressionItem *item = cminfo->items[i];
+
+						if (i == 0)
+							appendPQExpBuffer(q, "%s", item->amname);
+						else
+							appendPQExpBuffer(q, ", %s", item->amname);
+					}
+					appendPQExpBuffer(q, ")");
+				}
+				appendPQExpBuffer(q, ";\n");
+			}
 		}
 
 		if (ftoptions)
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 1789e18f46..a829528cd0 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -327,7 +327,8 @@ typedef struct _tableInfo
 	bool		needs_override; /* has GENERATED ALWAYS AS IDENTITY */
 	char	   *amname;			/* relation access method */
 	char	  **attcmnames;		/* per-attribute current compression method */
-
+	struct _attrCompressionInfo **attcompression; /* per-attribute all
+													 compression data */
 	/*
 	 * Stuff computed only for dumpable tables.
 	 */
@@ -356,6 +357,18 @@ typedef struct _attrDefInfo
 	bool		separate;		/* true if must dump as separate item */
 } AttrDefInfo;
 
+typedef struct _attrCompressionItem
+{
+	Oid			amoid;			/* attribute compression oid */
+	char	   *amname;			/* compression access method name */
+} AttrCompressionItem;
+
+typedef struct _attrCompressionInfo
+{
+	int			nitems;
+	AttrCompressionItem	**items;
+} AttrCompressionInfo;
+
 typedef struct _tableDataInfo
 {
 	DumpableObject dobj;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index ffa8d05edf..869fd3676d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2103,6 +2103,13 @@ psql_completion(const char *text, int start, int end)
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "(") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "("))
 		COMPLETE_WITH("n_distinct", "n_distinct_inherited");
+	/* ALTER TABLE ALTER [COLUMN] <foo> SET COMPRESSION */
+	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "COMPRESSION", MatchAny) ||
+			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "COMPRESSION", MatchAny))
+		COMPLETE_WITH("PRESERVE");
+	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "COMPRESSION", MatchAny, "PRESERVE") ||
+			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "COMPRESSION", MatchAny, "PRESERVE"))
+		COMPLETE_WITH("( ", "ALL");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET STORAGE */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "STORAGE") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "STORAGE"))
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index e5aea8a240..bd53f9bb0f 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -143,6 +143,13 @@ extern Oid	get_compression_am_oid(const char *amname, bool missing_ok);
 extern Oid	get_am_oid(const char *amname, bool missing_ok);
 extern char *get_am_name(Oid amOid);
 
+/* commands/compressioncmds.c */
+extern Oid GetAttributeCompression(Form_pg_attribute att,
+								   ColumnCompression *compression,
+								   bool *need_rewrite);
+extern ColumnCompression *MakeColumnCompression(Oid atttcompression);
+extern bool IsCompressionSupported(Form_pg_attribute att, Oid cmoid);
+
 /* support routines in commands/define.c */
 
 extern char *defGetString(DefElem *def);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index b3d30acc35..e6c98e65d4 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -97,5 +97,7 @@ extern void RangeVarCallbackOwnsRelation(const RangeVar *relation,
 										 Oid relId, Oid oldRelId, void *arg);
 extern bool PartConstraintImpliedByRelConstraint(Relation scanrel,
 												 List *partConstraint);
+extern void add_column_compression_dependency(Oid relid, int32 attnum,
+											  Oid cmoid);
 
 #endif							/* TABLECMDS_H */
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 20d6f96f62..24deaad253 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -481,6 +481,7 @@ typedef enum NodeTag
 	T_PartitionBoundSpec,
 	T_PartitionRangeDatum,
 	T_PartitionCmd,
+	T_ColumnCompression,
 	T_VacuumRelation,
 
 	/*
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f9a87dee02..ce0913e18a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -623,6 +623,20 @@ typedef struct RangeTableSample
 	int			location;		/* method name location, or -1 if unknown */
 } RangeTableSample;
 
+/*
+ * ColumnCompression - compression parameters for some attribute
+ *
+ * This represents compression information defined using clause:
+ * .. COMPRESSION <compression method> PRESERVE <compression methods>
+ */
+typedef struct ColumnCompression
+{
+	NodeTag		type;
+	char	   *cmname;
+	bool		preserve_all;
+	List	   *preserve;
+} ColumnCompression;
+
 /*
  * ColumnDef - column definition (used in various creates)
  *
@@ -646,7 +660,7 @@ typedef struct ColumnDef
 	NodeTag		type;
 	char	   *colname;		/* name of column */
 	TypeName   *typeName;		/* type of column */
-	char	   *compression;	/* compression method for column */
+	ColumnCompression *compression;	/* column compression */
 	int			inhcount;		/* number of times column is inherited */
 	bool		is_local;		/* column has local (non-inherited) def'n */
 	bool		is_not_null;	/* NOT NULL constraint specified? */
diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out
index 21c1b451d2..3ed33b6534 100644
--- a/src/test/regress/expected/compression.out
+++ b/src/test/regress/expected/compression.out
@@ -226,12 +226,47 @@ SELECT pg_column_compression(f1) FROM cmpart;
  lz4
 (2 rows)
 
+-- preserve old compression method
+ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION pglz PRESERVE (lz4);
+INSERT INTO cmdata VALUES (repeat('1234567890',1004));
+\d+ cmdata
+                                        Table "public.cmdata"
+ Column | Type | Collation | Nullable | Default | Storage  | Compression | Stats target | Description 
+--------+------+-----------+----------+---------+----------+-------------+--------------+-------------
+ f1     | text |           |          |         | extended | pglz        |              | 
+Indexes:
+    "idx" btree (f1)
+
+SELECT pg_column_compression(f1) FROM cmdata;
+ pg_column_compression 
+-----------------------
+ lz4
+ pglz
+(2 rows)
+
+\d+ cmdata
+                                        Table "public.cmdata"
+ Column | Type | Collation | Nullable | Default | Storage  | Compression | Stats target | Description 
+--------+------+-----------+----------+---------+----------+-------------+--------------+-------------
+ f1     | text |           |          |         | extended | pglz        |              | 
+Indexes:
+    "idx" btree (f1)
+
+ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4 PRESERVE ALL;
+SELECT pg_column_compression(f1) FROM cmdata;
+ pg_column_compression 
+-----------------------
+ lz4
+ pglz
+(2 rows)
+
 -- check data is ok
 SELECT length(f1) FROM cmdata;
  length 
 --------
   10000
-(1 row)
+  10040
+(2 rows)
 
 SELECT length(f1) FROM cmdata1;
  length 
diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out
index 64c5855bf7..36a5f8ba5e 100644
--- a/src/test/regress/expected/compression_1.out
+++ b/src/test/regress/expected/compression_1.out
@@ -205,12 +205,49 @@ SELECT pg_column_compression(f1) FROM cmpart;
 ERROR:  relation "cmpart" does not exist
 LINE 1: SELECT pg_column_compression(f1) FROM cmpart;
                                               ^
+-- preserve old compression method
+ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION pglz PRESERVE (lz4);
+ERROR:  "lz4" compression access method cannot be preserved
+INSERT INTO cmdata VALUES (repeat('1234567890',1004));
+\d+ cmdata
+                                        Table "public.cmdata"
+ Column | Type | Collation | Nullable | Default | Storage  | Compression | Stats target | Description 
+--------+------+-----------+----------+---------+----------+-------------+--------------+-------------
+ f1     | text |           |          |         | extended | pglz        |              | 
+Indexes:
+    "idx" btree (f1)
+
+SELECT pg_column_compression(f1) FROM cmdata;
+ pg_column_compression 
+-----------------------
+ pglz
+ pglz
+(2 rows)
+
+\d+ cmdata
+                                        Table "public.cmdata"
+ Column | Type | Collation | Nullable | Default | Storage  | Compression | Stats target | Description 
+--------+------+-----------+----------+---------+----------+-------------+--------------+-------------
+ f1     | text |           |          |         | extended | pglz        |              | 
+Indexes:
+    "idx" btree (f1)
+
+ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4 PRESERVE ALL;
+ERROR:  not built with lz4 support
+SELECT pg_column_compression(f1) FROM cmdata;
+ pg_column_compression 
+-----------------------
+ pglz
+ pglz
+(2 rows)
+
 -- check data is ok
 SELECT length(f1) FROM cmdata;
  length 
 --------
   10000
-(1 row)
+  10040
+(2 rows)
 
 SELECT length(f1) FROM cmdata1;
 ERROR:  relation "cmdata1" does not exist
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 830fdddf24..8f984510ac 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2066,19 +2066,21 @@ WHERE classid = 'pg_class'::regclass AND
 	    'concur_reindex_ind4'::regclass,
 	    'concur_reindex_matview'::regclass)
   ORDER BY 1, 2;
-                   obj                    |                           objref                           | deptype 
-------------------------------------------+------------------------------------------------------------+---------
- index concur_reindex_ind1                | constraint concur_reindex_ind1 on table concur_reindex_tab | i
- index concur_reindex_ind2                | collation "default"                                        | n
- index concur_reindex_ind2                | column c2 of table concur_reindex_tab                      | a
- index concur_reindex_ind3                | column c1 of table concur_reindex_tab                      | a
- index concur_reindex_ind3                | table concur_reindex_tab                                   | a
- index concur_reindex_ind4                | collation "default"                                        | n
- index concur_reindex_ind4                | column c1 of table concur_reindex_tab                      | a
- index concur_reindex_ind4                | column c2 of table concur_reindex_tab                      | a
- materialized view concur_reindex_matview | schema public                                              | n
- table concur_reindex_tab                 | schema public                                              | n
-(10 rows)
+                          obj                          |                           objref                           | deptype 
+-------------------------------------------------------+------------------------------------------------------------+---------
+ column c2 of materialized view concur_reindex_matview | access method pglz                                         | n
+ column c2 of table concur_reindex_tab                 | access method pglz                                         | n
+ index concur_reindex_ind1                             | constraint concur_reindex_ind1 on table concur_reindex_tab | i
+ index concur_reindex_ind2                             | collation "default"                                        | n
+ index concur_reindex_ind2                             | column c2 of table concur_reindex_tab                      | a
+ index concur_reindex_ind3                             | column c1 of table concur_reindex_tab                      | a
+ index concur_reindex_ind3                             | table concur_reindex_tab                                   | a
+ index concur_reindex_ind4                             | collation "default"                                        | n
+ index concur_reindex_ind4                             | column c1 of table concur_reindex_tab                      | a
+ index concur_reindex_ind4                             | column c2 of table concur_reindex_tab                      | a
+ materialized view concur_reindex_matview              | schema public                                              | n
+ table concur_reindex_tab                              | schema public                                              | n
+(12 rows)
 
 REINDEX INDEX CONCURRENTLY concur_reindex_ind1;
 REINDEX TABLE CONCURRENTLY concur_reindex_tab;
@@ -2095,19 +2097,21 @@ WHERE classid = 'pg_class'::regclass AND
 	    'concur_reindex_ind4'::regclass,
 	    'concur_reindex_matview'::regclass)
   ORDER BY 1, 2;
-                   obj                    |                           objref                           | deptype 
-------------------------------------------+------------------------------------------------------------+---------
- index concur_reindex_ind1                | constraint concur_reindex_ind1 on table concur_reindex_tab | i
- index concur_reindex_ind2                | collation "default"                                        | n
- index concur_reindex_ind2                | column c2 of table concur_reindex_tab                      | a
- index concur_reindex_ind3                | column c1 of table concur_reindex_tab                      | a
- index concur_reindex_ind3                | table concur_reindex_tab                                   | a
- index concur_reindex_ind4                | collation "default"                                        | n
- index concur_reindex_ind4                | column c1 of table concur_reindex_tab                      | a
- index concur_reindex_ind4                | column c2 of table concur_reindex_tab                      | a
- materialized view concur_reindex_matview | schema public                                              | n
- table concur_reindex_tab                 | schema public                                              | n
-(10 rows)
+                          obj                          |                           objref                           | deptype 
+-------------------------------------------------------+------------------------------------------------------------+---------
+ column c2 of materialized view concur_reindex_matview | access method pglz                                         | n
+ column c2 of table concur_reindex_tab                 | access method pglz                                         | n
+ index concur_reindex_ind1                             | constraint concur_reindex_ind1 on table concur_reindex_tab | i
+ index concur_reindex_ind2                             | collation "default"                                        | n
+ index concur_reindex_ind2                             | column c2 of table concur_reindex_tab                      | a
+ index concur_reindex_ind3                             | column c1 of table concur_reindex_tab                      | a
+ index concur_reindex_ind3                             | table concur_reindex_tab                                   | a
+ index concur_reindex_ind4                             | collation "default"                                        | n
+ index concur_reindex_ind4                             | column c1 of table concur_reindex_tab                      | a
+ index concur_reindex_ind4                             | column c2 of table concur_reindex_tab                      | a
+ materialized view concur_reindex_matview              | schema public                                              | n
+ table concur_reindex_tab                              | schema public                                              | n
+(12 rows)
 
 -- Check that comments are preserved
 CREATE TABLE testcomment (i int);
diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql
index b9daa33b74..5774b55d82 100644
--- a/src/test/regress/sql/compression.sql
+++ b/src/test/regress/sql/compression.sql
@@ -96,6 +96,15 @@ ALTER TABLE cmpart1 ALTER COLUMN f1 SET COMPRESSION pglz;
 ALTER TABLE cmpart2 ALTER COLUMN f1 SET COMPRESSION lz4;
 SELECT pg_column_compression(f1) FROM cmpart;
 
+-- preserve old compression method
+ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION pglz PRESERVE (lz4);
+INSERT INTO cmdata VALUES (repeat('1234567890',1004));
+\d+ cmdata
+SELECT pg_column_compression(f1) FROM cmdata;
+\d+ cmdata
+ALTER TABLE cmdata ALTER COLUMN f1 SET COMPRESSION lz4 PRESERVE ALL;
+SELECT pg_column_compression(f1) FROM cmdata;
+
 -- check data is ok
 SELECT length(f1) FROM cmdata;
 SELECT length(f1) FROM cmdata1;
-- 
2.17.0


--YZ5djTAD1cGYuMQK--





^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: [Proposal] Global temporary tables
@ 2022-02-27 00:21  Justin Pryzby <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Justin Pryzby @ 2022-02-27 00:21 UTC (permalink / raw)
  To: [email protected]; +Cc: Andrew Bille <[email protected]>; Pavel Stehule <[email protected]>; Tony Zhu <[email protected]>; [email protected]

I read through this.
Find attached some language fixes.  You should be able to apply each "fix"
patch on top of your own local branch with git am, and then squish them
together.  Let me know if you have trouble with that.

I think get_seqence_start_value() should be static.  (Or otherwise, it should
be in lsyscache.c).

The include added to execPartition.c seems to be unused.

+#define RELATION_IS_TEMP_ON_CURRENT_SESSION(relation) \
+#define RELATION_IS_TEMP(relation) \
+#define RelpersistenceTsTemp(relpersistence) \
+#define RELATION_GTT_ON_COMMIT_DELETE(relation)    \

=> These macros can evaluate their arguments multiple times.
You should add a comment to warn about that.  And maybe avoid passing them a
function argument, like: RelpersistenceTsTemp(get_rel_persistence(rte->relid))

+list_all_backend_gtt_frozenxids should return TransactionId not int.
The function name should say "oldest" and not "all" ?

I think the GUC should have a longer name.  max_active_gtt is too short for a
global var.

+#define    MIN_NUM_ACTIVE_GTT          0
+#define    DEFAULT_NUM_ACTIVE_GTT          1000
+#define    MAX_NUM_ACTIVE_GTT          1000000

+int        max_active_gtt = MIN_NUM_ACTIVE_GTT

It's being initialized to MIN, but then the GUC machinery sets it to DEFAULT.
By convention, it should be initialized to default.

fout->remoteVersion >= 140000

=> should say 15

describe.c has gettext_noop("session"), which is a half-truth.  The data is
per-session but the table definition is persistent..

You redirect stats from pg_class and pg_statistics to a local hash table.
This is pretty hairy :(
I guess you'd also need to handle pg_statistic_ext and ext_data.
pg_stats doesn't work, since the data isn't in pg_statistic - it'd need to look
at pg_get_gtt_statistics.

I wonder if there's a better way to do it, like updating pg_statistic but
forcing the changes to be rolled back when the session ends...  But I think
that would make longrunning sessions behave badly, the same as "longrunning
transactions".

Have you looked at Gilles Darold's GTT extension ?


Attachments:

  [text/x-diff] 0002-f-0002-gtt-v64-doc.txt (4.9K, ../../[email protected]/2-0002-f-0002-gtt-v64-doc.txt)
  download | inline diff:
From b89f3cc5c78e7b4c3e10ab39ef527b524d0d112d Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Sat, 1 Jan 2022 17:02:30 -0600
Subject: [PATCH 02/11] f!0002-gtt-v64-doc.patch

---
 doc/src/sgml/ref/create_table.sgml | 42 ++++++++++++++++--------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 408373323c..9cd5fc17f2 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -174,18 +174,18 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      <para>
       If specified, the table is created as a temporary table.
       Optionally, <literal>GLOBAL</literal> or <literal>LOCAL</literal>
-      can be written before <literal>TEMPORARY</literal> or <literal>TEMP</literal>.
+      may be written before <literal>TEMPORARY</literal> or <literal>TEMP</literal>.
       They represent two types of temporary tables supported by <productname>PostgreSQL</productname>:
-      global temporary table and local temporary table. Without specified
-      GLOBAL or LOCAL, a local temporary table is created by default.
+      global temporary table and local temporary table. If neither
+      GLOBAL or LOCAL is specified, a local temporary table is created by default.
      </para>
 
     <para>
-     Both types of temporary tables’ data are truncated at the
+     Data of both types of temporary tables is truncated at the
      end of a session or optionally at the end of the current transaction.
-     (see <literal>ON COMMIT</literal> below). For global temporary table,
-     its schema is reserved and reused by future sessions or transactions.
-     For local temporary table, both its data and its schema are dropped.
+     (see <literal>ON COMMIT</literal> below). For a global temporary table,
+     its table definition is preserved and reused by future sessions or transactions.
+     For a local temporary table, both its data and its table definition are dropped.
     </para>
 
     <variablelist>
@@ -194,10 +194,10 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       <listitem>
        <para>
         Global temporary table are defined just once and automatically exist
-        (starting with empty contents) in every session that needs them.
-        The schema definition of temporary tables is persistent and shared among sessions.
-        However, the data in temporary tables are kept private to sessions themselves,
-        even though they use same name and same schema.
+        (initially with empty contents) in every session.
+        The schema definition of a temporary table is persistent and common to all sessions.
+        However, the data in temporary tables is private to each sessions,
+        even though they share the same name and same table definition.
        </para>
       </listitem>
      </varlistentry>
@@ -206,15 +206,15 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       <term><literal>Local Temporary Table</literal></term>
      <listitem>
      <para>
-      Local temporary table are automatically dropped at the end of a
-      session (include schema and data). Future sessions need to create
-      their own temporary tables when they are used.
+      Local temporary tables are automatically dropped at the end of a
+      session (include table definition and data). Future sessions need to create
+      their own temporary tables before using them.
      </para>
      <para>
-      The default search_path includes the temporary schema first and so
-      identically named existing permanent tables are not chosen for new plans
-      while the temporary table exists, unless they are referenced
-      with schema-qualified names. Any indexes created on a temporary
+      The default <varname>search_path</varname> includes the temporary schema first, and so
+      identically-named, permanent tables of the same name as a temporary table are not chosen for new plans
+      so long as the temporary table exists, unless the permanent table is referenced
+      with a schema-qualified name. Any indexes created on a temporary
       table are automatically temporary as well.
      </para>
      </listitem>
@@ -229,9 +229,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      table is going to be used in complex queries, it is wise to run
      <command>ANALYZE</command> on the temporary table after it is populated.
     </para>
+
     <para>
-     The Temporary table resembles the SQL standard, but has some differences.
-     see <xref linkend="sql-createtable-compatibility"/> below.
+     The temporary table resembles the behavior defined in the SQL standard,
+     but has some differences.
+     See <xref linkend="sql-createtable-compatibility"/> below.
     </para>
 
     </listitem>
-- 
2.17.1



  [text/x-diff] 0004-f-0003-gtt-v64-implementation.txt (20.2K, ../../[email protected]/3-0004-f-0003-gtt-v64-implementation.txt)
  download | inline diff:
From 63d4a62043bd1d2af381c632553f85c89de4f36f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Sat, 1 Jan 2022 16:47:14 -0600
Subject: [PATCH 04/11] f!0003-gtt-v64-implementation.patch

---
 src/backend/catalog/heap.c          |  8 ++++----
 src/backend/catalog/index.c         | 24 +++++++++++-----------
 src/backend/catalog/storage_gtt.c   | 32 ++++++++++++++---------------
 src/backend/commands/analyze.c      |  4 ++--
 src/backend/commands/cluster.c      |  6 +++---
 src/backend/commands/sequence.c     |  6 +++---
 src/backend/commands/tablecmds.c    | 10 ++++-----
 src/backend/commands/vacuum.c       |  2 +-
 src/backend/commands/view.c         |  2 +-
 src/backend/parser/analyze.c        |  2 +-
 src/backend/parser/parse_utilcmd.c  |  2 +-
 src/backend/postmaster/autovacuum.c |  4 ++--
 src/backend/storage/ipc/procarray.c |  5 ++++-
 src/backend/utils/adt/dbsize.c      |  2 +-
 src/bin/pg_dump/pg_dump.c           |  2 +-
 src/include/catalog/pg_class.h      |  2 +-
 16 files changed, 58 insertions(+), 55 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 8e5b56f602..d24afa45c1 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2023,15 +2023,15 @@ heap_drop_with_catalog(Oid relid)
 		update_default_partition_oid(parentOid, InvalidOid);
 
 	/*
-	 * Only when other sessions are not using this Global temporary table,
-	 * is it allowed to DROP it.
+	 * It is not allowed to DROP a Global temporary table when other
+	 * sessions are using it
 	 */
 	if (RELATION_IS_GLOBAL_TEMP(rel))
 	{
 		if (is_other_backend_use_gtt(RelationGetRelid(rel)))
 			ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot drop global temporary table %s when other backend attached it.",
+				 errmsg("cannot drop global temporary table %s being accessed by another backend",
 						RelationGetRelationName(rel))));
 	}
 
@@ -3466,7 +3466,7 @@ heap_truncate_one_rel(Relation rel)
 
 	/*
 	 * After the data is cleaned up on the GTT, the transaction information
-	 * for the data(stored in local hash table) is also need reset.
+	 * for the data(stored in local hash table) also needs to be reset.
 	 */
 	if (RELATION_IS_GLOBAL_TEMP(rel))
 		up_gtt_relstats(RelationGetRelid(rel), 0, 0, 0, RecentXmin, GetOldestMultiXactId());
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index be08ba376a..e2483426d2 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -741,11 +741,11 @@ index_create(Relation heapRelation,
 	/* For global temporary table only */
 	if (RELATION_IS_GLOBAL_TEMP(heapRelation))
 	{
-		/* No support create index on global temporary table with concurrent mode */
+		/* No support for creating index on global temporary table with concurrent mode */
 		Assert(!concurrent);
 
 		/*
-		 * For the case that some backend is applied relcache message to create
+		 * For the case that some backend is applied relcache message to create XXX
 		 * an index on a global temporary table, if this table in the current
 		 * backend are not initialized, the creation of index storage on the
 		 * table are also skipped.
@@ -2201,18 +2201,18 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
 	CheckTableNotInUse(userIndexRelation, "DROP INDEX");
 
 	/*
-	 * Allow to drop index on global temporary table when only current
-	 * backend use it.
+	 * Disallow dropping index on global temporary table if another
+	 * backend is using it.
 	 */
 	if (RELATION_IS_GLOBAL_TEMP(userHeapRelation) &&
 		is_other_backend_use_gtt(RelationGetRelid(userHeapRelation)))
 	{
-		ereport(ERROR,
+		ereport(ERROR, // XXX
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 			 errmsg("cannot drop index %s on global temporary table %s",
 					RelationGetRelationName(userIndexRelation), RelationGetRelationName(userHeapRelation)),
-					errdetail("Because the index is created on the global temporary table and other backend attached it."),
-					errhint("Please try detach all sessions using this temporary table and try again.")));
+					errdetail("The index is created on the global temporary table and other backend attached it."),
+					errhint("Detach other sessions using this temporary table and try again.")));
 	}
 
 	/*
@@ -2926,12 +2926,12 @@ index_update_stats(Relation rel,
 		/* For global temporary table */
 		if (is_gtt)
 		{
-			/* Update GTT'statistics into local relcache */
+			/* Update GTT's statistics into local relcache */
 			rel->rd_rel->relpages = (int32) relpages;
 			rel->rd_rel->reltuples = (float4) reltuples;
 			rel->rd_rel->relallvisible = (int32) relallvisible;
 
-			/* Update GTT'statistics into local hashtable */
+			/* Update GTT's statistics into local hashtable */
 			up_gtt_relstats(RelationGetRelid(rel), relpages, reltuples, relallvisible,
 							InvalidTransactionId, InvalidMultiXactId);
 		}
@@ -3066,7 +3066,7 @@ index_build(Relation heapRelation,
 		pgstat_progress_update_multi_param(6, progress_index, progress_vals);
 	}
 
-	/* For build index on global temporary table */
+	/* If building index on global temporary table */
 	if (RELATION_IS_GLOBAL_TEMP(indexRelation))
 	{
 		/*
@@ -3075,7 +3075,7 @@ index_build(Relation heapRelation,
 		 */
 		if (!gtt_storage_attached(RelationGetRelid(indexRelation)))
 		{
-			/* Before create init storage, fix the local Relcache first */
+			/* Before creating init storage, fix the local Relcache first */
 			force_enable_gtt_index(indexRelation);
 
 			Assert(gtt_storage_attached(RelationGetRelid(heapRelation)));
@@ -3651,7 +3651,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		if (OidIsValid(params->tablespaceOid))
 			ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("The tablespace of global temporary table can not be changed")));
+				 errmsg("The tablespace of global temporary table cannot be changed")));
 
 		if (!gtt_storage_attached(indexId))
 		{
diff --git a/src/backend/catalog/storage_gtt.c b/src/backend/catalog/storage_gtt.c
index d50e1fa9af..1660163e99 100644
--- a/src/backend/catalog/storage_gtt.c
+++ b/src/backend/catalog/storage_gtt.c
@@ -1,12 +1,12 @@
 /*-------------------------------------------------------------------------
  *
  * storage_gtt.c
- *	  The body implementation of Global Temparary table.
+ *	  The body implementation of Global Temporary table.
  *
  * IDENTIFICATION
  *	  src/backend/catalog/storage_gtt.c
  *
- *	  See src/backend/catalog/GTT_README for Global temparary table's
+ *	  See src/backend/catalog/GTT_README for Global temporary table's
  *	  requirements and design.
  *
  *-------------------------------------------------------------------------
@@ -197,7 +197,7 @@ active_gtt_shared_hash_size(void)
 }
 
 /*
- * Initialization shared hash table for GTT.
+ * Initialization of shared hash table for GTT.
  */
 void
 active_gtt_shared_hash_init(void)
@@ -277,7 +277,7 @@ gtt_storage_checkin(Oid relid)
 
 /*
  * Remove the GTT relid record from the shared hash table which means that current backend is
- * not use this GTT.
+ * not using this GTT.
  */
 static void
 gtt_storage_checkout(Oid relid, bool isCommit)
@@ -298,7 +298,7 @@ gtt_storage_checkout(Oid relid, bool isCommit)
 	{
 		LWLockRelease(&gtt_shared_ctl->lock);
 		if (isCommit)
-			elog(WARNING, "relid %u not exist in gtt shared hash when drop local storage", relid);
+			elog(WARNING, "relid %u doesn't exist in gtt shared hash when dropping local storage", relid);
 
 		return;
 	}
@@ -319,9 +319,9 @@ gtt_storage_checkout(Oid relid, bool isCommit)
 
 /*
  * Gets usage information for a GTT from shared hash table.
- * The information is in the form of bitmap.
- * Quickly copy the entire bitmap from shared memory and return it.
- * that to avoid holding locks for a long time.
+ * The information is in the form of a bitmap.
+ * Quickly copy the entire bitmap from shared memory and return it,
+ * to avoid holding locks for a long time.
  */
 static Bitmapset *
 copy_active_gtt_bitmap(Oid relid)
@@ -417,18 +417,18 @@ remember_gtt_storage_info(RelFileNode rnode, Relation rel)
 	if (max_active_gtt <= 0)
 		ereport(ERROR,
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-			 errmsg("Global temporary table feature is disable"),
+			 errmsg("Global temporary table feature is disabled"),
 			 errhint("You might need to increase max_active_global_temporary_table to enable this feature.")));
 
 	if (RecoveryInProgress())
-		elog(ERROR, "readonly mode not support access global temporary table");
+		elog(ERROR, "recovery mode does not support accessing global temporary table");
 
 	if (rel->rd_rel->relkind == RELKIND_INDEX &&
 		rel->rd_index &&
 		(!rel->rd_index->indisvalid ||
 		 !rel->rd_index->indisready ||
 		 !rel->rd_index->indislive))
-		 elog(ERROR, "invalid gtt index %s not allow to create storage", RelationGetRelationName(rel));
+		 elog(ERROR, "invalid gtt index %s not allowed to create storage", RelationGetRelationName(rel));
 
 	/* First time through: initialize the hash table */
 	if (gtt_storage_local_hash == NULL)
@@ -757,7 +757,7 @@ up_gtt_relstats(Oid relid,
 
 /*
  * Search GTT relstats(relpage/reltuple/relallvisible)
- * from local has.
+ * from local hash.
  */
 bool
 get_gtt_relstats(Oid relid, BlockNumber *relpages, double *reltuples,
@@ -852,7 +852,7 @@ up_gtt_att_statistic(Oid reloid, int attnum, bool inh, int natts,
 	}
 	MemoryContextSwitchTo(oldcontext);
 
-	if (!found)
+	if (!found) // XXX: should be an ereport with a hint ?
 		elog(WARNING, "analyze can not update relid %u column %d statistics after add or drop column, try truncate table first", reloid, attnum);
 
 	return;
@@ -960,7 +960,7 @@ remove_gtt_relfrozenxid_from_ordered_list(Oid relfrozenxid)
 
 /*
  * Update of backend Level oldest relfrozenxid to MyProc.
- * This makes each backend's oldest RelFrozenxID globally visible.
+ * This makes each backend's oldest RelFrozenXID globally visible.
  */
 static void
 set_gtt_session_relfrozenxid(void)
@@ -1282,7 +1282,7 @@ pg_list_gtt_relfrozenxids(PG_FUNCTION_ARGS)
 }
 
 /*
- * In order to build the GTT index, force enable GTT'index.
+ * In order to build the GTT index, force enable GTT index.
  */
 void
 force_enable_gtt_index(Relation index)
@@ -1331,7 +1331,7 @@ gtt_fix_index_backend_state(Relation index)
 
 /*
  * During the SQL initialization of the executor (InitPlan)
- * Initialize storage of GTT GTT'indexes and build empty index.
+ * Initialize storage of GTT's indexes and build empty index.
  */
 void
 init_gtt_storage(CmdType operation, Relation relation)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 72218b2c66..6dee51784f 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -188,8 +188,8 @@ analyze_rel(Oid relid, RangeVar *relation,
 	}
 
 	/*
-	 * Skip the global temporary table that did not initialize the storage
-	 * in this backend.
+	 * Skip global temporary tables for which storage was not initialized by
+	 * this backend.
 	 */
 	if (RELATION_IS_GLOBAL_TEMP(onerel) &&
 		!gtt_storage_attached(RelationGetRelid(onerel)))
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 535f537aae..ff6ae583f5 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -392,15 +392,15 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 	}
 
 	/*
-	 * Skip the global temporary table that did not initialize the storage
-	 * in this backend.
+	 * Skip global temporary tables for which storage was not initialized by
+	 * this backend.
 	 */
 	if (RELATION_IS_GLOBAL_TEMP(OldHeap))
 	{
 		if (gtt_storage_attached(RelationGetRelid(OldHeap)))
 			ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("not support cluster global temporary table")));
+				 errmsg("clustering is not supported on a global temporary table")));
 
 		relation_close(OldHeap, AccessExclusiveLock);
 		pgstat_progress_end_command();
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index af12dd2cdc..4d5aedf2c3 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -461,7 +461,7 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt)
 		if (is_other_backend_use_gtt(RelationGetRelid(seqrel)))
 			ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter global temporary sequence %s when other backend attached it.",
+				 errmsg("cannot alter global temporary sequence %s being accessed by another backend",
 						RelationGetRelationName(seqrel))));
 	}
 
@@ -1194,7 +1194,7 @@ init_sequence(Oid relid, SeqTable *p_elm, Relation *p_rel)
 	*p_elm = elm;
 	*p_rel = seqrel;
 
-	/* Initializes the storage for sequence which the global temporary table belongs. */
+	/* Initializes the storage for sequence which belongs to a global temporary table. */
 	if (RELATION_IS_GLOBAL_TEMP(seqrel) &&
 		!gtt_storage_attached(RelationGetRelid(seqrel)))
 	{
@@ -1999,7 +1999,7 @@ get_seqence_start_value(Oid seqid)
 }
 
 /*
- * Initialize sequence which global temporary table belongs.
+ * Initialize sequence which belongs to a global temporary table.
  */
 void
 gtt_init_seq(Relation rel)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7aa3beba2c..e4b67f8731 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2018,8 +2018,8 @@ ExecuteTruncateGuts(List *explicit_rels,
 		}
 
 		/*
-		 * Skip the global temporary table that is not initialized for storage
-		 * in current backend.
+		 * Skip global temporary tables for which the current backend has not
+		 * initialized storage.
 		 */
 		if (RELATION_IS_GLOBAL_TEMP(rel))
 		{
@@ -3365,7 +3365,7 @@ CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId)
 	if (RELATION_IS_GLOBAL_TEMP(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("The tablespace of global temporary table can not be changed")));
+				 errmsg("The tablespace of global temporary table cannot be changed")));
 
 	return true;
 }
@@ -4136,13 +4136,13 @@ AlterTable(AlterTableStmt *stmt, LOCKMODE lockmode,
 	/* Caller is required to provide an adequate lock. */
 	rel = relation_open(context->relid, NoLock);
 
-	/* We allow to alter global temporary table only current backend use it */
+	/* We disallow altering a global temporary table if another backend is using it */
 	if (RELATION_IS_GLOBAL_TEMP(rel))
 	{
 		if (is_other_backend_use_gtt(RelationGetRelid(rel)))
 			ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter global temporary table %s when other backend attached it.",
+				 errmsg("cannot alter global temporary table %s being accessed by another backend",
 						RelationGetRelationName(rel))));
 	}
 
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 47c734aa1c..3a48892feb 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2140,7 +2140,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	}
 
 	/*
-	 * Skip those global temporary table that are not initialized in
+	 * Skip global temporary tables that are not initialized in the
 	 * current backend.
 	 */
 	if (RELATION_IS_GLOBAL_TEMP(rel) &&
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index cd2bd57e38..79e4b0c9c7 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -543,7 +543,7 @@ DefineView(ViewStmt *stmt, const char *queryString,
 				(errcode(ERRCODE_SYNTAX_ERROR),
 				 errmsg("views cannot be unlogged because they do not have storage")));
 
-	/* Global temporary table are not sensible. */
+	/* Global temporary views are not sensible. */
 	if (stmt->view->relpersistence == RELPERSISTENCE_GLOBAL_TEMP)
 		ereport(ERROR,
 				(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 75a0940b25..4b96429f30 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2912,7 +2912,7 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
 		if (is_query_using_gtt(query))
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("materialized views must not use global temporary tables or views")));
+					 errmsg("materialized views must not use global temporary tables")));
 
 		/*
 		 * A materialized view would either need to save parameters for use in
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 45241b8b0f..c9bef46339 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -449,7 +449,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 
 	/*
 	 * If a sequence is bound to a global temporary table, then the sequence
-	 * must been "global temporary"
+	 * must be "global temporary"
 	 */
 	if (cxt->relation->relpersistence == RELPERSISTENCE_GLOBAL_TEMP)
 		seqstmt->sequence->relpersistence = cxt->relation->relpersistence;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 5601f15d18..fa8c391d40 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2119,8 +2119,8 @@ do_autovacuum(void)
 		else if (classForm->relpersistence == RELPERSISTENCE_GLOBAL_TEMP)
 		{
 			/*
-			 * Aotuvacuum cannot vacuum the private data stored in each backend
-			 * that belongs to global temporary table, so skip them.
+			 * Autovacuum cannot vacuum the private data stored in each backend
+			 * that belongs to global temporary tables, so skip them.
 			 */
 			continue;
 		}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index e9cea9b423..0b207dca16 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -5180,6 +5180,9 @@ KnownAssignedXidsReset(void)
 /*
  * search all active backend to get oldest frozenxid
  * for global temporary table.
+ * Return the oldest frozenxid, or InvalidTransactionId if none.
+ * If pids!=NULL, it's populated with qualifying backend pids.
+ * If xids!=NULL, it's populated with qualifying backend frozenxids.
  */
 int
 list_all_backend_gtt_frozenxids(int max_size, int *pids, uint32 *xids, int *n)
@@ -5216,7 +5219,7 @@ list_all_backend_gtt_frozenxids(int max_size, int *pids, uint32 *xids, int *n)
 	if (max_size > 0 && max_size < arrayP->numProcs)
 	{
 		LWLockRelease(ProcArrayLock);
-		elog(ERROR, "list_all_gtt_frozenxids require more array");
+		elog(ERROR, "list_all_gtt_frozenxids requires a longer array");
 	}
 
 	for (index = 0; index < arrayP->numProcs; index++)
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 9a6e7cee24..b86bc4fc69 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -984,7 +984,7 @@ pg_relation_filepath(PG_FUNCTION_ARGS)
 			break;
 		case RELPERSISTENCE_GLOBAL_TEMP:
 			/*
-			 * For global temporary table ,each backend has its own storage,
+			 * For global temporary table, each backend has its own storage,
 			 * also only sees its own storage. Use Backendid to identify them.
 			 */
 			backend = BackendIdForTempRelations();
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3100ddc704..caab5dc36a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15431,7 +15431,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 
 		/*
 		 * Transaction information for the global temporary table is not stored
-		 * in the pg_class.
+		 * in pg_class.
 		 */
 		if (tbinfo->relpersistence == RELPERSISTENCE_GLOBAL_TEMP)
 		{
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index e23cb49c01..658e9be8de 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -169,10 +169,10 @@ DECLARE_INDEX(pg_class_tblspc_relfilenode_index, 3455, ClassTblspcRelfilenodeInd
 #define		  RELKIND_PARTITIONED_TABLE 'p' /* partitioned table */
 #define		  RELKIND_PARTITIONED_INDEX 'I' /* partitioned index */
 
+#define		  RELPERSISTENCE_GLOBAL_TEMP	'g' /* global temporary table */
 #define		  RELPERSISTENCE_PERMANENT	'p' /* regular table */
 #define		  RELPERSISTENCE_UNLOGGED	'u' /* unlogged permanent table */
 #define		  RELPERSISTENCE_TEMP		't' /* temporary table */
-#define		  RELPERSISTENCE_GLOBAL_TEMP	'g' /* global temporary table */
 
 /* default selection for replica identity (primary key or nothing) */
 #define		  REPLICA_IDENTITY_DEFAULT	'd'
-- 
2.17.1



  [text/x-diff] 0006-f-0004-gtt-v64-regress.txt (4.4K, ../../[email protected]/4-0006-f-0004-gtt-v64-regress.txt)
  download | inline diff:
From 8311d1d40294865a419b9bd0950c3cd642744e28 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Sat, 1 Jan 2022 16:46:54 -0600
Subject: [PATCH 06/11] f!0004-gtt-v64-regress.patch

---
 src/test/regress/expected/global_temporary_table.out | 12 ++++++------
 src/test/regress/sql/global_temporary_table.sql      |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/test/regress/expected/global_temporary_table.out b/src/test/regress/expected/global_temporary_table.out
index 2ada5fa6b9..0aeb53d46d 100644
--- a/src/test/regress/expected/global_temporary_table.out
+++ b/src/test/regress/expected/global_temporary_table.out
@@ -1,5 +1,5 @@
 --
--- GLobal emparary table test case 
+-- Global temporary table test case 
 --
 CREATE SCHEMA IF NOT EXISTS global_temporary_table;
 set search_path=global_temporary_table,sys;
@@ -153,16 +153,16 @@ ERROR:  Only support alter global temporary table in an empty context.
 HINT:  Please create a new connection and execute ALTER TABLE on the new connection.
 -- should fail
 cluster gtt_on_commit_default using gtt_on_commit_default_pkey;
-ERROR:  not support cluster global temporary table
+ERROR:  clustering is not supported on a global temporary table
 -- should fail
 create table gtt_on_commit_default(a int primary key, b text) on commit delete rows;
 ERROR:  ON COMMIT can only be used on temporary tables
 -- should fail
 REINDEX (TABLESPACE pg_default) TABLE gtt_test_createindex;
-ERROR:  The tablespace of global temporary table can not be changed
+ERROR:  The tablespace of global temporary table cannot be changed
 -- should fail
 REINDEX (TABLESPACE pg_default) INDEX gtt_idx_1;
-ERROR:  The tablespace of global temporary table can not be changed
+ERROR:  The tablespace of global temporary table cannot be changed
 -- should fail
 alter table gtt_on_commit_default set ( on_commit_delete_rows='true');
 ERROR:  table cannot add or modify on commit parameter by ALTER TABLE command.
@@ -184,7 +184,7 @@ create global temp table gtt4(a int primary key, b text) with(on_commit_delete_r
 ERROR:  could not create global temporary table with on commit and with clause at same time
 -- should fail
 CREATE MATERIALIZED VIEW mv_gtt_on_commit_default as select * from gtt_on_commit_default;
-ERROR:  materialized views must not use global temporary tables or views
+ERROR:  materialized views must not use global temporary tables
 -- test create table as select
 CREATE GLOBAL TEMPORARY TABLE test_create_table_as AS SELECT 1 AS a;
 -- test copy stmt
@@ -482,7 +482,7 @@ select * from pg_gtt_stats where tablename = 'temp_table_test_systemview' order
 (2 rows)
 
 -- get all object info in current schema
-select relname ,relkind, relpersistence, reloptions from pg_class c, pg_namespace n where c.relnamespace = n.oid and n.nspname = 'global_temporary_table' order by relkind,relpersistence,relname;
+select relname, relkind, relpersistence, reloptions from pg_class c, pg_namespace n where c.relnamespace = n.oid and n.nspname = 'global_temporary_table' order by relkind,relpersistence,relname;
                      relname                     | relkind | relpersistence |          reloptions           
 -------------------------------------------------+---------+----------------+-------------------------------
  global_temp_partition_01_2021_id_seq            | S       | g              | {on_commit_delete_rows=false}
diff --git a/src/test/regress/sql/global_temporary_table.sql b/src/test/regress/sql/global_temporary_table.sql
index 51cd9bbbc0..e47c6aa3cf 100644
--- a/src/test/regress/sql/global_temporary_table.sql
+++ b/src/test/regress/sql/global_temporary_table.sql
@@ -1,5 +1,5 @@
 --
--- GLobal emparary table test case 
+-- Global temporary table test case
 --
 
 CREATE SCHEMA IF NOT EXISTS global_temporary_table;
@@ -306,7 +306,7 @@ select schemaname, tablename, relpages, reltuples, relallvisible from pg_gtt_rel
 select * from pg_gtt_stats where tablename = 'temp_table_test_systemview' order by tablename;
 
 -- get all object info in current schema
-select relname ,relkind, relpersistence, reloptions from pg_class c, pg_namespace n where c.relnamespace = n.oid and n.nspname = 'global_temporary_table' order by relkind,relpersistence,relname;
+select relname, relkind, relpersistence, reloptions from pg_class c, pg_namespace n where c.relnamespace = n.oid and n.nspname = 'global_temporary_table' order by relkind,relpersistence,relname;
 
 reset search_path;
 drop schema global_temporary_table cascade;
-- 
2.17.1



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: [Proposal] Global temporary tables
@ 2022-02-27 03:17  Pavel Stehule <[email protected]>
  parent: Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Pavel Stehule @ 2022-02-27 03:17 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: [email protected]; Andrew Bille <[email protected]>; Tony Zhu <[email protected]>; PostgreSQL Hackers <[email protected]>

Hi

You redirect stats from pg_class and pg_statistics to a local hash table.
> This is pretty hairy :(
> I guess you'd also need to handle pg_statistic_ext and ext_data.
> pg_stats doesn't work, since the data isn't in pg_statistic - it'd need to
> look
> at pg_get_gtt_statistics.
>

Without this, the GTT will be terribly slow like current temporary tables
with a lot of problems with bloating of pg_class, pg_attribute and
pg_depend tables.

Regards

Pavel


^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2022-02-27 03:17 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10 06:19 [PATCH v24 7/7] Add support for PRESERVE Dilip Kumar <[email protected]>
2022-02-27 00:21 Re: [Proposal] Global temporary tables Justin Pryzby <[email protected]>
2022-02-27 03:17 ` Re: [Proposal] Global temporary tables Pavel Stehule <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox