Remove restrictions for changing compression settings

.unreleased/pr_6545

@@ -0,0 +1 @@
+Implements: #6545 Remove restrictions for changing compression settings

src/compression_with_clause.c

@@ -5,19 +5,21 @@
  */
 
 #include <postgres.h>
-#include <fmgr.h>
 #include <access/htup_details.h>
 #include <catalog/dependency.h>
 #include <catalog/namespace.h>
-#include <catalog/pg_type.h>
 #include <catalog/pg_trigger.h>
+#include <catalog/pg_type.h>
 #include <commands/trigger.h>
+#include <fmgr.h>
+#include <parser/parser.h>
 #include <storage/lmgr.h>
 #include <utils/builtins.h>
 #include <utils/lsyscache.h>
-#include <parser/parser.h>
+#include <utils/typcache.h>
 
 #include "compat/compat.h"
+#include "ts_catalog/array_utils.h"
 
 #include "compression_with_clause.h"
 
@@ -73,7 +75,7 @@
 	return true;
 }
 
-static List *
+static ArrayType *
 parse_segment_collist(char *inpstr, Hypertable *hypertable)
 {
 	StringInfoData buf;
@@ -83,7 +85,7 @@
 	RawStmt *raw;
 
 	if (strlen(inpstr) == 0)
-		return NIL;
+		return NULL;
 
 	initStringInfo(&buf);
 
@@ -121,7 +123,7 @@
 	if (select->sortClause != NIL)
 		throw_segment_by_error(inpstr);
 
-	List *collist = NIL;
+	ArrayType *segmentby = NULL;
 	foreach (lc, select->groupClause)
 	{
 		if (!IsA(lfirst(lc), ColumnRef))
@@ -134,10 +136,32 @@
 		if (!IsA(linitial(cf->fields), String))
 			throw_segment_by_error(inpstr);
 
-		collist = lappend(collist, pstrdup(strVal(linitial(cf->fields))));
+		char *colname = strVal(linitial(cf->fields));
+		AttrNumber col_attno = get_attnum(hypertable->main_table_relid, colname);
+		if (col_attno == InvalidAttrNumber)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("column \"%s\" does not exist", colname),
+					 errhint("The timescaledb.compress_segmentby option must reference a valid "
+							 "column.")));
+		}
+
+		/* get normalized column name */
+		colname = get_attname(hypertable->main_table_relid, col_attno, false);
+
+		/* check if segmentby columns are distinct. */
+		if (ts_array_is_member(segmentby, colname))
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("duplicate column name \"%s\"", colname),
+					 errhint("The timescaledb.compress_segmentby option must reference distinct "
+							 "column.")));
+
+		segmentby = ts_array_add_element_text(segmentby, pstrdup(colname));
 	}
 
-	return collist;
+	return segmentby;
 }
 
 static inline void
@@ -152,17 +176,18 @@
 }
 
 /* compress_orderby is parsed same as order by in select queries */
-static List *
+static OrderBySettings
 parse_order_collist(char *inpstr, Hypertable *hypertable)
 {
 	StringInfoData buf;
 	List *parsed;
 	ListCell *lc;
 	SelectStmt *select;
 	RawStmt *raw;
+	OrderBySettings settings = { 0 };
 
 	if (strlen(inpstr) == 0)
-		return NIL;
+		return settings;
 
 	initStringInfo(&buf);
 
@@ -199,12 +224,12 @@
 	if (select->groupClause != NIL)
 		throw_order_by_error(inpstr);
 
-	List *collist = NIL;
 	foreach (lc, select->sortClause)
 	{
 		SortBy *sort_by;
 		ColumnRef *cf;
 		CompressedParsedCol *col = (CompressedParsedCol *) palloc(sizeof(*col));
+		bool desc, nullsfirst;
 
 		if (!IsA(lfirst(lc), SortBy))
 			throw_order_by_error(inpstr);
@@ -221,32 +246,65 @@
 			throw_order_by_error(inpstr);
 
 		namestrcpy(&col->colname, strVal(linitial(cf->fields)));
+		char *colname = strVal(linitial(cf->fields));
+
+		AttrNumber col_attno = get_attnum(hypertable->main_table_relid, colname);
+		if (col_attno == InvalidAttrNumber)
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("column \"%s\" does not exist", NameStr(col->colname)),
+					 errhint("The timescaledb.compress_orderby option must reference a valid "
+							 "column.")));
+
+		Oid col_type = get_atttype(hypertable->main_table_relid, col_attno);
+		TypeCacheEntry *type = lookup_type_cache(col_type, TYPECACHE_LT_OPR);
+
+		if (!OidIsValid(type->lt_opr))
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_FUNCTION),
+					 errmsg("invalid ordering column type %s", format_type_be(col_type)),
+					 errdetail("Could not identify a less-than operator for the type.")));
+
+		/* get normalized column name */
+		colname = get_attname(hypertable->main_table_relid, col_attno, false);
+
+		/* check if orderby columns are distinct. */
+		if (ts_array_is_member(settings.orderby, colname))
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("duplicate column name \"%s\"", colname),
+					 errhint("The timescaledb.compress_orderby option must reference distinct "
+							 "column.")));
 
 		if (sort_by->sortby_dir != SORTBY_ASC && sort_by->sortby_dir != SORTBY_DESC &&
 			sort_by->sortby_dir != SORTBY_DEFAULT)
 			throw_order_by_error(inpstr);
-		col->desc = sort_by->sortby_dir == SORTBY_DESC;
+
+		desc = sort_by->sortby_dir == SORTBY_DESC;
 
 		if (sort_by->sortby_nulls == SORTBY_NULLS_DEFAULT)
 		{
 			/* default null ordering is LAST for ASC, FIRST for DESC */
-			col->nullsfirst = col->desc;
+			nullsfirst = desc;
 		}
 		else
 		{
-			col->nullsfirst = sort_by->sortby_nulls == SORTBY_NULLS_FIRST;
+			nullsfirst = sort_by->sortby_nulls == SORTBY_NULLS_FIRST;
 		}
 
-		collist = lappend(collist, (void *) col);
+		settings.orderby = ts_array_add_element_text(settings.orderby, pstrdup(colname));
+		settings.orderby_desc = ts_array_add_element_bool(settings.orderby_desc, desc);
+		settings.orderby_nullsfirst =
+			ts_array_add_element_bool(settings.orderby_nullsfirst, nullsfirst);
 	}
 
-	return collist;
+	return settings;
 }
 
 /* returns List of CompressedParsedCol
  * compress_segmentby = `col1,col2,col3`
  */
-List *
+ArrayType *
 ts_compress_hypertable_parse_segment_by(WithClauseResult *parsed_options, Hypertable *hypertable)
 {
 	if (parsed_options[CompressSegmentBy].is_default == false)
@@ -255,13 +313,13 @@
 		return parse_segment_collist(TextDatumGetCString(textarg), hypertable);
 	}
 	else
-		return NIL;
+		return NULL;
 }
 
 /* returns List of CompressedParsedCol
  * E.g. timescaledb.compress_orderby = 'col1 asc nulls first,col2 desc,col3'
  */
-List *
+OrderBySettings
 ts_compress_hypertable_parse_order_by(WithClauseResult *parsed_options, Hypertable *hypertable)
 {
 	if (parsed_options[CompressOrderBy].is_default == false)
@@ -270,7 +328,7 @@
 		return parse_order_collist(TextDatumGetCString(textarg), hypertable);
 	}
 	else
-		return NIL;
+		return (OrderBySettings){ 0 };
 }
 
 /* returns List of CompressedParsedCol

src/compression_with_clause.h

@@ -29,11 +29,18 @@ typedef struct
 	bool desc;
 } CompressedParsedCol;
 
+typedef struct
+{
+	ArrayType *orderby;
+	ArrayType *orderby_desc;
+	ArrayType *orderby_nullsfirst;
+} OrderBySettings;
+
 extern TSDLLEXPORT WithClauseResult *ts_compress_hypertable_set_clause_parse(const List *defelems);
-extern TSDLLEXPORT List *ts_compress_hypertable_parse_segment_by(WithClauseResult *parsed_options,
-																 Hypertable *hypertable);
-extern TSDLLEXPORT List *ts_compress_hypertable_parse_order_by(WithClauseResult *parsed_options,
-															   Hypertable *hypertable);
+extern TSDLLEXPORT ArrayType *
+ts_compress_hypertable_parse_segment_by(WithClauseResult *parsed_options, Hypertable *hypertable);
+extern TSDLLEXPORT OrderBySettings
+ts_compress_hypertable_parse_order_by(WithClauseResult *parsed_options, Hypertable *hypertable);
 extern TSDLLEXPORT Interval *
 ts_compress_hypertable_parse_chunk_time_interval(WithClauseResult *parsed_options,
 												 Hypertable *hypertable);

src/hypertable.c

@@ -2214,7 +2214,6 @@ bool
 ts_hypertable_set_compress_interval(Hypertable *ht, int64 compress_interval)
 {
 	Assert(!TS_HYPERTABLE_IS_INTERNAL_COMPRESSION_TABLE(ht));
-	Assert(TS_HYPERTABLE_HAS_COMPRESSION_ENABLED(ht));
 
 	Dimension *time_dimension =
 		ts_hyperspace_get_mutable_dimension(ht->space, DIMENSION_TYPE_OPEN, 0);

src/process_utility.c

@@ -3969,26 +3969,18 @@ process_altertable_set_options(AlterTableCmd *cmd, Hypertable *ht)
 	Assert(IsA(cmd->def, List));
 	inpdef = (List *) cmd->def;
 	ts_with_clause_filter(inpdef, &compress_options, &pg_options);
-	if (compress_options)
-	{
-		parse_results = ts_compress_hypertable_set_clause_parse(compress_options);
-		/* We allow updating compress chunk time interval independently of other compression
-		 * options. */
-		if (parse_results[CompressEnabled].is_default &&
-			parse_results[CompressChunkTimeInterval].is_default)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("the option timescaledb.compress must be set to true to enable "
-							"compression")));
-	}
-	else
+
+	if (!compress_options)
 		return DDL_CONTINUE;
 
 	if (pg_options != NIL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("only timescaledb.compress parameters allowed when specifying compression "
 						"parameters for hypertable")));
+
+	parse_results = ts_compress_hypertable_set_clause_parse(compress_options);
+
 	ts_cm_functions->process_compress_table(cmd, ht, parse_results);
 	return DDL_DONE;
 }

src/ts_catalog/compression_settings.c

@@ -207,6 +207,24 @@ ts_compression_settings_update(CompressionSettings *settings)
 	FormData_compression_settings *fd = &settings->fd;
 	ScanKeyData scankey[1];
 
+	if (settings->fd.orderby && settings->fd.segmentby)
+	{
+		Datum datum;
+		bool isnull;
+
+		ArrayIterator it = array_create_iterator(settings->fd.orderby, 0, NULL);
+		while (array_iterate(it, &datum, &isnull))
+		{
+			if (ts_array_is_member(settings->fd.segmentby, TextDatumGetCString(datum)))
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("cannot use column \"%s\" for both ordering and segmenting",
+								TextDatumGetCString(datum)),
+						 errhint("Use separate columns for the timescaledb.compress_orderby and"
+								 " timescaledb.compress_segmentby options.")));
+		}
+	}
+
 	/*
 	 * The default compression settings will always have orderby settings but the user may have
 	 * chosen to overwrite it. For both cases all 3 orderby arrays must either have the same number

tsl/src/compression/api.c

@@ -576,6 +576,11 @@ decompress_chunk_impl(Oid uncompressed_hypertable_relid, Oid uncompressed_chunk_
 
 	ts_hypertable_permissions_check(uncompressed_hypertable->main_table_relid, GetUserId());
 
+	if (TS_HYPERTABLE_IS_INTERNAL_COMPRESSION_TABLE(uncompressed_hypertable))
+		ereport(ERROR,
+				(errcode(ERRCODE_INTERNAL_ERROR),
+				 errmsg("decompress_chunk must not be called on the internal compressed chunk")));
+
 	compressed_hypertable =
 		ts_hypertable_get_by_id(uncompressed_hypertable->fd.compressed_hypertable_id);
 	if (compressed_hypertable == NULL)

tsl/src/compression/compression_storage.c

@@ -66,6 +66,8 @@ compression_hypertable_create(Hypertable *ht, Oid owner, Oid tablespace_oid)
 	RangeVar *compress_rel;
 	int32 compress_hypertable_id;
 
+	Assert(!TS_HYPERTABLE_HAS_COMPRESSION_TABLE(ht));
+
 	create = makeNode(CreateStmt);
 	create->tableElts = NIL;
 	create->inhRelations = NIL;