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

R4hul04 · 2025-03-12T00:05:37Z

Description

Currently, Babelfish does not support ALTER VIEW and CREATE OR ALTER VIEW syntax statement in the TSQL dialect. With this change, Babelfish will support ALTER VIEW operations, allowing users to modify existing views.

We assume views to not have any dependencies for alter to succeed.
For dependent procedures, alter view will continue to work and will not be affected.
Permissions on views won't be affected

The change was made to enhance Babelfish's compatibility with SQL Server by supporting the ALTER VIEW statement. This allows users to modify views in a manner consistent with SQL Server.

[NOTE]: ALTER VIEW for views having dependencies will be supported once Babelfish supports weak schema bound views.

Engine PR - babelfish-for-postgresql/postgresql_modified_for_babelfish#545

Signed off by: Rahul Parande [email protected]

Issues Resolved

JIRA - BABEL-2017

Test Scenarios Covered

Use case based -

CREATE OR ALTER VIEW v1 AS SELECT a, b FROM t;
GO
SELECT * FROM v1;
GO
~~START~~
int#!#varchar
1#!#a
2#!#b
3#!#c

Boundary conditions -

CREATE OR ALTER VIEW v1 AS SELECT a, b, c, d, e, f FROM t;
GO
~~ERROR (Code: 33557097)~~

~~ERROR (Message: column "e" does not exist)~~

-- Create view with existing object name [Error]
CREATE VIEW v1 AS SELECT a, b FROM t;
GO
~~ERROR (Code: 2714)~~

~~ERROR (Message: relation "v1" already exists)~~

Arbitrary inputs -

ALTER VIEW v7 AS SELECT a, b, CAST(c AS date) AS c, d, e FROM t4;
GO
~~ERROR (Code: 33557097)~~

~~ERROR (Message: cannot cast type "decimal" to date)~~

Negative test cases -

-- Error as dependent view exists
Drop table t;
GO
~~ERROR (Code: 3726)~~

~~ERROR (Message: cannot drop table t because other objects depend on it)~~

Minor version upgrade tests -
Major version upgrade tests -
Performance tests -
Tooling impact -
Client tests -

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

coveralls · 2025-03-12T19:27:14Z

Pull Request Test Coverage Report for Build 13938113398

Details

77 of 81 (95.06%) changed or added relevant lines in 3 files are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.03%) to 75.015%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
contrib/babelfishpg_tsql/src/pl_handler.c	65	69	94.2%

Files with Coverage Reduction	New Missed Lines	%
contrib/babelfishpg_tds/src/backend/tds/tdscomm.c	2	76.03%

Totals
Change from base Build 13937971111:	0.03%
Covered Lines:	47505
Relevant Lines:	63327

💛 - Coveralls

test/JDBC/expected/alter-view-vu-cleanup.out

contrib/babelfishpg_tsql/src/tsqlUnsupportedFeatureHandler.cpp

contrib/babelfishpg_tsql/src/pl_handler.c

contrib/babelfishpg_tsql/src/dbcmds.c

contrib/babelfishpg_tsql/src/pl_handler.c

contrib/babelfishpg_tsql/src/dbcmds.c

contrib/babelfishpg_tsql/src/pl_handler.c

test/JDBC/input/alter/alter-view-vu-verify.sql

contrib/babelfishpg_tsql/src/dbcmds.c

contrib/babelfishpg_tsql/src/pl_handler.c

forestkeeper · 2025-03-17T18:23:31Z

contrib/babelfishpg_tsql/src/dbcmds.c

@@ -149,7 +149,7 @@ gen_createdb_subcmds(const char *dbname, const char *owner)

 	/* create sysdatabases under current DB's DBO schema */
 	appendStringInfo(&query, "CREATE VIEW dummy.sysdatabases AS SELECT * FROM sys.sysdatabases; ");
-	appendStringInfo(&query, "ALTER VIEW dummy.sysdatabases OWNER TO dummy; ");


What's the purpose for this line ?

If we use ALTER VIEW, it will raise syntax error near OWNER while doing initbbf because of different ALTER VIEW behavior between PG and tsql. So we use ALTER TABLE instead to avoid that, as PG can use ALTER TABLE as well to change the ownership of view. (Postgresql Alter Table)

Pls add a comment into the code for this behavior and the purpose for this change.

Added comments in the code

Please add test cases for postgres endpoint. ALTER VIEW ... should continue to work as before for postgres endpoint.

Added test cases to check psql ALTER VIEW

contrib/babelfishpg_tsql/src/pl_handler.c

timchang514 · 2025-03-17T19:07:45Z

contrib/babelfishpg_tsql/src/pl_handler.c

+ * 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)


let's also add a test that verifies that the acl is properly updated when we do ALTER VIEW.

Added .mix test file to test ACL changes.

timchang514 · 2025-03-17T19:09:23Z

contrib/babelfishpg_tsql/src/tsqlIface.cpp

@@ -4376,6 +4377,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())


what is the purpose of these changes?

After the alter operation is complete, the definition that gets stored in catalogs is something like Alter view v1 as select 1; instead of Create view v1 as select 1;. This change allows storing correct definitions after ALTER/CREATE OR ALTER VIEW operations. Appropriate tests are added in the test files. SQL server does the same, it stores the definitions as CREATE VIEW instead of ALTER VIEW

Pls add the comments into the code as well.

Added comments in the code

forestkeeper · 2025-03-17T20:51:26Z

contrib/babelfishpg_tsql/src/pl_handler.c

+						StartTransactionCommand();
+
+						if (isCompleteQuery)
+							EventTriggerDDLCommandStart(parsetree);


Which is the test case for this ?

Without this code, DDL event triggers won't fire for ALTER VIEW operations. Currently we do not support DDL triggers for T-SQL, but tested it out from from PSQL endpoint. This code can be used later, once we support DDL triggers in T-SQL. Added comment in the code.

timchang514 · 2025-03-18T16:25:30Z

contrib/babelfishpg_tsql/src/pl_handler.c

+						else
+						{
+							/* Save ACL before dropping the view */
+							oldViewAcl = get_old_view_acl(oldViewOid);


To me it seems like get_old_view_acl can potentially return NULL. There is even a check for it in pg_class_update_acl. Then it would cause a crash once you hit pfree(oldViewAcl).

So either add a check that get_old_view_acl cannot return NULL - throw an error (then the case in pg_class_update_acl is not needed), or ensure that pfree isn't called on NULL.

Older version views may have NULL ACL, added check to pfree(oldViewAcl) only if oldViewAcl is non null

timchang514 · 2025-03-18T16:34:17Z

contrib/babelfishpg_tsql/src/pl_handler.c

+							originalView.objectId = oldViewOid;
+							originalView.classId = RelationRelationId;
+							originalView.objectSubId = 0;
+							performDeletion(&originalView, DROP_RESTRICT, 0);


DROP_RESTRICT means this will fail if there's any objects dependent on the view. Could we test this - create an object dependent on a view and attempt to alter the underlying view?

Added to test case

-Changed subcmd ALTER VIEW to ALTER TABLE in dbcmds.c -Removed usage of PG dialect to generate subcmds -Added acl tests and addressed comments -Added test case for dependent check -added assert for relacl null condition -Added more test cases to check psql alter view syntax Task : BABEL-2017

…bound views in Babelfish Cherry-Pick commit from PR from #3570 Currently, Babelfish does not support ALTER VIEW and CREATE OR ALTER VIEW syntax statement in the TSQL dialect. With this change, Babelfish will support ALTER VIEW operations, allowing users to modify existing views. We assume views to not have any dependencies for alter to succeed. For dependent procedures, alter view will continue to work and will not be affected. Permissions on views won't be affected The change was made to enhance Babelfish's compatibility with SQL Server by supporting the ALTER VIEW statement. This allows users to modify views in a manner consistent with SQL Server. [NOTE]: ALTER VIEW for views having dependencies will be supported once Babelfish supports weak schema bound views. Engine PR - babelfish-for-postgresql/postgresql_modified_for_babelfish#547 Signed off by: Rahul Parande <[email protected]> Issues Resolved: JIRA - BABEL-2017

R4hul04 marked this pull request as ready for review March 12, 2025 20:34

R4hul04 mentioned this pull request Mar 12, 2025

Added createoralter flag to ViewStmt node to recognize the ALTER VIEW statement babelfish-for-postgresql/postgresql_modified_for_babelfish#545

Merged

1 task

forestkeeper reviewed Mar 12, 2025

View reviewed changes

test/JDBC/expected/alter-view-vu-cleanup.out Show resolved Hide resolved

forestkeeper reviewed Mar 12, 2025

View reviewed changes

contrib/babelfishpg_tsql/src/tsqlUnsupportedFeatureHandler.cpp Outdated Show resolved Hide resolved

forestkeeper reviewed Mar 12, 2025

View reviewed changes

contrib/babelfishpg_tsql/src/pl_handler.c Outdated Show resolved Hide resolved

forestkeeper reviewed Mar 12, 2025

View reviewed changes

contrib/babelfishpg_tsql/src/dbcmds.c Outdated Show resolved Hide resolved

forestkeeper reviewed Mar 12, 2025

View reviewed changes

contrib/babelfishpg_tsql/src/pl_handler.c Show resolved Hide resolved

forestkeeper reviewed Mar 12, 2025

View reviewed changes

contrib/babelfishpg_tsql/src/pl_handler.c Outdated Show resolved Hide resolved

timchang514 reviewed Mar 13, 2025

View reviewed changes

timchang514 reviewed Mar 14, 2025

View reviewed changes

R4hul04 force-pushed the alter-view-support branch 2 times, most recently from 13d2b2a to 60a96a9 Compare March 15, 2025 21:31

forestkeeper reviewed Mar 17, 2025

View reviewed changes

R4hul04 force-pushed the alter-view-support branch from 83e22c8 to c95c503 Compare March 17, 2025 18:29

timchang514 reviewed Mar 17, 2025

View reviewed changes

forestkeeper reviewed Mar 17, 2025

View reviewed changes

timchang514 reviewed Mar 18, 2025

View reviewed changes

forestkeeper previously approved these changes Mar 18, 2025

View reviewed changes

R4hul04 dismissed forestkeeper’s stale review via ce25828 March 18, 2025 19:21

timchang514 approved these changes Mar 18, 2025

View reviewed changes

R4hul04 force-pushed the alter-view-support branch 3 times, most recently from d5efe07 to 35e9a8e Compare March 18, 2025 23:49

Rahul Parande added 2 commits March 19, 2025 03:16

Trigger Build

c64ba1f

R4hul04 force-pushed the alter-view-support branch from 35e9a8e to c64ba1f Compare March 19, 2025 03:18

R4hul04 mentioned this pull request Mar 19, 2025

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

Merged

1 task

forestkeeper approved these changes Mar 19, 2025

View reviewed changes

forestkeeper merged commit c6c0176 into babelfish-for-postgresql:BABEL_5_X_DEV Mar 19, 2025
47 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

R4hul04 commented Mar 12, 2025 •

edited

Loading

coveralls commented Mar 12, 2025 •

edited

Loading

forestkeeper Mar 17, 2025

R4hul04 Mar 17, 2025

forestkeeper Mar 17, 2025

R4hul04 Mar 18, 2025

tanscorpio7 Mar 18, 2025

R4hul04 Mar 18, 2025 •

edited

Loading

timchang514 Mar 17, 2025

R4hul04 Mar 18, 2025 •

edited

Loading

timchang514 Mar 17, 2025

R4hul04 Mar 17, 2025 •

edited

Loading

forestkeeper Mar 17, 2025

R4hul04 Mar 18, 2025 •

edited

Loading

forestkeeper Mar 17, 2025

R4hul04 Mar 18, 2025 •

edited

Loading

timchang514 Mar 18, 2025

R4hul04 Mar 18, 2025 •

edited

Loading

timchang514 Mar 18, 2025

R4hul04 Mar 18, 2025

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

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

Conversation

R4hul04 commented Mar 12, 2025 • edited Loading

Description

Issues Resolved

Test Scenarios Covered

Check List

coveralls commented Mar 12, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13938113398

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R4hul04 Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R4hul04 Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R4hul04 Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R4hul04 Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R4hul04 Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R4hul04 Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R4hul04 commented Mar 12, 2025 •

edited

Loading

coveralls commented Mar 12, 2025 •

edited

Loading

R4hul04 Mar 18, 2025 •

edited

Loading

R4hul04 Mar 18, 2025 •

edited

Loading

R4hul04 Mar 17, 2025 •

edited

Loading

R4hul04 Mar 18, 2025 •

edited

Loading

R4hul04 Mar 18, 2025 •

edited

Loading

R4hul04 Mar 18, 2025 •

edited

Loading