Add support for ALTER VIEW and CREATE OR ALTER VIEW syntax on schema bound views in Babelfish

This commit implements
- ALTER VIEW and CREATE OR ALTER VIEW syntax support
- Added createoralter flag to ViewStmt structure and grammar
- Added comprehensive test cases:
  * Basic alter view functionality
  * Schema bound views
  * Various test scenarios and edge cases
- Updated test files and expected outputs
- Fixed failing tests and naming conflicts
- Addressed review comments

Signed-off-by: Rahul Parande <rparande@amazon.com>

Author: Rahul Parande

contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-decl.y

@@ -14,7 +14,7 @@
 
 %type <node> tsql_stmt
 
-%type <node> tsql_AlterFunctionStmt tsql_CreatePartitionStmt
+%type <node> tsql_AlterFunctionStmt tsql_CreatePartitionStmt tsql_AlterViewStmt
 %type <node> tsql_CreateFunctionStmt tsql_VariableSetStmt tsql_CreateTrigStmt tsql_TransactionStmt tsql_UpdateStmt tsql_DeleteStmt tsql_IndexStmt
 %type <partspec> tsql_PartitionSpec
 %type <node> tsql_DropIndexStmt tsql_InsertStmt

contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-rule.y

@@ -2476,6 +2476,7 @@ tsql_stmt :
 			| AlterTSDictionaryStmt
 			| AlterUserMappingStmt
 			| tsql_AlterUserStmt
+			| tsql_AlterViewStmt
 			| AnalyzeStmt
 			| CallStmt
 			| CheckPointStmt
@@ -4110,6 +4111,35 @@ tsql_AlterFunctionStmt:
                 }
 		;
 
+tsql_AlterViewStmt: 
+            TSQL_ALTER VIEW qualified_name opt_column_list opt_reloptions
+                AS SelectStmt opt_check_option
+                {
+                    ViewStmt *n = makeNode(ViewStmt);
+                    n->view = $3;
+                    n->aliases = $4;
+                    n->query = $7;
+                    n->replace = true;
+                    n->options = $5;
+                    n->withCheckOption = $8;
+                    n->createOrAlter = true;
+                    $$ = (Node *) n;
+                }
+            | CREATE OR TSQL_ALTER VIEW qualified_name opt_column_list opt_reloptions
+                AS SelectStmt opt_check_option
+                {
+                    ViewStmt *n = makeNode(ViewStmt);
+                    n->view = $5;
+                    n->aliases = $6;
+                    n->query = $9;
+                    n->replace = false;
+                    n->options = $7;
+                    n->withCheckOption = $10;
+                    n->createOrAlter = true;
+                    $$ = (Node *) n;
+                }
+        ;
+		
 /*
  * These rules define the WITH clause in a CREATE PROCEDURE
  * or CREATE FUNCTION statement.  This is very similar to

contrib/babelfishpg_tsql/src/backend_parser/parser.c

@@ -321,6 +321,7 @@ pgtsql_base_yylex(YYSTYPE *lvalp, YYLTYPE * llocp, core_yyscan_t yyscanner)
 				case PROCEDURE:
 				case TSQL_PROC:
 				case FUNCTION:
+				case VIEW:
 					cur_token = TSQL_ALTER;
 					break;
 			}

contrib/babelfishpg_tsql/src/dbcmds.c

@@ -95,6 +95,7 @@ gen_createdb_subcmds(const char *dbname, const char *owner)
 	bool     	owner_is_sa_or_superuser = false;
 	const char     *db_datareader;
 	const char     *db_datawriter;
+	int old_dialect;
 
 	schema = get_dbo_schema_name(dbname);
 	dbo = get_dbo_role_name(dbname);
@@ -155,8 +156,15 @@ gen_createdb_subcmds(const char *dbname, const char *owner)
 	if (guest)
 		appendStringInfo(&query, "CREATE SCHEMA dummy AUTHORIZATION dummy; ");
 
+	old_dialect = sql_dialect;
+	sql_dialect = SQL_DIALECT_PG;
+	/* this query must be run in PG dialect or we will get a syntax error due to different
+	 * ALTER VIEW behavior between PG and TSQL
+	 */
 	res = raw_parser(query.data, RAW_PARSE_DEFAULT);
 
+	sql_dialect = old_dialect;
+
 	if (guest)
 	{
 		if (!owner_is_sa_or_superuser)

contrib/babelfishpg_tsql/src/pl_handler.c

@@ -39,6 +39,7 @@
 #include "commands/tablecmds.h"
 #include "commands/trigger.h"
 #include "commands/user.h"
+#include "commands/view.h"
 #include "common/md5.h"
 #include "common/string.h"
 #include "funcapi.h"
@@ -63,6 +64,7 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/guc_tables.h"
+#include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/plancache.h"
 #include "utils/ps_status.h"
@@ -142,6 +144,8 @@ static void call_prev_ProcessUtility(PlannedStmt *pstmt,
 						 QueryCompletion *qc);
 static void set_pgtype_byval(List *name, bool byval);
 static void pltsql_proc_get_oid_proname_proacl(AlterFunctionStmt *stmt, ParseState *pstate, Oid *oid, Acl **acl, bool *isSameFunc, bool is_proc);
+static Acl *get_old_view_acl(Oid oldViewOid);
+static void pg_class_update_acl(Oid newViewOid, Acl *oldViewAcl);
 static bool pltsql_truncate_identifier(char *ident, int len, bool warn);
 static Name pltsql_cstr_to_name(char *s, int len);
 extern void pltsql_add_guc_plan(CachedPlanSource *plansource);
@@ -2766,6 +2770,117 @@ bbf_ProcessUtility(PlannedStmt *pstmt,
 				}
 				break;
 			}
+
+			/* 
+			* This case handles the `ALTER VIEW` and `CREATE OR ALTER VIEW` statements in Babelfish.
+			*/
+			case T_ViewStmt:
+			{
+				ViewStmt *stmt = (ViewStmt *) parsetree;
+
+				/*
+				 * We are using PostgreSQL's existing ViewStmt node which is shared between PostgreSQL's
+				 * CREATE VIEW and T-SQL's ALTER VIEW/CREATE OR ALTER VIEW operations. To properly distinguish 
+				 * between these operations and not let CREATE VIEW inside this case we use createOrAlter flag
+				 *
+				 * Since both CREATE VIEW and CREATE OR ALTER VIEW set replace = false initially,
+				 * we use the 'createOrAlter' flag to distinguish between them and implement the
+				 * correct behavior when a view already exists
+				 */
+
+				if (sql_dialect == SQL_DIALECT_TSQL && (stmt->createOrAlter))    
+				{
+					/*
+					 * 1. Retrieve the OID of the old view using `RangeVarGetRelid()`.
+					 * 2. If the old view does not exist, create the new view using `DefineView()` and increment the command counter.
+ 					 * 3. If the old view exists:
+					 *    a. Save the ACL information of the current view.
+					 *    b. Drop the current view using `performDeletion()`.
+					 *    c. Create the new view using `DefineView()` and increment the command counter.
+					 *    f. Store the new view definition in the `bbf_view_def` catalog using `store_view_definition_hook()`.
+					 *    g. Update the ACL information for the new view using `pg_class_update_acl()`.
+					 */
+					ObjectAddress address, originalView;
+					Oid oldViewOid;
+					Acl *oldViewAcl = NULL;
+					bool isCompleteQuery = (context != PROCESS_UTILITY_SUBCOMMAND);
+					bool needCleanup;
+			
+					if (!IS_TDS_CLIENT())
+					{
+							ereport(ERROR,
+									(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+										errmsg("TSQL ALTER VIEW is not supported from PostgreSQL endpoint.")));
+					}
+
+					needCleanup = isCompleteQuery && EventTriggerBeginCompleteQuery();
+			
+					PG_TRY();
+					{
+						StartTransactionCommand();
+						
+						if (isCompleteQuery)
+							EventTriggerDDLCommandStart(parsetree);
+
+						/* Get the old view's OID and verify it exists */
+						oldViewOid = RangeVarGetRelid(stmt->view, AccessExclusiveLock, true);
+						
+						/* If the view does not exist, check if the stmt is CREATE OR ALTER VIEW / ALTER VIEW */
+						if (!OidIsValid(oldViewOid))
+						{
+							if(stmt->replace)	/* we have set replace to false for CREATE OR ALTER VIEW to avoid view does not exist error */
+							{
+								ereport(ERROR,
+										(errcode(ERRCODE_UNDEFINED_TABLE),
+											errmsg("view \"%s\" does not exist", 
+												stmt->view->relname)));
+							}
+							/* View doesn't exist - create it */
+							address = DefineView(stmt, queryString, pstmt->stmt_location, pstmt->stmt_len);
+							CommandCounterIncrement();
+						}
+						/* View exists */
+						else
+						{
+							/* Save ACL before dropping the view */
+							oldViewAcl = get_old_view_acl(oldViewOid);
+							CacheInvalidateRelcacheByRelid(oldViewOid);
+			
+							/* Drop the old view */
+							originalView.objectId = oldViewOid;
+							originalView.classId = RelationRelationId;
+							originalView.objectSubId = 0;
+							performDeletion(&originalView, DROP_RESTRICT, 0);
+							CommandCounterIncrement();
+			
+							/* Create new view */
+							stmt->replace = true; 
+							address = DefineView(stmt, queryString, pstmt->stmt_location, pstmt->stmt_len);
+							CommandCounterIncrement();
+							
+							/* Store the view definition in babelfish_view_def */
+							if(store_view_definition_hook)
+								store_view_definition_hook(queryString, address);
+
+							/* Update ACL info */
+							pg_class_update_acl(address.objectId, oldViewAcl);
+							pfree(oldViewAcl);
+						}
+						CommitTransactionCommand();
+					}
+					PG_FINALLY();
+					{
+						if (needCleanup)
+							EventTriggerEndCompleteQuery();
+					}
+					PG_END_TRY();
+					return; 
+				}
+				/* check that no T-SQL ALTER VIEW operations reach this point because they should have been handled earlier in the code.*/
+				Assert(!(sql_dialect == SQL_DIALECT_TSQL && stmt->createOrAlter));
+				break;
+			}
+
 		case T_AlterTableStmt:
 			{
 				AlterTableStmt *atstmt = (AlterTableStmt *) parsetree;
@@ -4814,6 +4929,73 @@ pltsql_proc_get_oid_proname_proacl(AlterFunctionStmt *stmt, ParseState *pstate,
 	*isSameFunc = OidIsValid(funcOid);
 }
 
+/*
+ * Get the acl of view from pg_class given its oid
+ */
+static Acl *
+get_old_view_acl(Oid oldViewOid)
+	{
+	HeapTuple tuple;
+	Datum aclDatum;
+	bool isNull;
+	Acl *oldViewAcl = NULL;
+
+	tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(oldViewOid));
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_TABLE),
+					errmsg("cache lookup failed for relation %u", oldViewOid)));
+
+	aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl, &isNull);
+	if (!isNull)
+		oldViewAcl = DatumGetAclPCopy(aclDatum);
+
+	ReleaseSysCache(tuple);
+
+	return oldViewAcl;
+	}
+
+/*
+ * Update the acl info of view in pg_class given its oid and acl info
+ */
+static void
+pg_class_update_acl(Oid newViewOid, Acl *oldViewAcl)
+	{
+	Relation pg_class_rel;
+	HeapTuple classtup;
+
+	Datum values[Natts_pg_class];
+	bool nulls[Natts_pg_class];
+	bool replaces[Natts_pg_class];
+	HeapTuple newtup;
+
+	/* Get the tuple from syscache */
+	classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(newViewOid));
+	if (!HeapTupleIsValid(classtup))
+		elog(ERROR, "cache lookup failed for relation %u", newViewOid);
+
+	pg_class_rel = table_open(RelationRelationId, RowExclusiveLock);
+
+	memset(values, 0, sizeof(values));
+	memset(nulls, false, sizeof(nulls));
+	memset(replaces, false, sizeof(replaces));
+
+	if (oldViewAcl != NULL)
+		values[Anum_pg_class_relacl - 1] = PointerGetDatum(oldViewAcl);
+	else
+		nulls[Anum_pg_class_relacl - 1] = true;
+	replaces[Anum_pg_class_relacl - 1] = true;    
+
+	newtup = heap_modify_tuple(classtup, RelationGetDescr(pg_class_rel),
+							values, nulls, replaces);
+
+	CatalogTupleUpdate(pg_class_rel, &newtup->t_self, newtup);
+
+	ReleaseSysCache(classtup);
+	heap_freetuple(newtup);
+	table_close(pg_class_rel, RowExclusiveLock);
+	}
+
 /*
  * Update the pg_type catalog entry for the given name to have
  * typbyval set to the given value.

contrib/babelfishpg_tsql/src/tsqlIface.cpp

@@ -4356,6 +4356,7 @@ storeOriginalQueryForBatchLevelStatement(TSqlParser::Batch_level_statementContex
 {
 	int startIndex = -1;
 	int endIndex = -1;
+	int alterIndex = -1;
 	std::string originalQueryCopy = originalQuery;
 
 	if ((ctx->create_or_alter_procedure() && ctx->create_or_alter_procedure()->ALTER()))
@@ -4372,6 +4373,25 @@ storeOriginalQueryForBatchLevelStatement(TSqlParser::Batch_level_statementContex
 		originalQueryCopy.replace(startIndex, endIndex - startIndex, "CREATE");
 		return pstrdup(originalQueryCopy.c_str());
 	}
+	else if (ctx->create_or_alter_view() && ctx->create_or_alter_view()->ALTER())
+	{
+		startIndex = ctx->create_or_alter_view()->ALTER()->getSymbol()->getStartIndex();
+		endIndex = startIndex + 5;
+		/* if the statement is "ALTER VIEW" */
+		if (!ctx->create_or_alter_view()->CREATE())
+		{
+			originalQueryCopy.replace(startIndex, endIndex - startIndex, "CREATE");
+		}
+		/* if the statement is "CREATE OR ALTER VIEW" */
+		else
+		{
+			startIndex = ctx->create_or_alter_view()->CREATE()->getSymbol()->getStartIndex();
+			alterIndex = ctx->create_or_alter_view()->ALTER()->getSymbol()->getStartIndex();
+			endIndex = alterIndex + 5;
+			originalQueryCopy.replace(startIndex, endIndex - startIndex, "CREATE");
+		}
+		return pstrdup(originalQueryCopy.c_str());
+	}
 	else
 		return pstrdup(originalQueryCopy.c_str());
 }

contrib/babelfishpg_tsql/src/tsqlUnsupportedFeatureHandler.cpp

@@ -485,9 +485,6 @@ antlrcpp::Any TsqlUnsupportedFeatureHandlerImpl::visitCreate_or_alter_trigger(TS
 
 antlrcpp::Any TsqlUnsupportedFeatureHandlerImpl::visitCreate_or_alter_view(TSqlParser::Create_or_alter_viewContext *ctx)
 {
-	if (ctx->ALTER())
-		handle(INSTR_UNSUPPORTED_TSQL_ALTER_VIEW, "ALTER VIEW", getLineAndPos(ctx->ALTER()));
-
 	/* escape hatch of SCHEMABINDING option*/
 	if (escape_hatch_schemabinding_view != EH_IGNORE)
 	{

test/JDBC/expected/BABEL-UNSUPPORTED.out

@@ -2414,16 +2414,8 @@ CREATE VIEW v_babel_2017 AS SELECT * FROM t_babel_2017;
 GO
 ALTER VIEW v_babel_2017 AS SELECT a FROM t_babel_2017;
 GO
-~~ERROR (Code: 33557097)~~
-
-~~ERROR (Message: 'ALTER VIEW' is not currently supported in Babelfish)~~
-
 CREATE OR ALTER VIEW v_babel_2017 AS SELECT b FROM t_babel_2017;
 GO
-~~ERROR (Code: 33557097)~~
-
-~~ERROR (Message: 'ALTER VIEW' is not currently supported in Babelfish)~~
-
 DROP VIEW v_babel_2017;
 GO
 DROP TABLE t_babel_2017;