Cherry-pick series of AO options/AO relations related commits.

src/backend/access/common/reloptions_gp.c

@@ -60,6 +60,7 @@ static relopt_bool boolRelOpts_gp[] =
 
 static relopt_int intRelOpts_gp[] =
 {
+
 	{
 		{
 			SOPT_FILLFACTOR,
@@ -116,6 +117,7 @@ static relopt_string stringRelOpts_gp[] =
 
 static void free_options_deep(relopt_value *options, int num_options);
 static relopt_value *get_option_set(relopt_value *options, int num_options, const char *opt_name);
+static bool reloption_is_default(const char *optstr, int optlen);
 
 /*
  * initialize_reloptions_gp
@@ -615,21 +617,6 @@ transformAOStdRdOptions(StdRdOptions *opts, Datum withOpts)
 				astate = accumArrayResult(astate, d, false, TEXTOID,
 										  CurrentMemoryContext);
 			}
-
-			/*
-			 * Record fillfactor only if it's specified in WITH clause.
-			 * Default fillfactor is assumed otherwise.
-			 */
-			soptLen = strlen(SOPT_FILLFACTOR);
-			if (withLen > soptLen &&
-				pg_strncasecmp(strval, SOPT_FILLFACTOR, soptLen) == 0)
-			{
-				d = CStringGetTextDatum(psprintf("%s=%d",
-												 SOPT_FILLFACTOR,
-												 opts->fillfactor));
-				astate = accumArrayResult(astate, d, false, TEXTOID,
-										  CurrentMemoryContext);
-			}
 		}
 	}
 
@@ -696,25 +683,139 @@ transformAOStdRdOptions(StdRdOptions *opts, Datum withOpts)
 		PointerGetDatum(NULL);
 }
 
+/* 
+ * Check if the given reloption string has default value.
+ */
+static bool
+reloption_is_default(const char *optstr, int optlen)
+{
+	char 		*defaultopt = NULL;
+	bool 		res;
+
+	if (optlen > strlen(SOPT_BLOCKSIZE) &&
+		pg_strncasecmp(optstr, SOPT_BLOCKSIZE, strlen(SOPT_BLOCKSIZE)) == 0)
+	{
+		defaultopt = psprintf("%s=%d",
+										 SOPT_BLOCKSIZE,
+										 AO_DEFAULT_BLOCKSIZE);
+	}
+	else if (optlen > strlen(SOPT_COMPTYPE) &&
+		pg_strncasecmp(optstr, SOPT_COMPTYPE, strlen(SOPT_COMPTYPE)) == 0)
+	{
+		defaultopt = psprintf("%s=%s",
+										 SOPT_COMPTYPE,
+										 AO_DEFAULT_COMPRESSTYPE);
+	}
+	else if (optlen > strlen(SOPT_COMPLEVEL) &&
+		pg_strncasecmp(optstr, SOPT_COMPLEVEL, strlen(SOPT_COMPLEVEL)) == 0)
+	{
+		defaultopt = psprintf("%s=%d",
+										 SOPT_COMPLEVEL,
+										 AO_DEFAULT_COMPRESSLEVEL);
+	}
+	else if (optlen > strlen(SOPT_CHECKSUM) &&
+		pg_strncasecmp(optstr, SOPT_CHECKSUM, strlen(SOPT_CHECKSUM)) == 0)
+	{
+		defaultopt = psprintf("%s=%s",
+										 SOPT_CHECKSUM,
+										 AO_DEFAULT_CHECKSUM ? "true" : "false");
+	}
+
+	if (defaultopt != NULL)
+		res = strlen(defaultopt) == optlen && 
+				pg_strncasecmp(optstr, defaultopt, optlen) == 0;
+	else
+		res = false;
+
+	if (defaultopt)
+		pfree(defaultopt);
+
+	return res;
+}
+
+/* 
+ * Check if two string arrays of reloptions are the same.
+ *
+ * Note that this will not handle the case where the option doesn't contain 
+ * the '=' sign in it, e.g. "checksum" vs. "checksum=true". But it seems 
+ * that at this point we should always have both options as "x=y" anyways.
+ */
+bool
+relOptionsEquals(Datum oldOptions, Datum newOptions)
+{
+	ArrayType 	*oldoptarray, *newoptarray;
+	Datum 		*opts1, *opts2;
+	int		noldoptions = 0, nnewoptions = 0;
+	int		i, j;
+
+	/* Deconstruct both options. */
+	if (PointerIsValid(DatumGetPointer(oldOptions)))
+	{
+		oldoptarray = DatumGetArrayTypeP(oldOptions);
+		deconstruct_array(oldoptarray, TEXTOID, -1, false, 'i',
+						  &opts1, NULL, &noldoptions);
+	}
+	if (PointerIsValid(DatumGetPointer(newOptions)))
+	{
+		newoptarray = DatumGetArrayTypeP(newOptions);
+		deconstruct_array(newoptarray, TEXTOID, -1, false, 'i',
+						  &opts2, NULL, &nnewoptions);
+	}
+
+	for (i = 0; i < nnewoptions; i++)
+	{
+		char 	*newopt_str = VARDATA(opts2[i]);
+		int	newopt_len = VARSIZE(opts2[i]) - VARHDRSZ;
+		int 	keylen;
+
+		/* Should be "x=y" but better panic here rather than returning wrong result. */
+		Assert(strchr(newopt_str, '=') != 0);
+
+		keylen = strchr(newopt_str, '=') - newopt_str;
+
+		/* Search for a match in old options. */
+		for (j = 0; j < noldoptions; j++)
+		{
+			char 	*oldopt_str = VARDATA(opts1[j]);
+			int	oldopt_len = VARSIZE(opts1[j]) - VARHDRSZ;
+
+			/* Not the same option. */
+			if (oldopt_len <= keylen || 
+					pg_strncasecmp(oldopt_str, newopt_str, keylen) != 0)
+				continue;
+
+			/* Old option should be as "x=y" too. */
+			Assert(oldopt_str[keylen] == '=');
+
+			/* Key found, now they must match exactly otherwise it's a changed option. */
+			if (oldopt_len != newopt_len ||
+					pg_strncasecmp(oldopt_str, newopt_str, oldopt_len) != 0)
+				return false;
+			else
+				break;
+		}
+
+		/* 
+		 * If key not found, then it must've changed unless it's a default value 
+		 * that doesn't appear in the old reloptions.
+		 */
+		if (j == noldoptions && !reloption_is_default(newopt_str, newopt_len))
+			return false;
+	}
+	return true;
+}
+
 void
 validate_and_adjust_options(StdRdOptions *result,
 							relopt_value *options,
 							int num_options, relopt_kind kind, bool validate)
 {
 	int			i;
-	relopt_value *fillfactor_opt;
 	relopt_value *blocksize_opt;
 	relopt_value *comptype_opt;
 	relopt_value *complevel_opt;
 	relopt_value *checksum_opt;
 
-	/* fillfactor */
-	fillfactor_opt = get_option_set(options, num_options, SOPT_FILLFACTOR);
-	if (fillfactor_opt != NULL)
-	{
-		result->fillfactor = fillfactor_opt->values.int_val;
-	}
-
 	/* blocksize */
 	blocksize_opt = get_option_set(options, num_options, SOPT_BLOCKSIZE);
 	if (blocksize_opt != NULL)
@@ -906,9 +1007,6 @@ validate_and_refill_options(StdRdOptions *result, relopt_value *options,
 		ao_storage_opts_changed &&
 		KIND_IS_APPENDOPTIMIZED(kind))
 	{
-		if (!(get_option_set(options, numrelopts, SOPT_FILLFACTOR)))
-			result->fillfactor = ao_storage_opts.fillfactor;
-
 		if (!(get_option_set(options, numrelopts, SOPT_BLOCKSIZE)))
 			result->blocksize = ao_storage_opts.blocksize;
 

src/backend/commands/cluster.c

@@ -739,7 +739,6 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
 	Datum		reloptions;
 	bool		isNull;
 	Oid			namespaceid;
-	bool 		valid_opts;
 
 	OldHeap = table_open(OIDOldHeap, lockmode);
 	OldHeapDesc = RelationGetDescr(OldHeap);
@@ -763,30 +762,70 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
 		reloptions = (Datum) 0;
 
 	/* 
-	 * Unless we are changing access method, look further.
+	 * Unless we are changing access method between heap and AO/CO, look further.
 	 */
 	/*
-	 * While changing access method from heap to AO/AOCO, the storage options
-	 * need to be picked from gp_default_storage_options since heap table does
-	 * not store those.
+	 * GPDB: some considerations when AM is going to change between heap and AO/CO:
+	 *
+	 * If user has also requested setting new reloptions, the new reloptions should have
+	 * replaced the old ones at this point. We just need to reuse those on the new table.
+	 *
+	 * If user does NOT request new reloptions, we should discard the existing reloptions.
+	 * And one more consideration if we are changing the table from heap to AO: we should
+	 * also pick up options from gp_default_storage_options, just like CREATE TABLE does.
 	 */
 	if (RelationIsHeap(OldHeap) && IsAccessMethodAO(NewAccessMethod))
 	{
-		valid_opts = false;
-		reloptions = (Datum) 0;
+		/*
+		 * Heap to AO/CO: filter out any reloptions that belong to heap, 
+		 * and pick up from gp_default_storage_options.
+		 */
+		int 		numoptions;
+		relopt_value 	*options;
+
+		/* 
+		 * Process the reloptions as for AO tables. And validate=false will silently 
+		 * filter out any reloptions that belong to heap.
+		 */
 		StdRdOptions *stdRdOptions = (StdRdOptions *)default_reloptions(reloptions,
-																		true,
-																		RELOPT_KIND_APPENDOPTIMIZED);
+																	false, /* validate */
+																	RELOPT_KIND_APPENDOPTIMIZED);
+
+		/* Pick up from gp_default_storage_options. */
+		options = parseRelOptions(reloptions, false, RELOPT_KIND_APPENDOPTIMIZED, &numoptions);
+		validate_and_refill_options(stdRdOptions, options, numoptions, RELOPT_KIND_APPENDOPTIMIZED, true);
+
+		/* Update the reloptions string. */
 		reloptions = transformAOStdRdOptions(stdRdOptions, reloptions);
 	}
-	/* When changing from AO/AOCO to heap, do not use any of the AO reloptions */
 	else if (RelationIsAppendOptimized(OldHeap) && NewAccessMethod == HEAP_TABLE_AM_OID)
 	{
-		valid_opts = true;
-		reloptions = (Datum) 0;
+		/*
+		 * AO/CO to Heap: unfortunately we don't have a convenient routine to transform
+		 * heap StdRdOptions back to reloption string. So we take a slightly different
+		 * approach than the case of heap to AO/CO: we check if there is any AO reloptions:
+		 * 
+		 * (1) If there is, just discard them (AO options do not apply to heap). 
+		 * (2) If there is none, that means we either have replaced it with heap reloptions
+		 * or the reloptions field is just empty, and either way we will pass the existing
+		 * reloptions on to the new table.
+		 *
+		 * This is possible because at this point we only have either AO/AOCO reloptions or
+		 * heap reloptions, but we cannot have both (see ATExecSetRelOptions).
+		 */
+		Datum 	aoreloptions = (Datum) 0;
+		StdRdOptions *stdRdOptions = (StdRdOptions *)default_reloptions(reloptions,
+																	false, /* validate */
+																	RELOPT_KIND_APPENDOPTIMIZED);
+
+		/*
+		 * Transform the stdRdOptions to get a reloptions string, from which we will 
+		 * know if there is any AO reloptions.
+		 */
+		aoreloptions = transformAOStdRdOptions(stdRdOptions, aoreloptions);
+		if (aoreloptions != (Datum) 0)
+			reloptions = (Datum) 0;
 	}
-	else
-		valid_opts = true;
 
 	if (relpersistence == RELPERSISTENCE_TEMP)
 		namespaceid = LookupCreationNamespace("pg_temp");
@@ -829,7 +868,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
 										  true,
 										  OIDOldHeap,
 										  NULL,
-										  valid_opts);
+										  true);
 	Assert(OIDNewHeap != InvalidOid);
 
 	ReleaseSysCache(tuple);