Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: BABEL_5_X_DEV__PG_17_X
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/backend/commands/dropcmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/objectaddress.h"
#include "catalog/pg_depend_d.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why adding this .h ?

#include "catalog/pg_namespace.h"
#include "catalog/pg_proc.h"
#include "commands/defrem.h"
#include "commands/tablecmds.h"
#include "miscadmin.h"
#include "parser/parser.h"
#include "parser/parse_type.h"
Expand Down Expand Up @@ -131,6 +133,13 @@ RemoveObjects(DropStmt *stmt)
table_close(relation, NoLock);

add_exact_object_address(&address, objects);

/* Check for strong views and handle weak views */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not move the open table into the hook implement ?

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

/* Here we really delete them. */
Expand Down
8 changes: 8 additions & 0 deletions src/backend/commands/tablecmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ InvokePreDropColumnHook_type InvokePreDropColumnHook = NULL;
check_extended_attoptions_hook_type check_extended_attoptions_hook = NULL;
find_attr_by_name_from_column_def_list_hook_type
find_attr_by_name_from_column_def_list_hook = NULL;
view_dependency_hook_type view_dependency_hook = NULL;

/*
* State information for ALTER TABLE
Expand Down Expand Up @@ -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)
{
if (!(*view_dependency_hook)(&obj, NULL, NULL))
continue;
}
}

performMultipleDeletions(objects, drop->behavior, flags);
Expand Down
6 changes: 6 additions & 0 deletions src/backend/commands/view.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
#include "parser/parser.h"
#include "parser/parse_relation.h"
#include "rewrite/rewriteDefine.h"
#include "rewrite/rewriteHandler.h"
Expand All @@ -36,6 +37,8 @@ static void checkViewColumns(TupleDesc newdesc, TupleDesc olddesc);

inherit_view_constraints_from_table_hook_type inherit_view_constraints_from_table_hook = NULL;

check_view_dependencies_hook_type check_view_dependencies_hook = NULL;

/*---------------------------------------------------------------------
* DefineVirtualRelation
*
Expand Down Expand Up @@ -381,6 +384,9 @@ DefineView(ViewStmt *stmt, const char *queryString,

viewParse = parse_analyze_fixedparams(rawstmt, queryString, NULL, 0, NULL);

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

/*
* The grammar should ensure that the result is a single SELECT Query.
* However, it doesn't forbid SELECT INTO, so we have to check for that.
Expand Down
10 changes: 9 additions & 1 deletion src/backend/rewrite/rewriteHandler.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "utils/rel.h"

bbfViewHasInsteadofTrigger_hook_type bbfViewHasInsteadofTrigger_hook = NULL; /** BBF Hook to check Instead Of trigger on View */
view_repair_hook_type view_repair_hook = NULL;

/* We use a list of these to detect recursion in RewriteQuery */
typedef struct rewrite_event
Expand Down Expand Up @@ -4403,7 +4404,6 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
return rewritten;
}


/*
* QueryRewrite -
* Primary entry point to the query rewriter.
Expand Down Expand Up @@ -4431,6 +4431,14 @@ QueryRewrite(Query *parsetree)
Assert(parsetree->canSetTag);

/*
* 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);
/*
* Step 1
*
* Apply all non-SELECT rules possibly getting 0 or many queries
Expand Down
106 changes: 60 additions & 46 deletions src/bin/pg_dump/pg_dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -15822,6 +15822,50 @@ dumpTable(Archive *fout, const TableInfo *tbinfo)
free(namecopy);
}

/*
* Create a dummy AS clause for a view. This is used when the real view
* definition has to be postponed because of circular dependencies.
* We must duplicate the view's external properties -- column names and types
* (including collation) -- so that it works for subsequent references.
*
* This returns a new buffer which must be freed by the caller.
*/
static PQExpBuffer
createDummyViewAsClause(Archive *fout, const TableInfo *tbinfo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not reuse the createDummyViewAsClause def from community ?

{
PQExpBuffer result = createPQExpBuffer();
int j;

appendPQExpBufferStr(result, "SELECT");

for (j = 0; j < tbinfo->numatts; j++)
{
if (j > 0)
appendPQExpBufferChar(result, ',');
appendPQExpBufferStr(result, "\n ");

appendPQExpBuffer(result, "NULL::%s", tbinfo->atttypnames[j]);

/*
* Must add collation if not default for the type, because CREATE OR
* REPLACE VIEW won't change it
*/
if (OidIsValid(tbinfo->attcollation[j]))
{
CollInfo *coll;

coll = findCollationByOid(tbinfo->attcollation[j]);
if (coll)
appendPQExpBuffer(result, " COLLATE %s",
fmtQualifiedDumpable(coll));
}

appendPQExpBuffer(result, " AS %s", fmtId(tbinfo->attnames[j]));
}

return result;
}

/*
* Create the AS clause for a view or materialized view. The semicolon is
* stripped because a materialized view must add a WITH NO DATA clause.
Expand All @@ -15833,6 +15877,7 @@ createViewAsClause(Archive *fout, const TableInfo *tbinfo)
{
PQExpBuffer query = createPQExpBuffer();
PQExpBuffer result = createPQExpBuffer();
PQExpBuffer dummyResult = createPQExpBuffer();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dummyresult doesn't need to be created as createPQExpBuffer.

PGresult *res;
int len;

Expand All @@ -15856,8 +15901,21 @@ createViewAsClause(Archive *fout, const TableInfo *tbinfo)
len = PQgetlength(res, 0, 0);

if (len == 0)
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 */
dummyResult = createDummyViewAsClause(fout, tbinfo);
appendPQExpBuffer(result, "%s", dummyResult->data);
destroyPQExpBuffer(dummyResult);

return result;
}

/* Strip off the trailing semicolon so that other things may follow. */
Assert(PQgetvalue(res, 0, 0)[len - 1] == ';');
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why move the whole function definition ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* definition has to be postponed because of circular dependencies.
* We must duplicate the view's external properties -- column names and types
* (including collation) -- so that it works for subsequent references.
*
* This returns a new buffer which must be freed by the caller.
*/
static PQExpBuffer
createDummyViewAsClause(Archive *fout, const TableInfo *tbinfo)
{
PQExpBuffer result = createPQExpBuffer();
int j;

appendPQExpBufferStr(result, "SELECT");

for (j = 0; j < tbinfo->numatts; j++)
{
if (j > 0)
appendPQExpBufferChar(result, ',');
appendPQExpBufferStr(result, "\n ");

appendPQExpBuffer(result, "NULL::%s", tbinfo->atttypnames[j]);

/*
* Must add collation if not default for the type, because CREATE OR
* REPLACE VIEW won't change it
*/
if (OidIsValid(tbinfo->attcollation[j]))
{
CollInfo *coll;

coll = findCollationByOid(tbinfo->attcollation[j]);
if (coll)
appendPQExpBuffer(result, " COLLATE %s",
fmtQualifiedDumpable(coll));
}

appendPQExpBuffer(result, " AS %s", fmtId(tbinfo->attnames[j]));
}

return result;
}

/*
* dumpTableSchema
* write the declaration (not data) of one user-defined table or view
Expand Down
4 changes: 4 additions & 0 deletions src/include/commands/tablecmds.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,8 @@ typedef int (*find_attr_by_name_from_column_def_list_hook_type) (
extern PGDLLEXPORT find_attr_by_name_from_column_def_list_hook_type
find_attr_by_name_from_column_def_list_hook;

/* Hook for handling view dependencies during table drops */
typedef bool (*view_dependency_hook_type) (const ObjectAddress *droppedObject, Relation depRel, ViewStmt *viewstmt);
extern PGDLLIMPORT view_dependency_hook_type view_dependency_hook;

#endif /* TABLECMDS_H */
3 changes: 3 additions & 0 deletions src/include/commands/view.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,7 @@ extern PGDLLEXPORT store_view_definition_hook_type store_view_definition_hook;

typedef void (*inherit_view_constraints_from_table_hook_type) (ColumnDef *col, Oid tableOid, AttrNumber colId);
extern PGDLLEXPORT inherit_view_constraints_from_table_hook_type inherit_view_constraints_from_table_hook;

typedef bool (*check_view_dependencies_hook_type) (Query *viewParse);
extern check_view_dependencies_hook_type check_view_dependencies_hook;
#endif /* VIEW_H */
4 changes: 4 additions & 0 deletions src/include/rewrite/rewriteHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,8 @@ extern void error_view_not_updatable(Relation view,
List *mergeActionList,
const char *detail);

/* View repair hook */
typedef bool (*view_repair_hook_type) (Query *parsetree);
extern PGDLLEXPORT view_repair_hook_type view_repair_hook;

#endif /* REWRITEHANDLER_H */