add modify grammar, consider partition table, visible coulumns should be more than one

YinZheng-Sun · YinZheng-Sun · commit c86bcd041b82 · 2023-09-06T10:29:14.000+08:00
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
@@ -162,7 +162,6 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
 		ColumnDef  *coldef = makeNode(ColumnDef);
 
 		coldef->inhcount = 0;
-		coldef->is_invisible = false;
 		coldef->is_local = true;
 		coldef->is_not_null = true;
 		coldef->is_from_type = false;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
@@ -382,8 +382,10 @@ static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, LOCKMO
 static void ATPrepSetNotNull(Relation rel, bool recurse, bool recursing);
 static ObjectAddress ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
 				 const char *colName, LOCKMODE lockmode);
+static void ATPrepSetVisible(Relation rel, bool recurse, bool recursing);
 static ObjectAddress ATExecSetVisible(AlteredTableInfo *tab, Relation rel,
 				 const char *colName, LOCKMODE lockmode);
+static void ATPrepSetInvisible(Relation rel, bool recurse, bool recursing);
 static ObjectAddress ATExecSetInvisible(AlteredTableInfo *tab, Relation rel,
 				 const char *colName, LOCKMODE lockmode);
 static ObjectAddress ATExecColumnDefault(Relation rel, const char *colName,
@@ -560,6 +562,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
 	Oid			ofTypeId;
 	ObjectAddress address;
+	int 		invisibleColumnCount;
 
 	/*
 	 * Truncate relname to appropriate length (probably a waste of time, as
@@ -744,6 +747,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	rawDefaults = NIL;
 	cookedDefaults = NIL;
 	attnum = 0;
+	invisibleColumnCount = 0;
 
 	foreach(listptr, stmt->tableElts)
 	{
@@ -788,9 +792,20 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 			attr->attidentity = colDef->identity;
 
 		if (colDef->is_invisible)
+		{
 			attr->attisinvisible = colDef->is_invisible;
+			invisibleColumnCount++;
+		}
 	}
 
+	/*
+	 * table must have at least one column that is not invisible
+	 */
+	if (invisibleColumnCount == attnum)
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("table must have at least one column that is not invisible")));
+
 	/*
 	 * Create the relation.  Inherited defaults and constraints are passed in
 	 * for immediate handling --- since they don't need parsing, they can be
@@ -2003,6 +2018,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 					 * merge the column options into the column from the type
 					 */
 					coldef->is_not_null = restdef->is_not_null;
+					coldef->is_invisible = restdef->is_invisible;
 					coldef->raw_default = restdef->raw_default;
 					coldef->cooked_default = restdef->cooked_default;
 					coldef->constraints = restdef->constraints;
@@ -2246,6 +2262,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 				def->inhcount = 1;
 				def->is_local = false;
 				def->is_not_null = attribute->attnotnull;
+				def->is_invisible = attribute->attisinvisible;
 				def->is_from_type = false;
 				def->storage = attribute->attstorage;
 				def->raw_default = NULL;
@@ -3843,12 +3860,14 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			break;
 		case AT_SetVisible:	/* ALTER COLUMN SET VISIBLE */
 			ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+			ATPrepSetVisible(rel, recurse, recursing);
 			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
 			/* No command-specific prep needed */
 			pass = AT_PASS_ADD_CONSTR;
 			break;
 		case AT_SetInvisible:	/* ALTER COLUMN SET INVISIBLE */
 			ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+			ATPrepSetInvisible(rel, recurse, recursing);
 			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
 			/* No command-specific prep needed */
 			pass = AT_PASS_ADD_CONSTR;
@@ -6272,6 +6291,25 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
 	return address;
 }
 
+static void
+ATPrepSetVisible(Relation rel, bool recurse, bool recursing)
+{
+	/*
+	 * If the parent is a partitioned table, Set Visible
+	 * constraints must be added to the child tables.
+	 */
+	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	{
+		PartitionDesc partdesc = RelationGetPartitionDesc(rel);
+
+		if (partdesc && partdesc->nparts > 0 && !recurse && !recursing)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+					 errmsg("cannot add constraint to only the partitioned table when partitions exist"),
+					 errhint("Do not specify the ONLY keyword.")));
+	}
+}
+
 /*
  * Return the address of the modified column.  If the column was already Invisible,
  * InvalidObjectAddress is returned.
@@ -6328,6 +6366,25 @@ ATExecSetVisible(AlteredTableInfo *tab, Relation rel,
 	return address;
 }
 
+static void
+ATPrepSetInvisible(Relation rel, bool recurse, bool recursing)
+{
+	/*
+	 * If the parent is a partitioned table, Set Invisible
+	 * constraints must be added to the child tables.
+	 */
+	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	{
+		PartitionDesc partdesc = RelationGetPartitionDesc(rel);
+
+		if (partdesc && partdesc->nparts > 0 && !recurse && !recursing)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+					 errmsg("cannot add constraint to only the partitioned table when partitions exist"),
+					 errhint("Do not specify the ONLY keyword.")));
+	}
+}
+
 /*
  * Return the address of the modified column.  If the column was already Invisible,
  * InvalidObjectAddress is returned.
@@ -6365,6 +6422,30 @@ ATExecSetInvisible(AlteredTableInfo *tab, Relation rel,
 
 	if (!((Form_pg_attribute) GETSTRUCT(tuple))->attisinvisible)
 	{
+		TupleDesc tupleDesc = RelationGetDescr(rel);
+		int numattrs = tupleDesc->natts;
+		int i;
+		bool visible_only = true;
+		Form_pg_attribute attr;
+
+		for (i = 0; i < numattrs; ++i) {
+			if (i == attnum - 1) {
+				continue;
+			}
+
+			attr = TupleDescAttr(tupleDesc, i);
+
+			if (!attr->attisinvisible) {
+				visible_only = false;
+				break;
+			}
+		}
+
+		if (visible_only)
+			ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("table must have at least one column that is not invisible")));
+		
 		((Form_pg_attribute) GETSTRUCT(tuple))->attisinvisible = true;
 
 		CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
@@ -666,7 +666,7 @@ static bool polar_is_ignore_user_defined_tablespace(char *tablespace_name);
 	LEADER LEADING LEAKPROOF LEARNER LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
 	LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P LOCKED LOGGED LOGS
 
-	MAPPING MATCH MATERIALIZED MAXVALUE METHOD MINUTE_P MINVALUE MODE MONTH_P MOVE
+	MAPPING MATCH MATERIALIZED MAXVALUE METHOD MINUTE_P MINVALUE MODE MODIFY MONTH_P MOVE
 
 	NAME_P NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NODE NONE
 	NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF
@@ -2201,6 +2201,22 @@ alter_table_cmd:
 					n->name = $3;
 					$$ = (Node *)n;
 				}
+			/* ALTER TABLE <name> MODIFY <colname> VISIBLE */
+			| MODIFY opt_column ColId VISIBLE 
+				{
+					AlterTableCmd *n = makeNode(AlterTableCmd);
+					n->subtype = AT_SetVisible;
+					n->name = $3;
+					$$ = (Node *)n;
+				}
+			/* ALTER TABLE <name> MODIFY <colname> INVISIBLE */
+			| MODIFY opt_column ColId INVISIBLE
+				{
+					AlterTableCmd *n = makeNode(AlterTableCmd);
+					n->subtype = AT_SetInvisible;
+					n->name = $3;
+					$$ = (Node *)n;
+				}
 			/* ALTER TABLE <name> ALTER [COLUMN] <colname> RESET ( column_parameter = value [, ... ] ) */
 			| ALTER opt_column ColId RESET reloptions
 				{
@@ -3445,7 +3461,6 @@ columnDef:	ColId Typename create_generic_options ColQualList
 					n->colname = $1;
 					n->typeName = $2;
 					n->inhcount = 0;
-					n->is_invisible = false;
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
@@ -3467,7 +3482,6 @@ columnOptions:	ColId ColQualList
 					n->colname = $1;
 					n->typeName = NULL;
 					n->inhcount = 0;
-					n->is_invisible = false;
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
@@ -3486,7 +3500,6 @@ columnOptions:	ColId ColQualList
 					n->colname = $1;
 					n->typeName = NULL;
 					n->inhcount = 0;
-					n->is_invisible = false;
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
@@ -3547,7 +3560,14 @@ ColConstraint:
  * or be part of a_expr NOT LIKE or similar constructs).
  */
 ColConstraintElem:
-			NOT NULL_P
+		    INVISIBLE
+				{
+					Constraint *n = makeNode(Constraint);
+					n->contype = CONSTR_INVISIBLE;
+					n->location = @1;
+					$$ = (Node *)n;
+				}
+			| NOT NULL_P
 				{
 					Constraint *n = makeNode(Constraint);
 					n->contype = CONSTR_NOTNULL;
@@ -3628,13 +3648,6 @@ ColConstraintElem:
 					n->initially_valid  = true;
 					$$ = (Node *)n;
 				}
-			| INVISIBLE
-				{
-					Constraint *n = makeNode(Constraint);
-					n->contype = CONSTR_INVISIBLE;
-					n->location = @1;
-					$$ = (Node *)n;
-				}
 		;
 
 generated_when:
@@ -15545,6 +15558,7 @@ unreserved_keyword:
 			| MINUTE_P
 			| MINVALUE
 			| MODE
+			| MODIFY
 			| MONTH_P
 			| MOVE
 			| NAME_P
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
@@ -261,6 +261,7 @@ PG_KEYWORD("method", METHOD, UNRESERVED_KEYWORD)
 PG_KEYWORD("minute", MINUTE_P, UNRESERVED_KEYWORD)
 PG_KEYWORD("minvalue", MINVALUE, UNRESERVED_KEYWORD)
 PG_KEYWORD("mode", MODE, UNRESERVED_KEYWORD)
+PG_KEYWORD("modify", MODIFY, UNRESERVED_KEYWORD)
 PG_KEYWORD("month", MONTH_P, UNRESERVED_KEYWORD)
 PG_KEYWORD("move", MOVE, UNRESERVED_KEYWORD)
 PG_KEYWORD("name", NAME_P, UNRESERVED_KEYWORD)
diff --git a/src/test/regress/expected/invisible.out b/src/test/regress/expected/invisible.out
@@ -14,6 +14,10 @@ CREATE TABLE TEST_INVISIBLE_TBL3(
     a INT INVISIBLE,
     b INT
 );
+create TABLE TEST_INVISIBLE_TBL4(
+    a int INVISIBLE
+);
+ERROR:  table must have at least one column that is not invisible
 INSERT INTO TEST_INVISIBLE_TBL1 VALUES (1, 1, 'A');
 ERROR:  INSERT has more expressions than target columns
 LINE 1: INSERT INTO TEST_INVISIBLE_TBL1 VALUES (1, 1, 'A');
@@ -85,7 +89,9 @@ SELECT * FROM TEST_INVISIBLE_TBL1;
  B
 (2 rows)
 
-ALTER TABLE TEST_INVISIBLE_TBL1 ALTER COLUMN a SET VISIBLE;
+ALTER TABLE TEST_INVISIBLE_TBL1 ALTER COLUMN a SET INVISIBLE;
+ERROR:  table must have at least one column that is not invisible
+ALTER TABLE TEST_INVISIBLE_TBL1 MODIFY a SET VISIBLE;
 SELECT * FROM TEST_INVISIBLE_TBL1;
  a | c 
 ---+---
diff --git a/src/test/regress/expected/invisible.out.replica b/src/test/regress/expected/invisible.out.replica
@@ -16,7 +16,11 @@ CREATE TABLE TEST_INVISIBLE_TBL3(
     a INT INVISIBLE,
     b INT
 );
-ERROR:  cannot execute INSERT in a read-only transaction
+ERROR:  cannot execute CREATE TABLE in a read-only transaction
+create TABLE TEST_INVISIBLE_TBL4(
+    a int INVISIBLE
+);
+ERROR:  cannot execute CREATE TABLE in a read-only transaction
 INSERT INTO TEST_INVISIBLE_TBL1 VALUES (1, 1, 'A');
 ERROR:  cannot execute INSERT in a read-only transaction
 INSERT INTO TEST_INVISIBLE_TBL1 VALUES (1, 'A');
@@ -88,7 +92,9 @@ SELECT * FROM TEST_INVISIBLE_TBL1;
  B
 (2 rows)
 
-ALTER TABLE TEST_INVISIBLE_TBL1 ALTER COLUMN a SET VISIBLE;
+ALTER TABLE TEST_INVISIBLE_TBL1 ALTER COLUMN a SET INVISIBLE;
+ERROR:  cannot execute ALTER TABLE in a read-only transaction
+ALTER TABLE TEST_INVISIBLE_TBL1 MODIFY a SET VISIBLE;
 ERROR:  cannot execute ALTER TABLE in a read-only transaction
 SELECT * FROM TEST_INVISIBLE_TBL1;
  a | c 
diff --git a/src/test/regress/sql/invisible.sql b/src/test/regress/sql/invisible.sql
@@ -18,6 +18,10 @@ CREATE TABLE TEST_INVISIBLE_TBL3(
     b INT
 );
 
+create TABLE TEST_INVISIBLE_TBL4(
+    a int INVISIBLE
+);
+
 
 INSERT INTO TEST_INVISIBLE_TBL1 VALUES (1, 1, 'A');
 INSERT INTO TEST_INVISIBLE_TBL1 VALUES (1, 'A');
@@ -47,7 +51,9 @@ SELECT a, b  FROM TEST_INVISIBLE_TBL3;
 ALTER TABLE TEST_INVISIBLE_TBL1 ALTER COLUMN b SET INVISIBLE;
 SELECT * FROM TEST_INVISIBLE_TBL1;
 
-ALTER TABLE TEST_INVISIBLE_TBL1 ALTER COLUMN a SET VISIBLE;
+ALTER TABLE TEST_INVISIBLE_TBL1 ALTER COLUMN a SET INVISIBLE;
+
+ALTER TABLE TEST_INVISIBLE_TBL1 MODIFY a VISIBLE;
 SELECT * FROM TEST_INVISIBLE_TBL1;
 
 DROP TABLE TEST_INVISIBLE_TBL1;
