Ao set ao aoco-related cherry-picks

src/backend/access/common/reloptions_gp.c

@@ -1670,7 +1670,7 @@ find_crsd(const char *column, List *stenc)
 List *
 transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
 						List *stenc, List *withOptions, List *parentenc,
-						bool explicitOnly, bool createDefaultOne)
+						bool explicitOnly, bool createDefaultOne, bool appendonly)
 {
 	ColumnReferenceStorageDirective *deflt = NULL;
 	ListCell   *lc;
@@ -1713,7 +1713,10 @@ transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
 
 			deflt = copyObject(c);
 
-			deflt->encoding = tam->transform_column_encoding_clauses(rel, deflt->encoding, true, false);
+			if (appendonly)
+				deflt->encoding = tam->transform_column_encoding_clauses(rel, deflt->encoding, true, false);
+			else
+				deflt->encoding = transformStorageEncodingClause(deflt->encoding, true);
 
 			/*
 			 * The default encoding and the with clause better not
@@ -1742,8 +1745,10 @@ transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
 			 * if current am not inmplement transform_column_encoding_clauses
 			 * then tmpenc not null but no need fill with options.
 			 */
-			if (tam->transform_column_encoding_clauses)
+			if (tam->transform_column_encoding_clauses && appendonly)
 				deflt->encoding = tam->transform_column_encoding_clauses(rel, tmpenc, false, false);
+			else
+				deflt->encoding = transformStorageEncodingClause(tmpenc, false);
 		}
 	}
 
@@ -1781,9 +1786,10 @@ transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
 			ColumnReferenceStorageDirective *s = find_crsd(d->colname, stenc);
 
 			if (s) {
-				encoding = tam->transform_column_encoding_clauses(rel, s->encoding, true, false);
+				if (tam->transform_column_encoding_clauses)
+					encoding = tam->transform_column_encoding_clauses(rel, s->encoding, true, false);
 			} else {
-				if (deflt)
+				if (deflt && deflt->encoding != NULL)
 					encoding = copyObject(deflt->encoding);
 				else if (!explicitOnly)
 				{
@@ -1796,7 +1802,7 @@ transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
 					else if (d->typeName) {
 						/* get encoding by type, still need do transform and validate */
 						encoding = get_type_encoding(d->typeName);
-						if (tam->transform_column_encoding_clauses)
+						if (tam->transform_column_encoding_clauses && appendonly)
 							encoding = tam->transform_column_encoding_clauses(rel, encoding, true, true);
 					}
 					if (!encoding && createDefaultOne) {

src/backend/catalog/pg_appendonly.c

@@ -21,6 +21,7 @@
 
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_appendonly.h"
+#include "catalog/pg_attribute_encoding.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_proc.h"
 #include "catalog/gp_fastsequence.h"
@@ -30,6 +31,7 @@
 #include "access/table.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
+#include "catalog/pg_attribute_encoding.h"
 #include "utils/builtins.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
@@ -547,9 +549,7 @@ ATAOEntries(Form_pg_class relform1, Form_pg_class relform2,
 					TransferAppendonlyEntries(relform2->oid, relform1->oid);
 					break;
 				case AO_COLUMN_TABLE_AM_OID:
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								errmsg("alter table does not support switch from Heap to AOCO")));
+					TransferAppendonlyEntries(relform2->oid, relform1->oid);
 					break;
 				case HEAP_TABLE_AM_OID:
 				default:
@@ -578,9 +578,7 @@ ATAOEntries(Form_pg_class relform1, Form_pg_class relform2,
 					swapAppendonlyEntriesUsingTAM(relform1, relform2, frozenXid, cutoffMulti);
 					break;
 				case AO_COLUMN_TABLE_AM_OID:
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								errmsg("alter table does not support switch from AO to AOCO")));
+					SwapAppendonlyEntries(relform1->oid, relform2->oid);
 					break;
 				default:
 					ereport(ERROR,
@@ -592,14 +590,16 @@ ATAOEntries(Form_pg_class relform1, Form_pg_class relform2,
 			switch(relform2->relam)
 			{
 				case HEAP_TABLE_AM_OID:
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								errmsg("alter table does not support switch from AOCO to Heap")));
+					/* For pg_appendonly entries, it's the same as AO->Heap. */
+					TransferAppendonlyEntries(relform1->oid, relform2->oid);
+					/* Remove the pg_attribute_encoding entries, since heap tables shouldn't have these. */
+					RemoveAttributeEncodingsByRelid(relform1->oid);
 					break;
 				case AO_ROW_TABLE_AM_OID:
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								errmsg("alter table does not support switch from AOCO to AO")));
+					/* For pg_appendonly entries, it's same as AO->AO/CO. */
+					SwapAppendonlyEntries(relform1->oid, relform2->oid);
+					/* For pg_attribute_encoding entries, it's same as AOCO->heap.*/
+					RemoveAttributeEncodingsByRelid(relform1->oid);
 					break;
 				case AO_COLUMN_TABLE_AM_OID:
 					swapAppendonlyEntriesUsingTAM(relform1, relform2, frozenXid, cutoffMulti);

src/backend/commands/cluster.c

@@ -908,14 +908,21 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
 		ReleaseSysCache(tuple);
 	}
 
-	if (RelationIsAppendOptimized(OldHeap) || NewAccessMethod == AO_ROW_TABLE_AM_OID)
+	if (IsAccessMethodAO(NewAccessMethod))
 		NewRelationCreateAOAuxTables(OIDNewHeap, createAoBlockDirectory);
 
 	CacheInvalidateRelcacheByRelid(OIDNewHeap);
 
-	cloneAttributeEncoding(OIDOldHeap,
-						   OIDNewHeap,
-						   RelationGetNumberOfAttributes(OldHeap));
+	/* 
+	 * Copy the pg_attribute_encoding entries over if new table needs them.
+	 * Note that in the case of AM change from heap/ao to aoco, we still need 
+	 * to do this since we created those entries for the heap/ao table at the 
+	 * phase 2 of ATSETAM (see ATExecCmd).
+	 */
+	if (NewAccessMethod == AO_COLUMN_TABLE_AM_OID)
+		cloneAttributeEncoding(OIDOldHeap,
+							   OIDNewHeap,
+							   RelationGetNumberOfAttributes(OldHeap));
 
 	table_close(OldHeap, NoLock);
 

src/backend/commands/tablecmds.c

@@ -584,6 +584,8 @@ static bool prebuild_temp_table(Relation rel, RangeVar *tmpname, DistributedBy *
 
 
 static void checkATSetDistributedByStandalone(AlteredTableInfo *tab, Relation rel);
+static void populate_rel_col_encodings(Relation rel, List *stenc, List *withOptions, Oid newAm);
+static void clear_rel_opts(Relation rel);
 
 
 /* ----------------------------------------------------------------
@@ -1145,7 +1147,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 								stmt->options,
 								parentenc,
 								relkind == RELKIND_PARTITIONED_TABLE,
-								AMHandlerIsAoCols(amHandlerOid) /* createDefaultOne*/);
+								AMHandlerIsAoCols(amHandlerOid) /* createDefaultOne*/, true);
+
 		if (!AMHandlerSupportEncodingClause(tam) && relkind != RELKIND_PARTITIONED_TABLE)
 			stmt->attr_encodings = NIL;
 	}
@@ -1332,7 +1335,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 									false,
 									accessMethodId != AO_COLUMN_TABLE_AM_OID
 									&& !stmt->partbound && !stmt->partspec
-									/* errorOnEncodingClause */);
+									/* errorOnEncodingClause */, true);
 
 		AddRelationAttributeEncodings(rel, part_attr_encodings);
 	}
@@ -4818,6 +4821,41 @@ AlterTable(AlterTableStmt *stmt, LOCKMODE lockmode,
 	}
 }
 
+/* 
+ * Populate the column encoding option for each column in the relation. 
+ */
+static void populate_rel_col_encodings(Relation rel, List *stenc, List *withOptions, Oid newAccessMethod)
+{
+	int 		attno;
+	List 		*colDefs = NIL;
+	const TableAmRoutine *tam;
+	TupleDesc 	tupdesc = RelationGetDescr(rel);
+
+	/* Figure out the column definition list. */
+	for (attno = 0; attno < tupdesc->natts; attno++)
+	{
+		Form_pg_attribute 	att = TupleDescAttr(tupdesc, attno);
+		ColumnDef 		*cd = makeColumnDef(NameStr(att->attname),
+								att->atttypid, 
+								att->atttypmod,
+								0);
+		colDefs = lappend(colDefs, cd);
+	}
+
+	tam = GetTableAmRoutineByAmId(newAccessMethod);
+
+	List *attr_encodings = transformColumnEncoding(tam /* TableAmRoutine */, rel,
+							colDefs /*column clauses*/,
+							stenc /*encoding clauses*/,
+							withOptions /*withOptions*/,
+							NULL,
+							false,
+							newAccessMethod == AO_COLUMN_TABLE_AM_OID, RelationIsAppendOptimized(rel));
+
+
+	AddRelationAttributeEncodings(rel, attr_encodings);
+}
+
 /*
  * AlterTableInternal
  *
@@ -6165,7 +6203,6 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
 			break;
 		case AT_SetAccessMethod:	/* SET ACCESS METHOD */
 			/* Set reloptions if specified any. Otherwise handled specially in Phase 3. */
-			if (cmd->def)
 			{
 				bool aoopt_changed = false;
 
@@ -6179,6 +6216,11 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
 				if (aoopt_changed)
 					tab->rewrite |= AT_REWRITE_ALTER_RELOPTS;
 			}
+
+			/* If we are changing AM to AOCO, add pg_attribute_encoding entries for each column. */
+			if (tab->newAccessMethod == AO_COLUMN_TABLE_AM_OID) 
+				populate_rel_col_encodings(rel, NULL, (List*)cmd->def, tab->newAccessMethod);
+
 			break;
 		case AT_SetTableSpace:	/* SET TABLESPACE */
 
@@ -8679,7 +8721,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 						NULL /* withOptions */,
 						NULL /* parentenc */,
 						false /* explicitOnly */,
-						RelationIsAoCols(rel) /* createDefaultOne */);
+						RelationIsAoCols(rel) /* createDefaultOne */, true);
 		/*
 		 * Store the encoding clause for AO/CO tables.
 		 */
@@ -16063,7 +16105,7 @@ ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacen
  *
  * GPDB specific arguments: 
  * 	aoopt_changed: whether any AO storage options have been changed in this function.
- * 	am_change_heap_ao: whether we are changing the AM from heap->AO/CO or vice-versa.
+ * 	newam: the new AM if we will change the table AM. It's InvalidOid if no change is needed.
  */
 static void
 ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
@@ -16085,7 +16127,14 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 	if (defList == NIL && operation != AT_ReplaceRelOptions)
 		return;					/* nothing to do */
 
-	newAM = OidIsValid(newAccessMethod) ? GetTableAmRoutineByAmId(newAccessMethod) : NULL;
+
+	/* If we are changing access method, simply remove all the existing ones. */
+	if (OidIsValid(newAccessMethod))
+	{
+		newAM = GetTableAmRoutineByAmId(newAccessMethod);
+		clear_rel_opts(rel);
+	} else
+		newAM = NULL;
 
 	pgclass = table_open(RelationRelationId, RowExclusiveLock);
 
@@ -16095,14 +16144,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 	if (!HeapTupleIsValid(tuple))
 		elog(ERROR, "cache lookup failed for relation %u", relid);
 
-	if (operation == AT_ReplaceRelOptions || 
-		(newAccessMethod != InvalidOid && IsAccessMethodAO(rel->rd_rel->relam) != IsAccessMethodAO(newAccessMethod)))
+	if (operation == AT_ReplaceRelOptions)
 	{
 		/*
-		 * If we're supposed to replace the reloptions list, or if we're
-		 * changing AM between heap and AO/CO so the old reloptions won't 
-		 * apply to the new table anymore, we just pretend there were
-		 * none before.
+		 * If we're supposed to replace the reloptions list, we just 
+		 * pretend there were none before.
 		 */
 		datum = (Datum) 0;
 		isnull = true;
@@ -17957,6 +18003,39 @@ get_rel_opts(Relation rel)
 	return newOptions;
 }
 
+/*
+ * GPDB: Convenience function to clear the pg_class.reloptions field for a given relation.
+ */
+static void
+clear_rel_opts(Relation rel)
+{
+	Datum           val[Natts_pg_class] = {0};
+	bool            null[Natts_pg_class] = {0};
+	bool            repl[Natts_pg_class] = {0};
+	Relation 	classrel;
+	HeapTuple 	tup;
+
+	classrel = table_open(RelationRelationId, RowExclusiveLock);
+
+	tup = SearchSysCacheCopy1(RELOID, RelationGetRelid(rel));
+	if (!HeapTupleIsValid(tup))
+		elog(ERROR, "cache lookup failed for relation %u", RelationGetRelid(rel));
+
+	val[Anum_pg_class_reloptions - 1] = (Datum) 0;
+	null[Anum_pg_class_reloptions - 1] = true;
+	repl[Anum_pg_class_reloptions - 1] = true;
+
+	tup = heap_modify_tuple(tup, RelationGetDescr(classrel),
+								val, null, repl);
+	CatalogTupleUpdate(classrel, &tup->t_self, tup);
+
+	heap_freetuple(tup);
+
+	table_close(classrel, RowExclusiveLock);
+
+	CommandCounterIncrement(); 
+}
+
 static RangeVar *
 make_temp_table_name(Relation rel, BackendId id)
 {

src/backend/nodes/makefuncs.c

@@ -506,6 +506,7 @@ makeColumnDef(const char *colname, Oid typeOid, int32 typmod, Oid collOid)
 	n->collClause = NULL;
 	n->collOid = collOid;
 	n->constraints = NIL;
+	n->encoding = NIL;
 	n->fdwoptions = NIL;
 	n->location = -1;
 

src/backend/parser/gram.y

@@ -3809,38 +3809,36 @@ alter_table_cmd:
 					n->newowner = $3;
 					$$ = (Node *)n;
 				}
-			/* ALTER TABLE <name> SET ACCESS METHOD <amname> */
-			| SET ACCESS METHOD name
-				{
-					AlterTableCmd *n = makeNode(AlterTableCmd);
-					n->subtype = AT_SetAccessMethod;
-					n->name = $4;
-					$$ = (Node *)n;
-				}
 			/* ALTER TABLE <name> SET ACCESS METHOD <amname> WITH (<reloptions>) */
-			| SET ACCESS METHOD name WITH definition
+			| SET ACCESS METHOD name OptWith
 				{
 					AlterTableCmd *n = makeNode(AlterTableCmd);
-					char *witham = greenplumLegacyAOoptions(n->name, &$6);
+					char *witham = greenplumLegacyAOoptions(n->name, &$5);
 					n->subtype = AT_SetAccessMethod;
 					n->name = $4;
 					/*
 					 * If there's any legacy AO options specified in the WITH
 					 * clause such as 'appendonly' or 'appendoptimized', it has
 					 * to match with the AM name.
 					 */
-					if (witham && 
-							(strlen(witham) != strlen(n->name) || 
-							strncmp(n->name, witham, strlen(n->name) != 0)))
+					if (witham) 
 					{
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-								 errmsg("ACCESS METHOD is specified as \"%s\" but "
-								 	"the WITH option indicates it to be \"%s\"",
-									n->name, witham),
-								 parser_errposition(@5)));
+						if (strlen(witham) != strlen(n->name) || 
+							strncmp(n->name, witham, strlen(n->name) != 0))
+							ereport(ERROR,
+									(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+									 errmsg("ACCESS METHOD is specified as \"%s\" but "
+										"the WITH option indicates it to be \"%s\"",
+										n->name, witham),
+									 parser_errposition(@5)));
+						else
+							ereport(NOTICE,
+									(errcode(ERRCODE_SYNTAX_ERROR),
+									 errmsg("Redundant clauses are used to indicate the access method."),
+									 errhint("Only one of these is needed to indicate access method: the "
+										"SET ACCESS METHOD clause or the options in the WITH clause.")));
 					}
-					n->def = (Node *) $6;
+					n->def = (Node *) $5;
 					$$ = (Node *)n;
 				}
 			/* ALTER TABLE <name> SET TABLESPACE <tablespacename> */

src/include/access/reloptions.h

@@ -305,7 +305,7 @@ extern void validate_and_adjust_options(StdRdOptions *result, relopt_value *opti
 extern void validateAOCOColumnEncodingClauses(List *aocoColumnEncoding);
 extern List *transformColumnEncoding(const TableAmRoutine *tam, Relation rel, List *colDefs,
 										List *stenc, List *withOptions, List *parentenc,
-										bool explicitOnly, bool createDefaultOne);
+										bool explicitOnly, bool createDefaultOne, bool appendonly);
 
 List* transfromColumnEncodingAocoRootPartition(List *colDefs, List *stenc, List *withOptions, bool errorOnEncodingClause);