[#24981] YSQL: Fix schema version mismatch error after failed ALTER T…

…ABLE

Summary:
In some specific cases, a failed ALTER TABLE operation does not adequately invalidate the table cache, causing a "schema mismatch" error in the subsequent command. Consider the following example:

```
CREATE TABLE pk(a int primary key);
INSERT INTO pk values (1);
CREATE TABLE fk(a int);
INSERT INTO fk values (2);
ALTER TABLE fk ADD FOREIGN KEY (a) REFERENCES pk; -- fails due to FK constraint violation
BEGIN;
SELECT * from pk; -- throws schema version mismatch error
COMMIT;
```

Before the start of ALTER TABLE, the schema version of both fk and pk is zero.

The above ALTER TABLE does the following:
1. Increments schema version of both relations to 1 (YBCPrepareAlterTableCmd() increments schema version of both - the main relation and dependent relations).
2. Invalidates any pre-existing table schema of the two relations (ATRewriteCatalogs()).
3. Loads schema of both the relations (version 1) to check for FK violation, which it finds to be there (ATRewriteTables()).
4. ybAlteredTableIds contains only the oid corresponding to fk. Hence, only fk's table cache is invalidated during error recovery. This is where the bug is (explained below).
5. DDL txn verification on master increments the schema version of both the relations to 2 (see YsqlDdlTxnAlterTableHelper()).

Because of step #4, the table cache still contains the stale entry of pk (corresponding to version 1). The subsequent SELECT operation ends up using it. This leads to the "schema version mismatch, expected 2, got 1" error.

Resolution:
On a failure, in step #4, invalidate the table cache entries of all the relations whose schema is incremented at the start (step #1) and then at the end by YBResetDdlState() (step #5). This is done by including the oids of the dependent relations in `ybAlteredTableIds`.
Jira: DB-14126

Test Plan:
   ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressTable'

Reviewers: fizaa

Reviewed By: fizaa

Subscribers: yql

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

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

@@ -5220,9 +5220,8 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
 										 AT_NUM_PASSES,
 										 main_relid,
 										 &rollbackHandle,
-										 false /* isPartitionOfAlteredTable */);
-	if (handles)
-		*ybAlteredTableIds = lappend_oid(*ybAlteredTableIds, main_relid);
+										 false /* isPartitionOfAlteredTable */,
+										 ybAlteredTableIds);
 	if (rollbackHandle)
 		*rollbackHandles = lappend(*rollbackHandles, rollbackHandle);
 
@@ -5246,9 +5245,8 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
 												   AT_NUM_PASSES,
 												   childrelid,
 												   &childRollbackHandle,
-												   true /*isPartitionOfAlteredTable */);
-		if (child_handles)
-			*ybAlteredTableIds = lappend_oid(*ybAlteredTableIds, childrelid);
+												   true /*isPartitionOfAlteredTable */,
+												   ybAlteredTableIds);
 		ListCell *listcell = NULL;
 		foreach(listcell, child_handles)
 		{

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

@@ -1215,7 +1215,8 @@ static List*
 YBCPrepareAlterTableCmd(AlterTableCmd* cmd, Relation rel, List *handles,
 						int* col, bool* needsYBAlter,
 						YBCPgStatement* rollbackHandle,
-						bool isPartitionOfAlteredTable)
+						bool isPartitionOfAlteredTable,
+						List *volatile *ybAlteredTableIds)
 {
 	Oid relationId = RelationGetRelid(rel);
 	Oid relfileNodeId = YbGetRelfileNodeId(rel);
@@ -1609,6 +1610,7 @@ YBCPrepareAlterTableCmd(AlterTableCmd* cmd, Relation rel, List *handles,
 				HandleYBStatus(
 					YBCPgAlterTableIncrementSchemaVersion(alter_cmd_handle));
 				handles = lappend(handles, alter_cmd_handle);
+				*ybAlteredTableIds = lappend_oid(*ybAlteredTableIds, relationId);
 				table_close(dependent_rel, AccessExclusiveLock);
 			}
 			*needsYBAlter = true;
@@ -1665,7 +1667,8 @@ YBCPrepareAlterTable(List** subcmds,
 					 int subcmds_size,
 					 Oid relationId,
 					 YBCPgStatement *rollbackHandle,
-					 bool isPartitionOfAlteredTable)
+					 bool isPartitionOfAlteredTable,
+					 List *volatile *ybAlteredTableIds)
 {
 	/* Appropriate lock was already taken */
 	Relation rel = relation_open(relationId, NoLock);
@@ -1693,7 +1696,8 @@ YBCPrepareAlterTable(List** subcmds,
 			handles = YBCPrepareAlterTableCmd(
 						(AlterTableCmd *) lfirst(lcmd), rel, handles,
 						&col, &needsYBAlter, rollbackHandle,
-						isPartitionOfAlteredTable);
+						isPartitionOfAlteredTable,
+						ybAlteredTableIds);
 		}
 	}
 	relation_close(rel, NoLock);
@@ -1702,7 +1706,7 @@ YBCPrepareAlterTable(List** subcmds,
 	{
 		return NULL;
 	}
-
+	*ybAlteredTableIds = lappend_oid(*ybAlteredTableIds, relationId);
 	return handles;
 }
 

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

@@ -97,7 +97,8 @@ extern List* YBCPrepareAlterTable(List** subcmds,
 										   int subcmds_size,
 										   Oid relationId,
 										   YBCPgStatement *rollbackHandle,
-										   bool isPartitionOfAlteredTable);
+										   bool isPartitionOfAlteredTable,
+										   List *volatile *ybAlteredTableIds);
 
 extern void YBCExecAlterTable(YBCPgStatement handle, Oid relationId);
 

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

@@ -493,3 +493,19 @@ ERROR:  check constraint "test_validate_constraint_part_check" of relation "test
 DELETE FROM test_validate_constraint_part WHERE a % 2 = 1;
 ALTER TABLE test_validate_constraint_part
     VALIDATE CONSTRAINT test_validate_constraint_part_check;
+-- validate fix for "schema version mismatch" after failed ALTER TABLE. 
+CREATE TABLE pk(a int primary key);
+INSERT INTO pk values (1);
+CREATE TABLE fk(a int);
+INSERT INTO fk values (2);
+ALTER TABLE fk ADD FOREIGN KEY (a) REFERENCES pk; -- should fail due to FK constraint violation.
+ERROR:  insert or update on table "fk" violates foreign key constraint "fk_a_fkey"
+DETAIL:  Key (a)=(2) is not present in table "pk".
+BEGIN;
+SELECT * from pk;
+ a 
+---
+ 1
+(1 row)
+
+COMMIT;

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

@@ -222,3 +222,13 @@ ALTER TABLE test_validate_constraint_part
 DELETE FROM test_validate_constraint_part WHERE a % 2 = 1;
 ALTER TABLE test_validate_constraint_part
     VALIDATE CONSTRAINT test_validate_constraint_part_check;
+
+-- validate fix for "schema version mismatch" after failed ALTER TABLE.
+CREATE TABLE pk(a int primary key);
+INSERT INTO pk values (1);
+CREATE TABLE fk(a int);
+INSERT INTO fk values (2);
+ALTER TABLE fk ADD FOREIGN KEY (a) REFERENCES pk; -- should fail due to FK constraint violation.
+BEGIN;
+SELECT * from pk;
+COMMIT;