Support for weak binding views to allow dropping of underlying objects #585

R4hul04 · 2025-06-05T17:20:45Z

Description

This PR introduces improvements to Babelfish's view handling, focusing on weak binding views. The changes enable more SQL Server-compatible behavior for view dependencies.

This PR introduces the following changes:

Added hooks in DROP codepath to allow dropping underlying objects (tables, functions, views) referenced by weak binding views without errors, while maintaining the view structure
Implemented a view repair mechanism that automatically fixes broken views when their referenced objects are recreated, eliminating the need for manual view recreation
Added validation to prevent creating schema-bound views that reference non-schema-bound views, matching SQL Server's behavior

Implementation Details

Added a new flags BBF_VIEW_DEF_FLAG_IS_BROKEN to track views with missing dependencies and BBF_VIEW_DEF_FLAG_IS_WEAK_VIEW to identify view's schemabinding property in babelfish_view_def catalog
Implemented hooks in object drop operations to detect and mark dependent views as broken
Created a view repair mechanism that activates when a broken view is accessed during the rewriting
Added recursive dependency tracking to handle chains of view dependencies

Issues Resolved

BABEL-1660

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 PostgreSQL license, 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.

Signed-off-by: Rahul Parande <[email protected]>

forestkeeper · 2025-06-05T23:42:48Z

src/backend/commands/dropcmds.c

@@ -131,6 +133,20 @@ RemoveObjects(DropStmt *stmt)
 			table_close(relation, NoLock);

 		add_exact_object_address(&address, objects);
+
+		/* Check for strong views and handle weak views */


Why not move the open table into the hook implement ?

views Signed-off-by: Rahul Parande <[email protected]>

Signed-off-by: Rahul Parande <[email protected]>

forestkeeper · 2025-06-12T20:50:42Z

src/bin/pg_dump/pg_dump.c

+ * This returns a new buffer which must be freed by the caller.
+ */
+static PQExpBuffer
+createDummyViewAsClause(Archive *fout, const TableInfo *tbinfo)


Why not reuse the createDummyViewAsClause def from community ?

We are reusing the same function. We are only moving it before createViewAsClause, since we need createDummyViewAsClause defined before the function call in createViewAsClause

We should not move this function's place in community code, since after moving, the follow up new commits diff from community will be lost for this function.

Agreed.
Added the function declaration of createDummyViewAsClause at the top instead of moving the whole function body.

forestkeeper · 2025-06-12T20:55:22Z

src/bin/pg_dump/pg_dump.c

@@ -15833,6 +15877,7 @@ createViewAsClause(Archive *fout, const TableInfo *tbinfo)
 {
 	PQExpBuffer query = createPQExpBuffer();
 	PQExpBuffer result = createPQExpBuffer();
+	PQExpBuffer dummyResult = createPQExpBuffer();


Dummyresult doesn't need to be created as createPQExpBuffer.

Removed dummyResult variable instead using the existing variable result

forestkeeper · 2025-06-12T20:56:48Z

src/bin/pg_dump/pg_dump.c

@@ -15869,50 +15927,6 @@ createViewAsClause(Archive *fout, const TableInfo *tbinfo)
 	return result;
 }

-/*
- * Create a dummy AS clause for a view.  This is used when the real view


Why move the whole function definition ?

Added createDummyViewAsClause inside createViewAsClause. Hence we needed to define createDummyViewAsClause before createViewAsClause.

We should find a alternative way rather than moving this function.

Added the function declaration of createDummyViewAsClause at the top instead of moving the whole function body.

forestkeeper · 2025-06-12T20:57:37Z

src/backend/commands/dropcmds.c

@@ -19,9 +19,11 @@
 #include "catalog/dependency.h"
 #include "catalog/namespace.h"
 #include "catalog/objectaddress.h"
+#include "catalog/pg_depend_d.h"


Why adding this .h ?

Removed pg_depend_d.h

Signed-off-by: Rahul Parande <[email protected]>

instead added it's declaration at the top to access the function Signed-off-by: Rahul Parande <[email protected]>

tanscorpio7 · 2025-06-17T18:50:19Z

src/backend/rewrite/rewriteHandler.c

+     * Step 0 (Babelfish extension)
+     *
+     * If this is a view with broken rules, try to repair it
+     * using the definition from babelfish_view_def
+     */
+	if (view_repair_hook)
+		(*view_repair_hook)(parsetree);
+	/*


Should we use a more generic name. pre_query_rewrite_hook

+1
Let's use the name pre_QueryRewrite_hook.

Renamed the hook to pre_QueryRewrite_hook

tanscorpio7 · 2025-06-17T18:57:27Z

src/backend/commands/tablecmds.c

+
+		/* Check for strong views and handle weak views */
+		if ((drop->removeType == OBJECT_TABLE || drop->removeType == OBJECT_VIEW) && view_dependency_hook)
+		{			
+			if (!(*view_dependency_hook)(&obj, NULL, NULL))
+				continue;
+		}


Let us move all comments and conditions inside the hook. Also would naming it pre_drop_object_hook make sense ?

Moved implementation to InvokeObjectDropHook instead.

forestkeeper · 2025-06-17T20:41:32Z

src/backend/commands/tablecmds.c

@@ -1676,6 +1677,13 @@ RemoveRelations(DropStmt *drop)
 		{
 			InvokeObjectDropHook(RelationRelationId,relOid,0);
 		}
+
+		/* Check for strong views and handle weak views */
+		if ((drop->removeType == OBJECT_TABLE || drop->removeType == OBJECT_VIEW) && view_dependency_hook)


Can we move this part also into the hook : (drop->removeType == OBJECT_TABLE || drop->removeType == OBJECT_VIEW) ?

Let's use InvokeObjectDropHook which already there in the function.

Moved implementation to use InvokeObjectDropHook instead.

kuntalghosh · 2025-06-18T05:36:27Z

src/backend/commands/dropcmds.c

+		if ((stmt->removeType == OBJECT_FUNCTION) && view_dependency_hook)
+		{			
+			if (!(*view_dependency_hook)(&address, NULL, NULL))
+				continue;
+		}


Let's make the hook generic and add all TSQL related check within the hook function. We're not worried about hook overhead in a DDL execution. Also, there's already a hook for this task - InvokeObjectDropHook. Let's plugin this hook here.

Used InvokeObjectDropHook instead of view_dependency_hook.

kuntalghosh · 2025-06-18T05:36:55Z

src/backend/commands/tablecmds.c

@@ -1676,6 +1677,13 @@ RemoveRelations(DropStmt *drop)
 		{
 			InvokeObjectDropHook(RelationRelationId,relOid,0);
 		}
+
+		/* Check for strong views and handle weak views */
+		if ((drop->removeType == OBJECT_TABLE || drop->removeType == OBJECT_VIEW) && view_dependency_hook)


Let's use InvokeObjectDropHook which already there in the function.

kuntalghosh · 2025-06-18T05:39:31Z

src/backend/commands/view.c

+	if (check_view_dependencies_hook)
+			(*check_view_dependencies_hook)(viewParse);
+


The function parse_analyze_fixedparams already has a hook inside it - post_parse_analyze_hook(). Let's use that.

Updated post_parse_analyze_hook() with the required logic.

kuntalghosh · 2025-06-18T05:42:22Z

src/backend/rewrite/rewriteHandler.c

+     * Step 0 (Babelfish extension)
+     *
+     * If this is a view with broken rules, try to repair it
+     * using the definition from babelfish_view_def
+     */
+	if (view_repair_hook)
+		(*view_repair_hook)(parsetree);
+	/*


+1
Let's use the name pre_QueryRewrite_hook.

kuntalghosh · 2025-06-18T06:06:34Z

src/bin/pg_dump/pg_dump.c

-		pg_fatal("definition of view \"%s\" appears to be empty (length zero)",
-				 tbinfo->dobj.name);
+	{
+		/* 
+		 * Handle broken views (with empty definitions)
+		 * Instead of failing, create a dummy SELECT that preserves the structure
+		 */
+		PQclear(res);
+		destroyPQExpBuffer(query);
+
+		/* Use createDummyViewAsClause to generate a compatible structure */
+		result = createDummyViewAsClause(fout, tbinfo);
+
+		return result;
+	}


Whenever we modify in pg_dump, we make sure that this is a Babelfish Database. Check the functions with bbf prefix. Ideally, the logic should be,

if (bbfShouldDumpViews(Archive *fout, const TableInfo *tbinfo)) { createTSQLViewAsClause() return; }

Within createTSQLViewAsClause, we should do all the handlings.

Do we need to create our own createTSQLViewAsClause function in dump_babel_utils.c which will be called specifically for babelfish database. This function will be called from createViewAsClause in pg_dump.c ?

I thought we will insert a dummy view query into pg_rewrite at the time of DROP which will avoid the need to alter dump restore. The dummy query can be prepared according to the columns.
We should avoid keeping empty string as rewrite rule since engine code assumes that is an impossible case.

InvokeObjectDropHook. 2. Removed check_view_dependencies_hook from view.c and used existing post_parse_analyze_hook. 3. Renamed view_repair_hook to pre_QueryRewrite_hook Signed-off-by: Rahul Parande <[email protected]>

Signed-off-by: Rahul Parande <[email protected]>

…Hook Signed-off-by: Rahul Parande <[email protected]>

as special call for dependency check during function drop Signed-off-by: Rahul Parande <[email protected]>

Rahul Parande added 5 commits June 5, 2025 17:18

Support for weak binding views to allow dropping of underlying objects

7149fc4

Signed-off-by: Rahul Parande <[email protected]>

Added view repair hook definiton in rewriteHandler.h

e8b57d6

Signed-off-by: Rahul Parande <[email protected]>

Fixed indentation

a63802e

Signed-off-by: Rahul Parande <[email protected]>

Fix indentation in pg_dump.c

e443af6

Signed-off-by: Rahul Parande <[email protected]>

Fixed variable declaration in pg_dump.c

00bb61a

Signed-off-by: Rahul Parande <[email protected]>

forestkeeper reviewed Jun 5, 2025

View reviewed changes

Rahul Parande added 4 commits June 9, 2025 19:12

Added hook in view creation path to check view references on strong

301e7cc

views Signed-off-by: Rahul Parande <[email protected]>

Added a dialect check when calling the hook in view.c

f292acc

Signed-off-by: Rahul Parande <[email protected]>

Removed dialect check from view.c and added inside hook

413f6d2

Signed-off-by: Rahul Parande <[email protected]>

Moved code for opening table inside hook instead in engine

e9b3925

Signed-off-by: Rahul Parande <[email protected]>

R4hul04 requested a review from kuntalghosh June 10, 2025 00:56

forestkeeper reviewed Jun 12, 2025

View reviewed changes

R4hul04 marked this pull request as ready for review June 16, 2025 19:05

R4hul04 requested a review from tanscorpio7 June 16, 2025 19:05

Rahul Parande added 2 commits June 17, 2025 17:49

Removed unnecessary header file pg_depend.h from dropcmds.c file

9e0f032

Signed-off-by: Rahul Parande <[email protected]>

Reverted the placement of createDummyViewAsClause to original and

97b0a95

instead added it's declaration at the top to access the function Signed-off-by: Rahul Parande <[email protected]>

tanscorpio7 suggested changes Jun 17, 2025

View reviewed changes

forestkeeper reviewed Jun 17, 2025

View reviewed changes

kuntalghosh requested changes Jun 18, 2025

View reviewed changes

Rahul Parande added 4 commits June 18, 2025 19:55

1. Replaced view_dependency_hook with pre-existing hook

1aed8d8

InvokeObjectDropHook. 2. Removed check_view_dependencies_hook from view.c and used existing post_parse_analyze_hook. 3. Renamed view_repair_hook to pre_QueryRewrite_hook Signed-off-by: Rahul Parande <[email protected]>

Removed unnecessary hook declarations

5b75cba

Signed-off-by: Rahul Parande <[email protected]>

Added dialect check for function drop before calling InvokeObjectDrop…

c9292bd

…Hook Signed-off-by: Rahul Parande <[email protected]>

updated argument in InvokeObjectDropHook for subid as -1 to recognize it

010f0f4

as special call for dependency check during function drop Signed-off-by: Rahul Parande <[email protected]>

		if (check_view_dependencies_hook)
		(*check_view_dependencies_hook)(viewParse);

Support for weak binding views to allow dropping of underlying objects #585

Are you sure you want to change the base?

Support for weak binding views to allow dropping of underlying objects #585

Uh oh!

Conversation

R4hul04 commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

R4hul04 Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

R4hul04 Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

R4hul04 commented Jun 5, 2025 •

edited

Loading

R4hul04 Jun 17, 2025 •

edited

Loading

R4hul04 Jun 18, 2025 •

edited

Loading