[BACKPORT 2024.1][#22874] YSQL: Fix cascaded drops on columns

Summary:
Commit 7c8343d / D35866 summary:

Currently, when an object that a column depends on is dropped, the drop isn't
correctly cascaded to the column in the DocDB table itself.

Example:

```
yugabyte=# CREATE TABLE test_dropcolumn(a int, b int, c int);
CREATE TABLE
yugabyte=# CREATE TYPE test_dropcolumn_type AS (a int, b int);
CREATE TYPE
yugabyte=# ALTER TABLE test_dropcolumn ADD COLUMN d test_dropcolumn_type;
ALTER TABLE
yugabyte=# DROP TYPE test_dropcolumn_type CASCADE;
NOTICE:  drop cascades to column t of table person
DROP TYPE
yugabyte=# ALTER TABLE test_dropcolumn ADD COLUMN d int;
ERROR:  The column already exists: d
```
Although the PG metadata for the column is dropped, the column itself is not
dropped in DocDB. This leads to an inconsistency between DocDB and PG.

This diff fixes the issue by adding a YB drop for the column inside `doDeletion`.
Note: this function is also used during `ALTER TABLE... DROP COLUMN`; however,
alter table commands execute YB schema changes separately (see `YBCPrepareAlterTableCmd`
and `YBCExecAlterTable`). So, an additional flag is added to skip the YB column
drop in `doDeletion` in case it is being invoked by the alter table flow.
Jira: DB-11776

Commit b81dbaa / D36329 summary:

The test became flaky after commit 7c8343d fixed the logic for cascaded drops on columns.

The test yb_feature_hash_types creates a domain and creates a table with a key column that has the domain as its type. However, since we currently don't support key column drops (#22902), the test fails when the cleanUpCustomEntities() function in BasePgSQLTest.java attempts to drop the domain (that has the key column as a dependant object) during cleanup.
Moreover, the test is flaky because sometimes the table may be dropped before the domain, as the order of the drops depends on the pg_depend scan output, which isn't deterministic because pg_depend doesn't have a primary key and internally uses ybctid.

Fix the test flakiness by dropping the table in yb_feature_hash_types before the test clean up logic.
Jira: DB-12029

Original commits:
7c8343d / D35866
b81dbaa / D36329

Test Plan: yb_alter_table

Reviewers: myang

Reviewed By: myang

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36286

src/postgres/src/backend/catalog/dependency.c

@@ -1162,8 +1162,19 @@ doDeletion(const ObjectAddress *object, int flags)
 				else
 				{
 					if (object->objectSubId != 0)
+					{
+						Relation yb_rel =
+							RelationIdGetRelation(object->objectId);
+
+						if (IsYBRelation(yb_rel) &&
+							!(flags & YB_SKIP_YB_DROP_COLUMN))
+							YBCDropColumn(yb_rel, object->objectSubId);
+
+						RelationClose(yb_rel);
+
 						RemoveAttributeById(object->objectId,
 											object->objectSubId);
+					}
 					else
 					{
 						Relation rel = RelationIdGetRelation(object->objectId);

src/postgres/src/backend/commands/tablecmds.c

@@ -7506,7 +7506,12 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
 	object.objectId = RelationGetRelid(rel);
 	object.objectSubId = attnum;
 
-	performDeletion(&object, behavior, 0);
+	/*
+	 * YB: Skip YB drop on the column, as that will be handled separately by
+	 * the ALTER TABLE flow.
+	 */
+	performDeletion(&object, behavior,
+		IsYugaByteEnabled() ? YB_SKIP_YB_DROP_COLUMN : 0);
 
 	/*
 	 * If we dropped the OID column, must adjust pg_class.relhasoids and tell

src/postgres/src/backend/commands/ybccmds.c

@@ -1971,3 +1971,19 @@ YBCUpdateAndPersistLSN(const char *stream_id, XLogRecPtr restart_lsn_hint,
 	HandleYBStatus(YBCPgUpdateAndPersistLSN(stream_id, restart_lsn_hint,
 											confirmed_flush, restart_lsn));
 }
+
+void
+YBCDropColumn(Relation rel, AttrNumber attnum)
+{
+	TupleDesc tupleDesc = RelationGetDescr(rel);
+	Form_pg_attribute attr = TupleDescAttr(tupleDesc, attnum - 1);
+	YBCPgStatement handle = NULL;
+	HandleYBStatus(YBCPgNewAlterTable(
+		YBCGetDatabaseOidByRelid(RelationGetRelid(rel)),
+		YbGetRelfileNodeId(rel),
+		&handle));
+	HandleYBStatus(YBCPgAlterTableDropColumn(
+		handle,
+		attr->attname.data));
+	HandleYBStatus(YBCPgExecAlterTable(handle));
+}

src/postgres/src/include/catalog/dependency.h

@@ -208,6 +208,8 @@ typedef enum ObjectClass
 #define PERFORM_DELETION_SKIP_ORIGINAL		0x0008	/* keep original obj */
 #define PERFORM_DELETION_SKIP_EXTENSIONS	0x0010	/* keep extensions */
 
+/* skip yb drop on a column -- used during ALTER TABLE */
+#define YB_SKIP_YB_DROP_COLUMN				0x8000
 
 /* in dependency.c */
 

src/postgres/src/include/commands/ybccmds.h

@@ -143,3 +143,5 @@ extern void YBCUpdateAndPersistLSN(const char *stream_id,
 								   XLogRecPtr restart_lsn_hint,
 								   XLogRecPtr confirmed_flush,
 								   YBCPgXLogRecPtr *restart_lsn);
+
+extern void YBCDropColumn(Relation rel, AttrNumber attnum);

src/postgres/src/test/regress/expected/yb_alter_table.out

@@ -412,3 +412,12 @@ ALTER TABLE foo ADD COLUMN f FLOAT DEFAULT random();
 NOTICE:  table rewrite may lead to inconsistencies
 DETAIL:  Concurrent DMLs may not be reflected in the new table.
 HINT:  See https://github.com/yugabyte/yugabyte-db/issues/19860. Set 'ysql_suppress_unsafe_alter_notice' yb-tserver gflag to true to suppress this notice.
+--
+-- Test for cascaded drops on columns
+--
+CREATE TABLE test_dropcolumn(a int, b int, c int);
+CREATE TYPE test_dropcolumn_type AS (a int, b int);
+ALTER TABLE test_dropcolumn ADD COLUMN d test_dropcolumn_type;
+DROP TYPE test_dropcolumn_type CASCADE; -- should drop the column d
+NOTICE:  drop cascades to column d of table test_dropcolumn
+ALTER TABLE test_dropcolumn ADD COLUMN d int;

src/postgres/src/test/regress/expected/yb_alter_type.out

@@ -132,6 +132,13 @@ DROP TYPE new_name;  -- fail
 ERROR:  cannot drop type new_name because other objects depend on it
 DETAIL:  column happiness_level of table holidays depends on type new_name
 HINT:  Use DROP ... CASCADE to drop the dependent objects too.
+DROP TYPE new_name CASCADE; -- fail (happiness_level is a key column)
+NOTICE:  drop cascades to column happiness_level of table holidays
+ERROR:  cannot remove a key column
+ALTER TABLE holidays DROP CONSTRAINT holidays_pkey;
+NOTICE:  table rewrite may lead to inconsistencies
+DETAIL:  Concurrent DMLs may not be reflected in the new table.
+HINT:  See https://github.com/yugabyte/yugabyte-db/issues/19860. Set 'ysql_suppress_unsafe_alter_notice' yb-tserver gflag to true to suppress this notice.
 DROP TYPE new_name CASCADE;
 NOTICE:  drop cascades to column happiness_level of table holidays
 \d holidays

src/postgres/src/test/regress/expected/yb_feature_hash_types.out

@@ -46,6 +46,10 @@ CREATE TABLE ft_h_tab_bool (feature_col BOOLEAN PRIMARY KEY);
 -- Enumerated Types
 CREATE TYPE feature_h_enum AS ENUM('one', 'two', 'three');
 CREATE TABLE ft_h_tab_enum (feature_col feature_h_enum PRIMARY KEY);
+-- YB(fizaa): Work-around for the test cleanup code that drops the enum type.
+-- Dropping the enum type will cause an error because a key column depends on it (GH #22902).
+-- By dropping the table now, the clean up code will succeed.
+DROP TABLE ft_h_tab_enum;
 --
 -- Geometric Types
 CREATE TABLE ft_h_tab_point (feature_col POINT PRIMARY KEY);
@@ -115,6 +119,10 @@ ERROR:  PRIMARY KEY containing column of type 'user_defined_type' not yet suppor
 -- Domain Types
 CREATE DOMAIN feature_h_domain AS INTEGER CHECK (VALUE > 0);
 CREATE TABLE ft_h_tab_domain (feature_col feature_h_domain PRIMARY KEY);
+-- YB(fizaa): Work-around for the test cleanup code that drops the domain.
+-- Dropping the domain will cause an error because a key column depends on it (GH #22902).
+-- By dropping the table now, the clean up code will succeed.
+DROP TABLE ft_h_tab_domain;
 --
 -- Object Identifier Types
 CREATE TABLE ft_h_tab_oid (feature_col OID PRIMARY KEY);

src/postgres/src/test/regress/expected/yb_feature_range_types.out

@@ -68,6 +68,10 @@ CREATE TABLE ft_r_tab_bool (feature_col BOOLEAN, PRIMARY KEY(feature_col ASC))
 -- Enumerated Types
 CREATE TYPE feature_r_enum AS ENUM('one', 'two', 'three');
 CREATE TABLE ft_r_tab_enum (feature_col feature_r_enum, PRIMARY KEY(feature_col ASC));
+-- YB(fizaa): Work-around for the test cleanup code that drops the enum type.
+-- Dropping the enum type will cause an error because a key column depends on it (GH #22902).
+-- By dropping the table now, the clean up code will succeed.
+DROP TABLE ft_r_tab_enum;
 --
 -- Geometric Types
 CREATE TABLE ft_r_tab_point (feature_col POINT, PRIMARY KEY(feature_col ASC));

src/postgres/src/test/regress/sql/yb_alter_table.sql

@@ -160,3 +160,11 @@ SELECT * FROM foo_part ORDER BY a, b;
 ALTER TABLE foo ADD COLUMN g int DEFAULT null NOT NULL;
 -- Test add column with volatile default value.
 ALTER TABLE foo ADD COLUMN f FLOAT DEFAULT random();
+--
+-- Test for cascaded drops on columns
+--
+CREATE TABLE test_dropcolumn(a int, b int, c int);
+CREATE TYPE test_dropcolumn_type AS (a int, b int);
+ALTER TABLE test_dropcolumn ADD COLUMN d test_dropcolumn_type;
+DROP TYPE test_dropcolumn_type CASCADE; -- should drop the column d
+ALTER TABLE test_dropcolumn ADD COLUMN d int;

src/postgres/src/test/regress/sql/yb_alter_type.sql

@@ -56,6 +56,8 @@ INSERT INTO holidays(num_weeks,happiness_level) VALUES (8, 'ecstatic');
 SELECT * FROM holidays ORDER BY num_weeks;
 
 DROP TYPE new_name;  -- fail
+DROP TYPE new_name CASCADE; -- fail (happiness_level is a key column)
+ALTER TABLE holidays DROP CONSTRAINT holidays_pkey;
 DROP TYPE new_name CASCADE;
 
 \d holidays

src/postgres/src/test/regress/sql/yb_feature_hash_types.sql

@@ -43,6 +43,10 @@ CREATE TABLE ft_h_tab_bool (feature_col BOOLEAN PRIMARY KEY);
 -- Enumerated Types
 CREATE TYPE feature_h_enum AS ENUM('one', 'two', 'three');
 CREATE TABLE ft_h_tab_enum (feature_col feature_h_enum PRIMARY KEY);
+-- YB(fizaa): Work-around for the test cleanup code that drops the enum type.
+-- Dropping the enum type will cause an error because a key column depends on it (GH #22902).
+-- By dropping the table now, the clean up code will succeed.
+DROP TABLE ft_h_tab_enum;
 --
 -- Geometric Types
 CREATE TABLE ft_h_tab_point (feature_col POINT PRIMARY KEY);
@@ -91,6 +95,10 @@ CREATE TABLE ft_h_tab_range (feature_col feature_h_range PRIMARY KEY);
 -- Domain Types
 CREATE DOMAIN feature_h_domain AS INTEGER CHECK (VALUE > 0);
 CREATE TABLE ft_h_tab_domain (feature_col feature_h_domain PRIMARY KEY);
+-- YB(fizaa): Work-around for the test cleanup code that drops the domain.
+-- Dropping the domain will cause an error because a key column depends on it (GH #22902).
+-- By dropping the table now, the clean up code will succeed.
+DROP TABLE ft_h_tab_domain;
 --
 -- Object Identifier Types
 CREATE TABLE ft_h_tab_oid (feature_col OID PRIMARY KEY);

src/postgres/src/test/regress/sql/yb_feature_range_types.sql

@@ -65,6 +65,10 @@ CREATE TABLE ft_r_tab_bool (feature_col BOOLEAN, PRIMARY KEY(feature_col ASC))
 -- Enumerated Types
 CREATE TYPE feature_r_enum AS ENUM('one', 'two', 'three');
 CREATE TABLE ft_r_tab_enum (feature_col feature_r_enum, PRIMARY KEY(feature_col ASC));
+-- YB(fizaa): Work-around for the test cleanup code that drops the enum type.
+-- Dropping the enum type will cause an error because a key column depends on it (GH #22902).
+-- By dropping the table now, the clean up code will succeed.
+DROP TABLE ft_r_tab_enum;
 --
 -- Geometric Types
 CREATE TABLE ft_r_tab_point (feature_col POINT, PRIMARY KEY(feature_col ASC));