From 60bab74ffe2dd71afb96c0199391972526aa8a61 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 08:00:45 +0000 Subject: [PATCH 01/14] Fix bugs for GRANT/REVOKE on SCHEMA --- contrib/babelfishpg_tsql/src/pl_exec-2.c | 35 +++--- contrib/babelfishpg_tsql/src/pl_handler.c | 102 +++++++++++++----- contrib/babelfishpg_tsql/src/pltsql_utils.c | 5 +- contrib/babelfishpg_tsql/src/tsqlIface.cpp | 20 ++++ .../src/tsqlUnsupportedFeatureHandler.cpp | 8 +- test/JDBC/expected/BABEL-GRANT.out | 34 +++++- test/JDBC/expected/GRANT_SCHEMA.out | 47 +++++++- test/JDBC/input/BABEL-GRANT.sql | 18 +++- test/JDBC/input/GRANT_SCHEMA.mix | 28 ++++- 9 files changed, 245 insertions(+), 52 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/pl_exec-2.c b/contrib/babelfishpg_tsql/src/pl_exec-2.c index 4e948a0be0f..05e74327aaa 100644 --- a/contrib/babelfishpg_tsql/src/pl_exec-2.c +++ b/contrib/babelfishpg_tsql/src/pl_exec-2.c @@ -3683,7 +3683,6 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) char *dbname = get_cur_db_name(); char *login = GetUserNameFromId(GetSessionUserId(), false); bool login_is_db_owner; - Oid datdba; char *rolname; char *schema_name; ListCell *lc; @@ -3695,20 +3694,24 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) * sysadmin or login is not the schema owner, then it doesn't have the permission to GRANT/REVOKE. */ login_is_db_owner = 0 == strncmp(login, get_owner_of_db(dbname), NAMEDATALEN); - datdba = get_role_oid("sysadmin", false); schema_name = get_physical_schema_name(dbname, stmt->schema_name); - schemaOid = LookupExplicitNamespace(schema_name, true); + + if(schema_name) + { + schemaOid = LookupExplicitNamespace(schema_name, true); + } + else + { + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_SCHEMA), + errmsg("An object or column name is missing or empty. For SELECT INTO statements, verify each column has a name. For other statements, look for empty alias names. Aliases defined as \"\" or [] are not allowed. Change the alias to a valid name."))); + } if (!OidIsValid(schemaOid)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_SCHEMA), errmsg("schema \"%s\" does not exist", schema_name))); - - if (!is_member_of_role(GetSessionUserId(), datdba) && !login_is_db_owner && !pg_namespace_ownercheck(schemaOid, GetUserId())) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("Cannot find the schema \"%s\", because it does not exist or you do not have permission.", stmt->schema_name))); foreach(lc1, stmt->privileges) { @@ -3716,22 +3719,30 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) foreach(lc, stmt->grantees) { char *grantee_name = (char *) lfirst(lc); + char *user = GetUserNameFromId(GetUserId(), false); Oid role_oid; bool grantee_is_db_owner; - rolname = get_physical_user_name(dbname, grantee_name); + if (strcmp(grantee_name, "public") != 0) + rolname = get_physical_user_name(dbname, grantee_name); + else + rolname = pstrdup("public"); role_oid = get_role_oid(rolname, true); grantee_is_db_owner = 0 == strncmp(grantee_name, get_owner_of_db(dbname), NAMEDATALEN); - - if (role_oid == InvalidOid) + if (strcmp(grantee_name, "public") != 0 && role_oid == InvalidOid) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("Cannot find the principal '%s', because it does not exist or you do not have permission.", grantee_name))); - if (pg_namespace_ownercheck(schemaOid, role_oid) || is_member_of_role(role_oid, datdba) || grantee_is_db_owner) + if ((strcmp(rolname, user) == 0) || pg_namespace_ownercheck(schemaOid, role_oid) || is_member_of_role(role_oid, get_sysadmin_oid()) || grantee_is_db_owner) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("Cannot grant, deny, or revoke permissions to sa, dbo, entity owner, information_schema, sys, or yourself."))); + if (!is_member_of_role(GetSessionUserId(), get_sysadmin_oid()) && !login_is_db_owner && !pg_namespace_ownercheck(schemaOid, GetUserId())) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("Cannot find the schema \"%s\", because it does not exist or you do not have permission.", stmt->schema_name))); + parsetree_list = gen_grantschema_subcmds(schema_name, rolname, stmt->is_grant, stmt->with_grant_option, priv_name); /* Run all subcommands */ foreach(parsetree_item, parsetree_list) diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 10297b0c90e..5a657ccc79d 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3641,8 +3641,14 @@ bbf_ProcessUtility(PlannedStmt *pstmt, char *permissions[] = {"select", "insert", "update", "references", "delete"}; for(i = 0; i < 5; i++) { - if ((rol_spec->rolename != NULL) && !check_bbf_schema_for_entry(logical_schema, obj, permissions[i], rol_spec->rolename)) - add_entry_to_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename, obj_type); + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + { + /* + * If object permission doesn't exist already, add an entry to the catalog. + */ + if (!check_bbf_schema_for_entry(logical_schema, obj, permissions[i], rol_spec->rolename)) + add_entry_to_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename, obj_type); + } } } break; @@ -3657,10 +3663,19 @@ bbf_ProcessUtility(PlannedStmt *pstmt, char *permissions[] = {"select", "insert", "update", "references", "delete"}; for(i = 0; i < 5; i++) { - if ((rol_spec->rolename != NULL) && check_bbf_schema_for_entry(logical_schema, "ALL", permissions[i], rol_spec->rolename) && !has_schema_perms) - has_schema_perms = true; - if ((rol_spec->rolename != NULL) && check_bbf_schema_for_entry(logical_schema, obj, permissions[i], rol_spec->rolename)) - del_from_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename); + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + { + /* + * 1. If only schema permission exists, don't revoke any permission. + * 2. If only object permission exists, delete entry from the catalog and revoke permission. + * 3. If both schema and object permission exist, don't revoke any permission but delete object + * entry from the catalog. + */ + if (check_bbf_schema_for_entry(logical_schema, "ALL", permissions[i], rol_spec->rolename) && !has_schema_perms) + has_schema_perms = true; + if (check_bbf_schema_for_entry(logical_schema, obj, permissions[i], rol_spec->rolename)) + del_from_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename); + } } if (has_schema_perms) return; @@ -3687,8 +3702,16 @@ bbf_ProcessUtility(PlannedStmt *pstmt, foreach(lc, grant->grantees) { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - if ((ap->cols == NULL) && (rol_spec->rolename != NULL) && !check_bbf_schema_for_entry(logical_schema, obj, ap->priv_name, rol_spec->rolename)) - add_entry_to_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename, obj_type); + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + { + /* + * Don't add an entry, if the permission is granted on column list. + */ + if (ap->cols != NULL) + break; + if (!check_bbf_schema_for_entry(logical_schema, obj, ap->priv_name, rol_spec->rolename)) + add_entry_to_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename, obj_type); + } } } else @@ -3700,17 +3723,25 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. */ - if ((logical_schema != NULL) && (rol_spec->rolename != NULL) && !check_bbf_schema_for_entry(logical_schema, "ALL", ap->priv_name, rol_spec->rolename)) + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) { - if (prev_ProcessUtility) - prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, - queryEnv, dest, qc); - else - standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, + if ((logical_schema != NULL) && !check_bbf_schema_for_entry(logical_schema, "ALL", ap->priv_name, rol_spec->rolename)) + { + if (prev_ProcessUtility) + prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, queryEnv, dest, qc); + else + standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, + queryEnv, dest, qc); + } + /* + * Don't remove an entry, if the permission is revoked on column list. + */ + if (ap->cols != NULL) + break; + if (check_bbf_schema_for_entry(logical_schema, obj, ap->priv_name, rol_spec->rolename)) + del_from_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename); } - if ((ap->cols == NULL) && (rol_spec->rolename != NULL) && check_bbf_schema_for_entry(logical_schema, obj, ap->priv_name, rol_spec->rolename)) - del_from_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename); } } } @@ -3761,8 +3792,11 @@ bbf_ProcessUtility(PlannedStmt *pstmt, foreach(lc, grant->grantees) { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - if ((rol_spec->rolename != NULL) && !check_bbf_schema_for_entry(logicalschema, funcname, "execute", rol_spec->rolename)) - add_entry_to_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename, obj_type); + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + { + if (!check_bbf_schema_for_entry(logicalschema, funcname, "execute", rol_spec->rolename)) + add_entry_to_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename, obj_type); + } } break; } @@ -3772,10 +3806,13 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); bool has_schema_perms = false; - if ((rol_spec->rolename != NULL) && check_bbf_schema_for_entry(logicalschema, "ALL", "execute", rol_spec->rolename) && !has_schema_perms) - has_schema_perms = true; - if ((rol_spec->rolename != NULL) && check_bbf_schema_for_entry(logicalschema, funcname, "execute", rol_spec->rolename)) - del_from_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename); + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + { + if (check_bbf_schema_for_entry(logicalschema, "ALL", "execute", rol_spec->rolename) && !has_schema_perms) + has_schema_perms = true; + if (check_bbf_schema_for_entry(logicalschema, funcname, "execute", rol_spec->rolename)) + del_from_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename); + } if (has_schema_perms) return; } @@ -3798,9 +3835,14 @@ bbf_ProcessUtility(PlannedStmt *pstmt, foreach(lc, grant->grantees) { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - /* Don't store a row in catalog, if permission is granted for column */ - if ((rol_spec->rolename != NULL) && !check_bbf_schema_for_entry(logicalschema, funcname, ap->priv_name, rol_spec->rolename)) - add_entry_to_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename, obj_type); + /* Add an entry to the catalog, if an entry doesn't exist already. */ + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + { + if (!check_bbf_schema_for_entry(logicalschema, funcname, ap->priv_name, rol_spec->rolename)) + { + add_entry_to_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename, obj_type); + } + } } } else @@ -3812,8 +3854,10 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. */ - if ((rol_spec->rolename != NULL) && !check_bbf_schema_for_entry(logicalschema, "ALL", ap->priv_name, rol_spec->rolename)) + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) { + if (!check_bbf_schema_for_entry(logicalschema, "ALL", ap->priv_name, rol_spec->rolename)) + { /* Execute REVOKE statement. */ if (prev_ProcessUtility) prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, @@ -3821,9 +3865,11 @@ bbf_ProcessUtility(PlannedStmt *pstmt, else standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, queryEnv, dest, qc); + } + /* Remove an entry from the catalog, if it exists. */ + if (check_bbf_schema_for_entry(logicalschema, funcname, ap->priv_name, rol_spec->rolename)) + del_from_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename); } - if ((rol_spec->rolename != NULL) && check_bbf_schema_for_entry(logicalschema, funcname, ap->priv_name, rol_spec->rolename)) - del_from_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename); } } } diff --git a/contrib/babelfishpg_tsql/src/pltsql_utils.c b/contrib/babelfishpg_tsql/src/pltsql_utils.c index df3c57df719..d635d81ab7a 100644 --- a/contrib/babelfishpg_tsql/src/pltsql_utils.c +++ b/contrib/babelfishpg_tsql/src/pltsql_utils.c @@ -1036,7 +1036,10 @@ update_GrantStmt(Node *n, const char *object, const char *obj_schema, const char if (grantee && stmt->grantees) { RoleSpec *tmp = (RoleSpec *) llast(stmt->grantees); - + if (strcmp(grantee, "public") == 0) + { + tmp->roletype = ROLESPEC_PUBLIC; + } tmp->rolename = pstrdup(grantee); } diff --git a/contrib/babelfishpg_tsql/src/tsqlIface.cpp b/contrib/babelfishpg_tsql/src/tsqlIface.cpp index 695b28dc468..ae4704c293c 100644 --- a/contrib/babelfishpg_tsql/src/tsqlIface.cpp +++ b/contrib/babelfishpg_tsql/src/tsqlIface.cpp @@ -5527,6 +5527,11 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) char *grantee_name = pstrdup(downcase_truncate_identifier(id_str.c_str(), id_str.length(), true)); grantee_list = lappend(grantee_list, grantee_name); } + if (prin->PUBLIC()) + { + char *grantee_name = (char *)"public"; + grantee_list = lappend(grantee_list, grantee_name); + } } result->grantees = grantee_list; return (PLtsql_stmt *) result; @@ -5555,6 +5560,11 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) char *grantee_name = pstrdup(downcase_truncate_identifier(id_str.c_str(), id_str.length(), true)); grantee_list = lappend(grantee_list, grantee_name); } + if (prin->PUBLIC()) + { + char *grantee_name = (char *)"public"; + grantee_list = lappend(grantee_list, grantee_name); + } } result->grantees = grantee_list; return (PLtsql_stmt *) result; @@ -5585,6 +5595,11 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) char *grantee_name = pstrdup(downcase_truncate_identifier(id_str.c_str(), id_str.length(), true)); grantee_list = lappend(grantee_list, grantee_name); } + if (prin->PUBLIC()) + { + char *grantee_name = (char *)"public"; + grantee_list = lappend(grantee_list, grantee_name); + } } List *privilege_list = NIL; for (auto perm: ctx->grant_statement()->permissions()->permission()) @@ -5637,6 +5652,11 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) char *grantee_name = pstrdup(downcase_truncate_identifier(id_str.c_str(), id_str.length(), true)); grantee_list = lappend(grantee_list, grantee_name); } + if (prin->PUBLIC()) + { + char *grantee_name = (char *)"public"; + grantee_list = lappend(grantee_list, grantee_name); + } } List *privilege_list = NIL; for (auto perm: ctx->revoke_statement()->permissions()->permission()) diff --git a/contrib/babelfishpg_tsql/src/tsqlUnsupportedFeatureHandler.cpp b/contrib/babelfishpg_tsql/src/tsqlUnsupportedFeatureHandler.cpp index 29cdc5ac320..64a4d12f89e 100644 --- a/contrib/babelfishpg_tsql/src/tsqlUnsupportedFeatureHandler.cpp +++ b/contrib/babelfishpg_tsql/src/tsqlUnsupportedFeatureHandler.cpp @@ -1704,7 +1704,8 @@ void TsqlUnsupportedFeatureHandlerImpl::checkSupportedGrantStmt(TSqlParser::Gran continue; else { - unsupported_feature = "GRANT PERMISSION " + perm->getText(); + unsupported_feature = "GRANT PERMISSION " + ::getFullText(single_perm); + std::transform(unsupported_feature.begin(), unsupported_feature.end(), unsupported_feature.begin(), ::toupper); handle(INSTR_UNSUPPORTED_TSQL_REVOKE_STMT, unsupported_feature.c_str(), getLineAndPos(perm)); } } @@ -1719,6 +1720,7 @@ void TsqlUnsupportedFeatureHandlerImpl::checkSupportedGrantStmt(TSqlParser::Gran if (obj_type && !(obj_type->OBJECT() || obj_type->SCHEMA())) { unsupported_feature = "GRANT ON " + obj_type->getText(); + std::transform(unsupported_feature.begin(), unsupported_feature.end(), unsupported_feature.begin(), ::toupper); handle(INSTR_UNSUPPORTED_TSQL_REVOKE_STMT, unsupported_feature.c_str(), getLineAndPos(obj_type)); } } @@ -1798,7 +1800,8 @@ void TsqlUnsupportedFeatureHandlerImpl::checkSupportedRevokeStmt(TSqlParser::Rev continue; else { - unsupported_feature = "REVOKE PERMISSION " + perm->getText(); + unsupported_feature = "REVOKE PERMISSION " + ::getFullText(single_perm); + std::transform(unsupported_feature.begin(), unsupported_feature.end(), unsupported_feature.begin(), ::toupper); handle(INSTR_UNSUPPORTED_TSQL_REVOKE_STMT, unsupported_feature.c_str(), getLineAndPos(perm)); } } @@ -1813,6 +1816,7 @@ void TsqlUnsupportedFeatureHandlerImpl::checkSupportedRevokeStmt(TSqlParser::Rev if (obj_type && !(obj_type->OBJECT() || obj_type->SCHEMA())) { unsupported_feature = "REVOKE ON " + obj_type->getText(); + std::transform(unsupported_feature.begin(), unsupported_feature.end(), unsupported_feature.begin(), ::toupper); handle(INSTR_UNSUPPORTED_TSQL_REVOKE_STMT, unsupported_feature.c_str(), getLineAndPos(obj_type)); } } diff --git a/test/JDBC/expected/BABEL-GRANT.out b/test/JDBC/expected/BABEL-GRANT.out index d3f48b241fe..2a59cf3dead 100644 --- a/test/JDBC/expected/BABEL-GRANT.out +++ b/test/JDBC/expected/BABEL-GRANT.out @@ -174,7 +174,7 @@ GO REVOKE SELECT ON SCHEMA::scm FROM guest; GO -GRANT SHOWPLAN ON OBJECT::t1 TO guest; -- unsupported permission +GRANT showplan ON OBJECT::t1 TO guest; -- unsupported permission GO ~~ERROR (Code: 33557097)~~ @@ -188,20 +188,48 @@ GO ~~ERROR (Message: 'REVOKE PERMISSION SHOWPLAN' is not currently supported in Babelfish)~~ -GRANT ALL ON SCHEMA::scm TO guest; -- unsupported class +GRANT ALL ON SCHEMA::scm TO guest; GO ~~ERROR (Code: 33557097)~~ ~~ERROR (Message: The all permission has been deprecated and is not available for this class of entity.)~~ -REVOKE ALL ON SCHEMA::scm TO guest; -- unsupported class +REVOKE ALL ON SCHEMA::scm TO guest; GO ~~ERROR (Code: 33557097)~~ ~~ERROR (Message: The all permission has been deprecated and is not available for this class of entity.)~~ +GRANT create table ON OBJECT::t1 TO guest; -- unsupported permission +GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: 'GRANT PERMISSION CREATE TABLE' is not currently supported in Babelfish)~~ + + +REVOKE create table ON OBJECT::t2 FROM alogin; -- unsupported permission +GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: 'REVOKE PERMISSION CREATE TABLE' is not currently supported in Babelfish)~~ + + +GRANT SELECT ON table::t1 TO guest; -- unsupported object +GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: 'GRANT ON TABLE' is not currently supported in Babelfish)~~ + + +REVOKE SELECT ON table::t1 FROM guest; -- unsupported object +GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: 'REVOKE ON TABLE' is not currently supported in Babelfish)~~ + + GRANT ALL ON OBJECT::t1 TO guest WITH GRANT OPTION AS superuser; GO ~~ERROR (Code: 33557097)~~ diff --git a/test/JDBC/expected/GRANT_SCHEMA.out b/test/JDBC/expected/GRANT_SCHEMA.out index 1891d685978..87418008cfd 100644 --- a/test/JDBC/expected/GRANT_SCHEMA.out +++ b/test/JDBC/expected/GRANT_SCHEMA.out @@ -51,6 +51,36 @@ go CREATE FUNCTION babel_4344_s1.babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.objects) END go +grant select on schema::babel_4344_s1 to public; +go + +-- tsql user=babel_4344_l1 password=12345678 +use babel_4344_d1; +go + +-- User has select privileges, tables and views be accessible +select * from babel_4344_s1.babel_4344_t1 +go +~~START~~ +int +~~END~~ + +select * from babel_4344_s1.babel_4344_v1; +go +~~START~~ +int +2 +~~END~~ + +use master; +go + +-- tsql +use babel_4344_d1; +go +revoke select on schema::babel_4344_s1 from public; +go + -- tsql user=babel_4344_l1 password=12345678 use babel_4344_d1; go @@ -135,7 +165,8 @@ grant execute on babel_4344_p1 to babel_4344_u1; go grant execute on babel_4344_s1.babel_4344_p1 to babel_4344_u1; go -grant execute on babel_4344_f1 to babel_4344_u1; +-- Mixed case +grant Execute on Babel_4344_F1 to babel_4344_u1; go grant execute on babel_4344_s1.babel_4344_f1 to babel_4344_u1; go @@ -154,6 +185,18 @@ go grant select on schema::babel_4344_s2 to guest; -- should pass go +grant select on schema::"" to guest; -- should fail +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: An object or column name is missing or empty. For SELECT INTO statements, verify each column has a name. For other statements, look for empty alias names. Aliases defined as "" or [] are not allowed. Change the alias to a valid name.)~~ + +grant select on schema::[] to guest; -- should fail +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: An object or column name is missing or empty. For SELECT INTO statements, verify each column has a name. For other statements, look for empty alias names. Aliases defined as "" or [] are not allowed. Change the alias to a valid name.)~~ + -- tsql user=babel_4344_l1 password=12345678 -- User has OBJECT privileges, should be accessible. @@ -242,7 +285,7 @@ grant select on schema::babel_4344_s1 to babel_4344_u1; -- should fail go ~~ERROR (Code: 33557097)~~ -~~ERROR (Message: Cannot find the schema "babel_4344_s1", because it does not exist or you do not have permission.)~~ +~~ERROR (Message: Cannot grant, deny, or revoke permissions to sa, dbo, entity owner, information_schema, sys, or yourself.)~~ use master; go diff --git a/test/JDBC/input/BABEL-GRANT.sql b/test/JDBC/input/BABEL-GRANT.sql index 3e0155ea283..8388f782ffd 100644 --- a/test/JDBC/input/BABEL-GRANT.sql +++ b/test/JDBC/input/BABEL-GRANT.sql @@ -164,16 +164,28 @@ GO REVOKE SELECT ON SCHEMA::scm FROM guest; GO -GRANT SHOWPLAN ON OBJECT::t1 TO guest; -- unsupported permission +GRANT showplan ON OBJECT::t1 TO guest; -- unsupported permission GO REVOKE SHOWPLAN ON OBJECT::t2 TO alogin; -- unsupported permission GO -GRANT ALL ON SCHEMA::scm TO guest; -- unsupported class +GRANT ALL ON SCHEMA::scm TO guest; GO -REVOKE ALL ON SCHEMA::scm TO guest; -- unsupported class +REVOKE ALL ON SCHEMA::scm TO guest; +GO + +GRANT create table ON OBJECT::t1 TO guest; -- unsupported permission +GO + +REVOKE create table ON OBJECT::t2 FROM alogin; -- unsupported permission +GO + +GRANT SELECT ON table::t1 TO guest; -- unsupported object +GO + +REVOKE SELECT ON table::t1 FROM guest; -- unsupported object GO GRANT ALL ON OBJECT::t1 TO guest WITH GRANT OPTION AS superuser; diff --git a/test/JDBC/input/GRANT_SCHEMA.mix b/test/JDBC/input/GRANT_SCHEMA.mix index 1572bea803b..fee9245050c 100644 --- a/test/JDBC/input/GRANT_SCHEMA.mix +++ b/test/JDBC/input/GRANT_SCHEMA.mix @@ -51,6 +51,27 @@ go CREATE FUNCTION babel_4344_s1.babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.objects) END go +grant select on schema::babel_4344_s1 to public; +go + +-- tsql user=babel_4344_l1 password=12345678 +use babel_4344_d1; +go + +-- User has select privileges, tables and views be accessible +select * from babel_4344_s1.babel_4344_t1 +go +select * from babel_4344_s1.babel_4344_v1; +go +use master; +go + +-- tsql +use babel_4344_d1; +go +revoke select on schema::babel_4344_s1 from public; +go + -- tsql user=babel_4344_l1 password=12345678 use babel_4344_d1; go @@ -99,7 +120,8 @@ grant execute on babel_4344_p1 to babel_4344_u1; go grant execute on babel_4344_s1.babel_4344_p1 to babel_4344_u1; go -grant execute on babel_4344_f1 to babel_4344_u1; +-- Mixed case +grant Execute on Babel_4344_F1 to babel_4344_u1; go grant execute on babel_4344_s1.babel_4344_f1 to babel_4344_u1; go @@ -110,6 +132,10 @@ grant select on schema::babel_4344_s2 to jdbc_user; -- should fail go grant select on schema::babel_4344_s2 to guest; -- should pass go +grant select on schema::"" to guest; -- should fail +go +grant select on schema::[] to guest; -- should fail +go -- tsql user=babel_4344_l1 password=12345678 -- User has OBJECT privileges, should be accessible. From c765748bd0fe96224ca5ea2b8e561768f8bf8275 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 08:36:47 +0000 Subject: [PATCH 02/14] Add collate --- contrib/babelfishpg_tsql/sql/ownership.sql | 10 +++++----- .../sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql | 10 +++++----- test/JDBC/expected/GRANT_SCHEMA.out | 8 ++++---- test/JDBC/input/GRANT_SCHEMA.mix | 8 ++++---- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/contrib/babelfishpg_tsql/sql/ownership.sql b/contrib/babelfishpg_tsql/sql/ownership.sql index ee3a5446a43..09abeba9b90 100644 --- a/contrib/babelfishpg_tsql/sql/ownership.sql +++ b/contrib/babelfishpg_tsql/sql/ownership.sql @@ -17,11 +17,11 @@ GRANT SELECT on sys.babelfish_sysdatabases TO PUBLIC; -- BABELFISH_SCHEMA_PERMISSIONS CREATE TABLE sys.babelfish_schema_permissions ( dbid smallint NOT NULL, - schema_name NAME NOT NULL, - object_name NAME NOT NULL, - permission NAME NOT NULL, - grantee NAME NOT NULL, - object_type NAME, + schema_name NAME NOT NULL COLLATE "C", + object_name NAME NOT NULL COLLATE "C", + permission NAME NOT NULL COLLATE "C", + grantee NAME NOT NULL COLLATE "C", + object_type NAME COLLATE "C", PRIMARY KEY(dbid, schema_name, object_name, permission, grantee) ); diff --git a/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql b/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql index 05f0aadbbc1..e4330dd37e2 100644 --- a/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql +++ b/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql @@ -892,11 +892,11 @@ LANGUAGE plpgsql STABLE; -- BABELFISH_SCHEMA_PERMISSIONS CREATE TABLE IF NOT EXISTS sys.babelfish_schema_permissions ( dbid smallint NOT NULL, - schema_name NAME NOT NULL, - object_name NAME NOT NULL, - permission NAME NOT NULL, - grantee NAME NOT NULL, - object_type NAME, + schema_name NAME NOT NULL COLLATE "C", + object_name NAME NOT NULL COLLATE "C", + permission NAME NOT NULL COLLATE "C", + grantee NAME NOT NULL COLLATE "C", + object_type NAME COLLATE "C", PRIMARY KEY(dbid, schema_name, object_name, permission, grantee) ); diff --git a/test/JDBC/expected/GRANT_SCHEMA.out b/test/JDBC/expected/GRANT_SCHEMA.out index 87418008cfd..aedf72a578e 100644 --- a/test/JDBC/expected/GRANT_SCHEMA.out +++ b/test/JDBC/expected/GRANT_SCHEMA.out @@ -51,7 +51,7 @@ go CREATE FUNCTION babel_4344_s1.babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.objects) END go -grant select on schema::babel_4344_s1 to public; +grant SELECT on schema::babel_4344_S1 to public; go -- tsql user=babel_4344_l1 password=12345678 @@ -147,9 +147,9 @@ go -- GRANT OBJECT privilege use babel_4344_d1; go -grant select on babel_4344_t1 to babel_4344_u1; +grant SELECT on babel_4344_t1 to BABEL_4344_U1; go -grant select on babel_4344_s1.babel_4344_t1 to babel_4344_u1; +grant SELECT on babel_4344_s1.babel_4344_t1 to babel_4344_u1; go grant all on babel_4344_s1.babel_4344_t1 to babel_4344_u1; go @@ -183,7 +183,7 @@ go ~~ERROR (Message: Cannot find the principal 'jdbc_user', because it does not exist or you do not have permission.)~~ -grant select on schema::babel_4344_s2 to guest; -- should pass +grant SELECT on schema::babel_4344_s2 to guest; -- should pass go grant select on schema::"" to guest; -- should fail go diff --git a/test/JDBC/input/GRANT_SCHEMA.mix b/test/JDBC/input/GRANT_SCHEMA.mix index fee9245050c..7794b72ff72 100644 --- a/test/JDBC/input/GRANT_SCHEMA.mix +++ b/test/JDBC/input/GRANT_SCHEMA.mix @@ -51,7 +51,7 @@ go CREATE FUNCTION babel_4344_s1.babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.objects) END go -grant select on schema::babel_4344_s1 to public; +grant SELECT on schema::babel_4344_S1 to public; go -- tsql user=babel_4344_l1 password=12345678 @@ -102,9 +102,9 @@ go -- GRANT OBJECT privilege use babel_4344_d1; go -grant select on babel_4344_t1 to babel_4344_u1; +grant SELECT on babel_4344_t1 to BABEL_4344_U1; go -grant select on babel_4344_s1.babel_4344_t1 to babel_4344_u1; +grant SELECT on babel_4344_s1.babel_4344_t1 to babel_4344_u1; go grant all on babel_4344_s1.babel_4344_t1 to babel_4344_u1; go @@ -130,7 +130,7 @@ grant select on schema::babel_4344_s2 to babel_4344_u1; -- should fail go grant select on schema::babel_4344_s2 to jdbc_user; -- should fail go -grant select on schema::babel_4344_s2 to guest; -- should pass +grant SELECT on schema::babel_4344_s2 to guest; -- should pass go grant select on schema::"" to guest; -- should fail go From 72155d587b8faecf7f5b6dfad49377dd15c1f81e Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 09:19:59 +0000 Subject: [PATCH 03/14] Update collation to sys.database_default --- contrib/babelfishpg_tsql/sql/ownership.sql | 10 +++++----- .../sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/contrib/babelfishpg_tsql/sql/ownership.sql b/contrib/babelfishpg_tsql/sql/ownership.sql index 09abeba9b90..a632a918157 100644 --- a/contrib/babelfishpg_tsql/sql/ownership.sql +++ b/contrib/babelfishpg_tsql/sql/ownership.sql @@ -17,11 +17,11 @@ GRANT SELECT on sys.babelfish_sysdatabases TO PUBLIC; -- BABELFISH_SCHEMA_PERMISSIONS CREATE TABLE sys.babelfish_schema_permissions ( dbid smallint NOT NULL, - schema_name NAME NOT NULL COLLATE "C", - object_name NAME NOT NULL COLLATE "C", - permission NAME NOT NULL COLLATE "C", - grantee NAME NOT NULL COLLATE "C", - object_type NAME COLLATE "C", + schema_name NAME NOT NULL COLLATE sys.database_default, + object_name NAME NOT NULL COLLATE sys.database_default, + permission NAME NOT NULL COLLATE sys.database_default, + grantee NAME NOT NULL COLLATE sys.database_default, + object_type NAME COLLATE sys.database_default, PRIMARY KEY(dbid, schema_name, object_name, permission, grantee) ); diff --git a/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql b/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql index e4330dd37e2..cdf3e45516c 100644 --- a/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql +++ b/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql @@ -892,11 +892,11 @@ LANGUAGE plpgsql STABLE; -- BABELFISH_SCHEMA_PERMISSIONS CREATE TABLE IF NOT EXISTS sys.babelfish_schema_permissions ( dbid smallint NOT NULL, - schema_name NAME NOT NULL COLLATE "C", - object_name NAME NOT NULL COLLATE "C", - permission NAME NOT NULL COLLATE "C", - grantee NAME NOT NULL COLLATE "C", - object_type NAME COLLATE "C", + schema_name NAME NOT NULL COLLATE sys.database_default, + object_name NAME NOT NULL COLLATE sys.database_default, + permission NAME NOT NULL COLLATE sys.database_default, + grantee NAME NOT NULL COLLATE sys.database_default, + object_type NAME COLLATE sys.database_default, PRIMARY KEY(dbid, schema_name, object_name, permission, grantee) ); From 26f14088ef08056a390b2e1c3eccc8cdbcc50eb3 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 10:45:43 +0000 Subject: [PATCH 04/14] Address comments --- contrib/babelfishpg_tsql/src/catalog.c | 170 ++++++++++++++++--------- test/JDBC/expected/GRANT_SCHEMA.out | 62 +++++++++ test/JDBC/input/GRANT_SCHEMA.mix | 48 +++++++ 3 files changed, 223 insertions(+), 57 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/catalog.c b/contrib/babelfishpg_tsql/src/catalog.c index 8f1f9bdc2a2..56c6a44ac7d 100644 --- a/contrib/babelfishpg_tsql/src/catalog.c +++ b/contrib/babelfishpg_tsql/src/catalog.c @@ -2885,41 +2885,54 @@ check_bbf_schema_for_entry(const char *schema_name, { Relation bbf_schema_rel; HeapTuple tuple_bbf_schema; - ScanKeyData key[5]; - TableScanDesc scan; + ScanKeyData scanKey[5]; + SysScanDesc scan; bool catalog_entry_exists = false; int16 dbid = get_cur_db_id(); bbf_schema_rel = table_open(get_bbf_schema_perms_oid(), AccessShareLock); - ScanKeyInit(&key[0], + ScanKeyInit(&scanKey[0], Anum_bbf_schema_perms_dbid, BTEqualStrategyNumber, F_INT2EQ, Int16GetDatum(dbid)); - ScanKeyInit(&key[1], + ScanKeyEntryInitialize(&scanKey[1], 0, Anum_bbf_schema_perms_schema_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(schema_name)); - ScanKeyInit(&key[2], + ScanKeyEntryInitialize(&scanKey[2], 0, Anum_bbf_schema_perms_object_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(object_name)); - ScanKeyInit(&key[3], + ScanKeyEntryInitialize(&scanKey[3], 0, Anum_bbf_schema_perms_permission, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(permission)); - ScanKeyInit(&key[4], + ScanKeyEntryInitialize(&scanKey[4], 0, Anum_bbf_schema_perms_grantee, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(grantee)); + scan = systable_beginscan(bbf_schema_rel, + get_bbf_schema_perms_idx_oid(), + true, NULL, 5, scanKey); - scan = table_beginscan_catalog(bbf_schema_rel, 5, key); - - tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection); + tuple_bbf_schema = systable_getnext(scan); if (HeapTupleIsValid(tuple_bbf_schema)) catalog_entry_exists = true; - table_endscan(scan); + systable_endscan(scan); table_close(bbf_schema_rel, AccessShareLock); return catalog_entry_exists; } @@ -2932,7 +2945,7 @@ check_bbf_schema_for_schema(const char *schema_name, Relation bbf_schema_rel; HeapTuple tuple_bbf_schema; ScanKeyData key[4]; - TableScanDesc scan; + SysScanDesc scan; bool catalog_entry_exists = false; int16 dbid = get_cur_db_id(); @@ -2942,26 +2955,37 @@ check_bbf_schema_for_schema(const char *schema_name, Anum_bbf_schema_perms_dbid, BTEqualStrategyNumber, F_INT2EQ, Int16GetDatum(dbid)); - ScanKeyInit(&key[1], + ScanKeyEntryInitialize(&key[1], 0, Anum_bbf_schema_perms_schema_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(schema_name)); - ScanKeyInit(&key[2], + ScanKeyEntryInitialize(&key[2], 0, Anum_bbf_schema_perms_object_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(object_name)); - ScanKeyInit(&key[3], + ScanKeyEntryInitialize(&key[3], 0, Anum_bbf_schema_perms_permission, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(permission)); - scan = table_beginscan_catalog(bbf_schema_rel, 4, key); + scan = systable_beginscan(bbf_schema_rel, + get_bbf_schema_perms_idx_oid(), + true, NULL, 4, key); - tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection); + tuple_bbf_schema = systable_getnext(scan); if (HeapTupleIsValid(tuple_bbf_schema)) catalog_entry_exists = true; - table_endscan(scan); + systable_endscan(scan); table_close(bbf_schema_rel, AccessShareLock); return catalog_entry_exists; } @@ -2974,41 +2998,54 @@ del_from_bbf_schema(const char *schema_name, { Relation bbf_schema_rel; HeapTuple tuple_bbf_schema; - ScanKeyData key[5]; - TableScanDesc scan; + ScanKeyData scanKey[5]; + SysScanDesc scan; int16 dbid = get_cur_db_id(); bbf_schema_rel = table_open(get_bbf_schema_perms_oid(), RowExclusiveLock); - ScanKeyInit(&key[0], + ScanKeyInit(&scanKey[0], Anum_bbf_schema_perms_dbid, BTEqualStrategyNumber, F_INT2EQ, Int16GetDatum(dbid)); - ScanKeyInit(&key[1], + ScanKeyEntryInitialize(&scanKey[1], 0, Anum_bbf_schema_perms_schema_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(schema_name)); - ScanKeyInit(&key[2], + ScanKeyEntryInitialize(&scanKey[2], 0, Anum_bbf_schema_perms_object_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(object_name)); - ScanKeyInit(&key[3], + ScanKeyEntryInitialize(&scanKey[3], 0, Anum_bbf_schema_perms_permission, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(permission)); - ScanKeyInit(&key[4], + ScanKeyEntryInitialize(&scanKey[4], 0, Anum_bbf_schema_perms_grantee, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(grantee)); + scan = systable_beginscan(bbf_schema_rel, + get_bbf_schema_perms_idx_oid(), + true, NULL, 5, scanKey); - scan = table_beginscan_catalog(bbf_schema_rel, 5, key); - - tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection); + tuple_bbf_schema = systable_getnext(scan); if (HeapTupleIsValid(tuple_bbf_schema)) CatalogTupleDelete(bbf_schema_rel, &tuple_bbf_schema->t_self); - table_endscan(scan); + systable_endscan(scan); table_close(bbf_schema_rel, RowExclusiveLock); CommandCounterIncrement(); @@ -3035,9 +3072,12 @@ clean_up_bbf_schema(const char *schema_name, Anum_bbf_schema_perms_dbid, BTEqualStrategyNumber, F_INT2EQ, Int16GetDatum(dbid)); - ScanKeyInit(&scanKey[1], + ScanKeyEntryInitialize(&scanKey[1], 0, Anum_bbf_schema_perms_schema_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(schema_name)); scan = systable_beginscan(bbf_schema_rel, get_bbf_schema_perms_idx_oid(), @@ -3050,13 +3090,19 @@ clean_up_bbf_schema(const char *schema_name, Anum_bbf_schema_perms_dbid, BTEqualStrategyNumber, F_INT2EQ, Int16GetDatum(dbid)); - ScanKeyInit(&scanKey[1], + ScanKeyEntryInitialize(&scanKey[1], 0, Anum_bbf_schema_perms_schema_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(schema_name)); - ScanKeyInit(&scanKey[2], + ScanKeyEntryInitialize(&scanKey[2], 0, Anum_bbf_schema_perms_object_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(object_name)); scan = systable_beginscan(bbf_schema_rel, get_bbf_schema_perms_idx_oid(), @@ -3079,7 +3125,7 @@ grant_perms_to_objects_in_schema(const char *schema_name, const char *permission, const char *grantee) { - TableScanDesc scan; + SysScanDesc scan; Relation bbf_schema_rel; HeapTuple tuple_bbf_schema; const char *object_name; @@ -3095,21 +3141,31 @@ grant_perms_to_objects_in_schema(const char *schema_name, Anum_bbf_schema_perms_dbid, BTEqualStrategyNumber, F_INT2EQ, Int16GetDatum(dbid)); - ScanKeyInit(&scanKey[1], + ScanKeyEntryInitialize(&scanKey[1], 0, Anum_bbf_schema_perms_schema_name, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(schema_name)); - ScanKeyInit(&scanKey[2], + ScanKeyEntryInitialize(&scanKey[2], 0, Anum_bbf_schema_perms_permission, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(permission)); - ScanKeyInit(&scanKey[3], + ScanKeyEntryInitialize(&scanKey[3], 0, Anum_bbf_schema_perms_grantee, - BTEqualStrategyNumber, F_NAMEEQ, + BTEqualStrategyNumber, + InvalidOid, + tsql_get_server_collation_oid_internal(false), + F_NAMEEQ, CStringGetDatum(grantee)); - scan = table_beginscan_catalog(bbf_schema_rel, 4, scanKey); - tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection); + scan = systable_beginscan(bbf_schema_rel, get_bbf_schema_perms_idx_oid(), + true, NULL, 4, scanKey); + tuple_bbf_schema = systable_getnext(scan); while (HeapTupleIsValid(tuple_bbf_schema)) { @@ -3162,9 +3218,9 @@ grant_perms_to_objects_in_schema(const char *schema_name, /* make sure later steps can see the object created here */ CommandCounterIncrement(); } - tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection); + tuple_bbf_schema = systable_getnext(scan); } - table_endscan(scan); + systable_endscan(scan); table_close(bbf_schema_rel, AccessShareLock); } diff --git a/test/JDBC/expected/GRANT_SCHEMA.out b/test/JDBC/expected/GRANT_SCHEMA.out index aedf72a578e..5acf305efc3 100644 --- a/test/JDBC/expected/GRANT_SCHEMA.out +++ b/test/JDBC/expected/GRANT_SCHEMA.out @@ -12,9 +12,18 @@ go create user babel_4344_u1 for login babel_4344_l1; go +create login αιώνια with password = '12345678' +go + +create user αιώνια for login αιώνια; +go + create schema babel_4344_s1; go +create schema αγάπη; +go + create schema babel_4344_s2 authorization babel_4344_u1; go @@ -27,6 +36,9 @@ go create table babel_4344_s2.babel_4344_t1(a int); go +create table αγάπη.abc(a int); +go + create table babel_4344_t3(a int, b int); go @@ -54,6 +66,23 @@ go grant SELECT on schema::babel_4344_S1 to public; go +grant select on schema::αγάπη to αιώνια; +go + +-- tsql user=αιώνια password=12345678 +use babel_4344_d1; +go + +select * from αγάπη.abc; +go +~~START~~ +int +~~END~~ + + +use master; +go + -- tsql user=babel_4344_l1 password=12345678 use babel_4344_d1; go @@ -640,9 +669,18 @@ go drop schema babel_4344_s2; go +drop table αγάπη.abc; +go + +drop schema αγάπη; +go + drop user babel_4344_u1; go +drop user αιώνια; +go + use master; go @@ -672,3 +710,27 @@ void -- tsql drop login babel_4344_l1; go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'αιώνια' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go +~~START~~ +bool +t +~~END~~ + + +-- Wait to sync with another session +SELECT pg_sleep(1); +go +~~START~~ +void + +~~END~~ + + +-- tsql +drop login αιώνια; +go diff --git a/test/JDBC/input/GRANT_SCHEMA.mix b/test/JDBC/input/GRANT_SCHEMA.mix index 7794b72ff72..a6510a9ed96 100644 --- a/test/JDBC/input/GRANT_SCHEMA.mix +++ b/test/JDBC/input/GRANT_SCHEMA.mix @@ -12,9 +12,18 @@ go create user babel_4344_u1 for login babel_4344_l1; go +create login αιώνια with password = '12345678' +go + +create user αιώνια for login αιώνια; +go + create schema babel_4344_s1; go +create schema αγάπη; +go + create schema babel_4344_s2 authorization babel_4344_u1; go @@ -27,6 +36,9 @@ go create table babel_4344_s2.babel_4344_t1(a int); go +create table αγάπη.abc(a int); +go + create table babel_4344_t3(a int, b int); go @@ -54,6 +66,19 @@ go grant SELECT on schema::babel_4344_S1 to public; go +grant select on schema::αγάπη to αιώνια; +go + +-- tsql user=αιώνια password=12345678 +use babel_4344_d1; +go + +select * from αγάπη.abc; +go + +use master; +go + -- tsql user=babel_4344_l1 password=12345678 use babel_4344_d1; go @@ -388,9 +413,18 @@ go drop schema babel_4344_s2; go +drop table αγάπη.abc; +go + +drop schema αγάπη; +go + drop user babel_4344_u1; go +drop user αιώνια; +go + use master; go @@ -410,3 +444,17 @@ go -- tsql drop login babel_4344_l1; go + +-- psql +-- Need to terminate active session before cleaning up the login +SELECT pg_terminate_backend(pid) FROM pg_stat_get_activity(NULL) +WHERE sys.suser_name(usesysid) = 'αιώνια' AND backend_type = 'client backend' AND usesysid IS NOT NULL; +go + +-- Wait to sync with another session +SELECT pg_sleep(1); +go + +-- tsql +drop login αιώνια; +go From 762b4b981509422f7e06d02ebe6d696982e25b8f Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 11:55:23 +0000 Subject: [PATCH 05/14] Empty commit From bf3844d8b6c6e2c9d84118e788a682158cadf501 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 18:24:31 +0000 Subject: [PATCH 06/14] Use pstrdup --- contrib/babelfishpg_tsql/src/tsqlIface.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/tsqlIface.cpp b/contrib/babelfishpg_tsql/src/tsqlIface.cpp index ae4704c293c..11147de2269 100644 --- a/contrib/babelfishpg_tsql/src/tsqlIface.cpp +++ b/contrib/babelfishpg_tsql/src/tsqlIface.cpp @@ -5529,7 +5529,7 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) } if (prin->PUBLIC()) { - char *grantee_name = (char *)"public"; + char *grantee_name = pstrdup("public");//(char *)"public"; grantee_list = lappend(grantee_list, grantee_name); } } @@ -5562,7 +5562,7 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) } if (prin->PUBLIC()) { - char *grantee_name = (char *)"public"; + char *grantee_name = pstrdup("public"); grantee_list = lappend(grantee_list, grantee_name); } } @@ -5597,7 +5597,7 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) } if (prin->PUBLIC()) { - char *grantee_name = (char *)"public"; + char *grantee_name = pstrdup("public"); grantee_list = lappend(grantee_list, grantee_name); } } @@ -5654,7 +5654,7 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) } if (prin->PUBLIC()) { - char *grantee_name = (char *)"public"; + char *grantee_name = pstrdup("public"); grantee_list = lappend(grantee_list, grantee_name); } } From 836f8af02f99ae536c92994b6d57c326a3d73542 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 21:06:43 +0000 Subject: [PATCH 07/14] Add test --- test/JDBC/expected/GRANT_SCHEMA.out | 22 ++++++++++++++++++++++ test/JDBC/input/GRANT_SCHEMA.mix | 11 +++++++++++ 2 files changed, 33 insertions(+) diff --git a/test/JDBC/expected/GRANT_SCHEMA.out b/test/JDBC/expected/GRANT_SCHEMA.out index 5acf305efc3..062797a00d1 100644 --- a/test/JDBC/expected/GRANT_SCHEMA.out +++ b/test/JDBC/expected/GRANT_SCHEMA.out @@ -328,6 +328,19 @@ go use master; go +-- psql +-- GRANT statement add an entry to the catalog +select schema_name, object_name, permission, grantee from sys.babelfish_schema_permissions +where schema_name = 'babel_4344_s1' collate "C" and object_name = 'ALL' collate "C" order by permission; +go +~~START~~ +name#!#name#!#name#!#name +babel_4344_s1#!#ALL#!#execute#!#babel_4344_d1_babel_4344_u1 +babel_4344_s1#!#ALL#!#insert#!#babel_4344_d1_babel_4344_u1 +babel_4344_s1#!#ALL#!#select#!#babel_4344_d1_babel_4344_u1 +~~END~~ + + -- tsql user=babel_4344_l1 password=12345678 -- User has OBJECT and SCHEMA privileges, should be accessible. use babel_4344_d1; @@ -570,6 +583,15 @@ go use master; go +-- psql +-- REVOKE on schema removes the entry from the catalog +select * from sys.babelfish_schema_permissions where schema_name = 'babel_4344_s1' collate sys.database_default; +go +~~START~~ +int2#!#name#!#name#!#name#!#name#!#name +~~END~~ + + -- tsql user=babel_4344_l1 password=12345678 -- User has no privileges, shouldn't be accessible. use babel_4344_d1; diff --git a/test/JDBC/input/GRANT_SCHEMA.mix b/test/JDBC/input/GRANT_SCHEMA.mix index a6510a9ed96..a7175f2fd53 100644 --- a/test/JDBC/input/GRANT_SCHEMA.mix +++ b/test/JDBC/input/GRANT_SCHEMA.mix @@ -207,6 +207,12 @@ go use master; go +-- psql +-- GRANT statement add an entry to the catalog +select schema_name, object_name, permission, grantee from sys.babelfish_schema_permissions +where schema_name = 'babel_4344_s1' collate "C" and object_name = 'ALL' collate "C" order by permission; +go + -- tsql user=babel_4344_l1 password=12345678 -- User has OBJECT and SCHEMA privileges, should be accessible. use babel_4344_d1; @@ -338,6 +344,11 @@ go use master; go +-- psql +-- REVOKE on schema removes the entry from the catalog +select * from sys.babelfish_schema_permissions where schema_name = 'babel_4344_s1' collate sys.database_default; +go + -- tsql user=babel_4344_l1 password=12345678 -- User has no privileges, shouldn't be accessible. use babel_4344_d1; From 9edc046a32094207b6a24a6ccaff4cc5a693b9a7 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Fri, 17 Nov 2023 21:21:22 +0000 Subject: [PATCH 08/14] Fix sql validation test --- .../python/expected/sql_validation_framework/expected_create.out | 1 - 1 file changed, 1 deletion(-) diff --git a/test/python/expected/sql_validation_framework/expected_create.out b/test/python/expected/sql_validation_framework/expected_create.out index 0aa3907ff7b..700ab049b73 100644 --- a/test/python/expected/sql_validation_framework/expected_create.out +++ b/test/python/expected/sql_validation_framework/expected_create.out @@ -73,7 +73,6 @@ Could not find tests for procedure sys.printarg Could not find tests for procedure sys.sp_cursor_list Could not find tests for procedure sys.sp_describe_cursor Could not find tests for table sys.babelfish_helpcollation -Could not find tests for table sys.babelfish_schema_permissions Could not find tests for table sys.babelfish_syslanguages Could not find tests for table sys.service_settings Could not find tests for table sys.spt_datatype_info_table From 32307b193d09d0a7ebde070099c7c7e602c0a4cc Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Mon, 20 Nov 2023 05:17:42 +0000 Subject: [PATCH 09/14] Empty commit From 4da4b77f52302209469f17718b734fe85ab84232 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Mon, 20 Nov 2023 11:58:28 +0000 Subject: [PATCH 10/14] Address comments --- contrib/babelfishpg_tsql/src/catalog.c | 13 +++++++-- contrib/babelfishpg_tsql/src/pl_exec-2.c | 7 ++++- contrib/babelfishpg_tsql/src/pl_handler.c | 31 ++++++++++++---------- contrib/babelfishpg_tsql/src/tsqlIface.cpp | 2 +- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/catalog.c b/contrib/babelfishpg_tsql/src/catalog.c index 56c6a44ac7d..38af3dfeb42 100644 --- a/contrib/babelfishpg_tsql/src/catalog.c +++ b/contrib/babelfishpg_tsql/src/catalog.c @@ -2937,6 +2937,9 @@ check_bbf_schema_for_entry(const char *schema_name, return catalog_entry_exists; } +/* + * Checks if a particular schema has any SCHEMA level permission granted to any user. + */ bool check_bbf_schema_for_schema(const char *schema_name, const char *object_name, @@ -3047,8 +3050,6 @@ del_from_bbf_schema(const char *schema_name, systable_endscan(scan); table_close(bbf_schema_rel, RowExclusiveLock); - - CommandCounterIncrement(); } void @@ -3120,6 +3121,14 @@ clean_up_bbf_schema(const char *schema_name, table_close(bbf_schema_rel, RowExclusiveLock); } +/* + * REVOKE on SCHEMA: Revokes permission from all the objects belonging to that schema, + * but objects which has explicit OBJECT level permission should be accessible. + * + * For all objects belonging to that schema which has OBJECT level permission, + * we need to grant the permission explicitly after REVOKE has been executed. + */ + void grant_perms_to_objects_in_schema(const char *schema_name, const char *permission, diff --git a/contrib/babelfishpg_tsql/src/pl_exec-2.c b/contrib/babelfishpg_tsql/src/pl_exec-2.c index 05e74327aaa..c8412e193de 100644 --- a/contrib/babelfishpg_tsql/src/pl_exec-2.c +++ b/contrib/babelfishpg_tsql/src/pl_exec-2.c @@ -3688,6 +3688,7 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) ListCell *lc; ListCell *lc1; Oid schemaOid; + char *user = GetUserNameFromId(GetUserId(), false); /* * If the login is not the db owner or the login is not the member of @@ -3719,7 +3720,6 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) foreach(lc, stmt->grantees) { char *grantee_name = (char *) lfirst(lc); - char *user = GetUserNameFromId(GetUserId(), false); Oid role_oid; bool grantee_is_db_owner; if (strcmp(grantee_name, "public") != 0) @@ -3781,8 +3781,13 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) grant_perms_to_objects_in_schema(stmt->schema_name, priv_name, rolname); del_from_bbf_schema(stmt->schema_name, "ALL", priv_name, rolname); } + pfree(rolname); } } + pfree(user); + pfree(schema_name); + pfree(dbname); + pfree(login); return PLTSQL_RC_OK; } diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 5a657ccc79d..72835a6dc16 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3638,10 +3638,13 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); int i = 0; + bool rol_is_not_public = false; char *permissions[] = {"select", "insert", "update", "references", "delete"}; - for(i = 0; i < 5; i++) + if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + rol_is_not_public = true; + if (rol_is_not_public) { - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + for(i = 0; i < 5; i++) { /* * If object permission doesn't exist already, add an entry to the catalog. @@ -3774,19 +3777,20 @@ bbf_ProcessUtility(PlannedStmt *pstmt, logicalschema = get_logical_schema_name(schemaname, true); funcname = strVal(func); } - /* - * Case: When ALL PRIVILEGES is revoked internally during create function. - * Check if schema entry exists in the catalog, do not revoke any permission if exists. - */ - if (pstmt->stmt_len == 0 && list_length(grant->privileges) == 0) - { - if(check_bbf_schema_for_schema(logicalschema, "ALL", "execute")) - return; - break; - } + /* If ALL PRIVILEGES is granted/revoked. */ if (list_length(grant->privileges) == 0) { + /* + * Case: When ALL PRIVILEGES is revoked internally during create function. + * Check if schema entry exists in the catalog, do not revoke any permission if exists. + */ + if (pstmt->stmt_len == 0) + { + if(check_bbf_schema_for_schema(logicalschema, "ALL", "execute")) + return; + } + if (grant->is_grant) { foreach(lc, grant->grantees) @@ -3798,7 +3802,6 @@ bbf_ProcessUtility(PlannedStmt *pstmt, add_entry_to_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename, obj_type); } } - break; } else { @@ -3816,8 +3819,8 @@ bbf_ProcessUtility(PlannedStmt *pstmt, if (has_schema_perms) return; } - break; } + break; } foreach(lc1, grant->privileges) { diff --git a/contrib/babelfishpg_tsql/src/tsqlIface.cpp b/contrib/babelfishpg_tsql/src/tsqlIface.cpp index 11147de2269..be3ceece6cb 100644 --- a/contrib/babelfishpg_tsql/src/tsqlIface.cpp +++ b/contrib/babelfishpg_tsql/src/tsqlIface.cpp @@ -5529,7 +5529,7 @@ makeGrantdbStatement(TSqlParser::Security_statementContext *ctx) } if (prin->PUBLIC()) { - char *grantee_name = pstrdup("public");//(char *)"public"; + char *grantee_name = pstrdup("public"); grantee_list = lappend(grantee_list, grantee_name); } } From c4a935270aeb5c82d02d65a4914472eef8f42c19 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Mon, 20 Nov 2023 15:56:22 +0000 Subject: [PATCH 11/14] Handle public --- contrib/babelfishpg_tsql/src/catalog.c | 16 ++- contrib/babelfishpg_tsql/src/pl_handler.c | 125 ++++++++-------------- 2 files changed, 62 insertions(+), 79 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/catalog.c b/contrib/babelfishpg_tsql/src/catalog.c index 38af3dfeb42..15865b30395 100644 --- a/contrib/babelfishpg_tsql/src/catalog.c +++ b/contrib/babelfishpg_tsql/src/catalog.c @@ -2843,6 +2843,10 @@ add_entry_to_bbf_schema(const char *schema_name, bool new_record_nulls_bbf_schema[BBF_SCHEMA_PERMS_NUM_OF_COLS]; int16 dbid = get_cur_db_id(); + /* Immediately return, if grantee is NULL or PUBLIC. */ + if ((grantee == NULL) || (strcmp(grantee, "public") == 0)) + return; + /* Fetch the relation */ bbf_schema_rel = table_open(get_bbf_schema_perms_oid(), RowExclusiveLock); @@ -2890,6 +2894,10 @@ check_bbf_schema_for_entry(const char *schema_name, bool catalog_entry_exists = false; int16 dbid = get_cur_db_id(); + /* Immediately return false, if grantee is NULL or PUBLIC. */ + if ((grantee == NULL) || (strcmp(grantee, "public") == 0)) + return false; + bbf_schema_rel = table_open(get_bbf_schema_perms_oid(), AccessShareLock); ScanKeyInit(&scanKey[0], @@ -3005,6 +3013,10 @@ del_from_bbf_schema(const char *schema_name, SysScanDesc scan; int16 dbid = get_cur_db_id(); + /* Immediately return, if grantee is NULL or PUBLIC. */ + if ((grantee == NULL) || (strcmp(grantee, "public") == 0)) + return; + bbf_schema_rel = table_open(get_bbf_schema_perms_oid(), RowExclusiveLock); ScanKeyInit(&scanKey[0], @@ -3122,11 +3134,13 @@ clean_up_bbf_schema(const char *schema_name, } /* + * CONTEXT * REVOKE on SCHEMA: Revokes permission from all the objects belonging to that schema, * but objects which has explicit OBJECT level permission should be accessible. * + * grant_perms_to_objects_in_schema: * For all objects belonging to that schema which has OBJECT level permission, - * we need to grant the permission explicitly after REVOKE has been executed. + * It grants the permission explicitly after REVOKE has been executed. */ void diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 72835a6dc16..285c4a2ad86 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3638,20 +3638,14 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); int i = 0; - bool rol_is_not_public = false; char *permissions[] = {"select", "insert", "update", "references", "delete"}; - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) - rol_is_not_public = true; - if (rol_is_not_public) + for(i = 0; i < 5; i++) { - for(i = 0; i < 5; i++) - { - /* - * If object permission doesn't exist already, add an entry to the catalog. - */ - if (!check_bbf_schema_for_entry(logical_schema, obj, permissions[i], rol_spec->rolename)) - add_entry_to_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename, obj_type); - } + /* + * If object permission doesn't exist already, add an entry to the catalog. + */ + if (!check_bbf_schema_for_entry(logical_schema, obj, permissions[i], rol_spec->rolename)) + add_entry_to_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename, obj_type); } } break; @@ -3666,19 +3660,15 @@ bbf_ProcessUtility(PlannedStmt *pstmt, char *permissions[] = {"select", "insert", "update", "references", "delete"}; for(i = 0; i < 5; i++) { - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) - { - /* - * 1. If only schema permission exists, don't revoke any permission. - * 2. If only object permission exists, delete entry from the catalog and revoke permission. - * 3. If both schema and object permission exist, don't revoke any permission but delete object - * entry from the catalog. - */ - if (check_bbf_schema_for_entry(logical_schema, "ALL", permissions[i], rol_spec->rolename) && !has_schema_perms) - has_schema_perms = true; - if (check_bbf_schema_for_entry(logical_schema, obj, permissions[i], rol_spec->rolename)) - del_from_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename); - } + /* + * 1. If only schema permission exists, don't revoke any permission. + * 2. If only object permission exists, delete entry from the catalog and revoke permission. + * 3. If both schema and object permission exist, don't revoke any permission but delete object + * entry from the catalog. + */ + if (check_bbf_schema_for_entry(logical_schema, "ALL", permissions[i], rol_spec->rolename) && !has_schema_perms) + has_schema_perms = true; + del_from_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename); } if (has_schema_perms) return; @@ -3705,16 +3695,13 @@ bbf_ProcessUtility(PlannedStmt *pstmt, foreach(lc, grant->grantees) { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) - { - /* - * Don't add an entry, if the permission is granted on column list. - */ - if (ap->cols != NULL) - break; - if (!check_bbf_schema_for_entry(logical_schema, obj, ap->priv_name, rol_spec->rolename)) - add_entry_to_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename, obj_type); - } + /* + * Don't add an entry, if the permission is granted on column list. + */ + if (ap->cols != NULL) + break; + if (!check_bbf_schema_for_entry(logical_schema, obj, ap->priv_name, rol_spec->rolename)) + add_entry_to_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename, obj_type); } } else @@ -3726,25 +3713,21 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. */ - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + if ((logical_schema != NULL) && !check_bbf_schema_for_entry(logical_schema, "ALL", ap->priv_name, rol_spec->rolename)) { - if ((logical_schema != NULL) && !check_bbf_schema_for_entry(logical_schema, "ALL", ap->priv_name, rol_spec->rolename)) - { - if (prev_ProcessUtility) - prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, + if (prev_ProcessUtility) + prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, + queryEnv, dest, qc); + else + standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, queryEnv, dest, qc); - else - standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, - queryEnv, dest, qc); - } - /* - * Don't remove an entry, if the permission is revoked on column list. - */ - if (ap->cols != NULL) - break; - if (check_bbf_schema_for_entry(logical_schema, obj, ap->priv_name, rol_spec->rolename)) - del_from_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename); } + /* + * Don't remove an entry, if the permission is revoked on column list. + */ + if (ap->cols != NULL) + break; + del_from_bbf_schema(logical_schema, obj, ap->priv_name, rol_spec->rolename); } } } @@ -3796,11 +3779,8 @@ bbf_ProcessUtility(PlannedStmt *pstmt, foreach(lc, grant->grantees) { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) - { - if (!check_bbf_schema_for_entry(logicalschema, funcname, "execute", rol_spec->rolename)) - add_entry_to_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename, obj_type); - } + if (!check_bbf_schema_for_entry(logicalschema, funcname, "execute", rol_spec->rolename)) + add_entry_to_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename, obj_type); } } else @@ -3809,13 +3789,9 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); bool has_schema_perms = false; - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) - { - if (check_bbf_schema_for_entry(logicalschema, "ALL", "execute", rol_spec->rolename) && !has_schema_perms) - has_schema_perms = true; - if (check_bbf_schema_for_entry(logicalschema, funcname, "execute", rol_spec->rolename)) - del_from_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename); - } + if (check_bbf_schema_for_entry(logicalschema, "ALL", "execute", rol_spec->rolename) && !has_schema_perms) + has_schema_perms = true; + del_from_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename); if (has_schema_perms) return; } @@ -3839,12 +3815,9 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); /* Add an entry to the catalog, if an entry doesn't exist already. */ - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + if (!check_bbf_schema_for_entry(logicalschema, funcname, ap->priv_name, rol_spec->rolename)) { - if (!check_bbf_schema_for_entry(logicalschema, funcname, ap->priv_name, rol_spec->rolename)) - { - add_entry_to_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename, obj_type); - } + add_entry_to_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename, obj_type); } } } @@ -3854,13 +3827,11 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); /* - * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. - * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. - */ - if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) + * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. + * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. + */ + if (!check_bbf_schema_for_entry(logicalschema, "ALL", ap->priv_name, rol_spec->rolename)) { - if (!check_bbf_schema_for_entry(logicalschema, "ALL", ap->priv_name, rol_spec->rolename)) - { /* Execute REVOKE statement. */ if (prev_ProcessUtility) prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, @@ -3868,11 +3839,9 @@ bbf_ProcessUtility(PlannedStmt *pstmt, else standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, queryEnv, dest, qc); - } - /* Remove an entry from the catalog, if it exists. */ - if (check_bbf_schema_for_entry(logicalschema, funcname, ap->priv_name, rol_spec->rolename)) - del_from_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename); } + /* Remove an entry from the catalog, if it exists. */ + del_from_bbf_schema(logicalschema, funcname, ap->priv_name, rol_spec->rolename); } } } From eab4945aa8f8c7d6a09e8a828d02177a62d8b209 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Mon, 20 Nov 2023 18:44:27 +0000 Subject: [PATCH 12/14] Add more tests --- test/JDBC/expected/GRANT_SCHEMA.out | 105 +++++++++++++++++++++++++--- test/JDBC/input/GRANT_SCHEMA.mix | 70 +++++++++++++++++-- 2 files changed, 162 insertions(+), 13 deletions(-) diff --git a/test/JDBC/expected/GRANT_SCHEMA.out b/test/JDBC/expected/GRANT_SCHEMA.out index 062797a00d1..935acbe2f53 100644 --- a/test/JDBC/expected/GRANT_SCHEMA.out +++ b/test/JDBC/expected/GRANT_SCHEMA.out @@ -57,13 +57,16 @@ go create proc babel_4344_s1.babel_4344_p1 as select 2; go +create proc babel_4344_s1.babel_4344_p3 as select 3; +go + CREATE FUNCTION babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.tables) END go CREATE FUNCTION babel_4344_s1.babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.objects) END go -grant SELECT on schema::babel_4344_S1 to public; +grant SELECT on schema::babel_4344_S1 to public, αιώνια; go grant select on schema::αγάπη to αιώνια; @@ -80,6 +83,13 @@ int ~~END~~ +select * from babel_4344_S1.babel_4344_t1; +go +~~START~~ +int +~~END~~ + + use master; go @@ -107,7 +117,7 @@ go -- tsql use babel_4344_d1; go -revoke select on schema::babel_4344_s1 from public; +revoke select on schema::babel_4344_s1 from public, αιώνια; go -- tsql user=babel_4344_l1 password=12345678 @@ -194,6 +204,12 @@ grant execute on babel_4344_p1 to babel_4344_u1; go grant execute on babel_4344_s1.babel_4344_p1 to babel_4344_u1; go +-- inside a transaction, permission will not be granted since it is rolled back +begin transaction; +exec sp_executesql N'grant execute on babel_4344_s1.babel_4344_p3 to babel_4344_u1;'; +rollback transaction; +go + -- Mixed case grant Execute on Babel_4344_F1 to babel_4344_u1; go @@ -220,12 +236,28 @@ go ~~ERROR (Message: An object or column name is missing or empty. For SELECT INTO statements, verify each column has a name. For other statements, look for empty alias names. Aliases defined as "" or [] are not allowed. Change the alias to a valid name.)~~ -grant select on schema::[] to guest; -- should fail +grant select on schema::non_existing_schema to guest; -- should fail +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: schema "non_existing_schema" does not exist)~~ + +-- grant statement via a procedure +create procedure grant_perm_proc as begin exec('grant select on schema::[] to guest') end; +go +exec grant_perm_proc; -- should fail go ~~ERROR (Code: 33557097)~~ ~~ERROR (Message: An object or column name is missing or empty. For SELECT INTO statements, verify each column has a name. For other statements, look for empty alias names. Aliases defined as "" or [] are not allowed. Change the alias to a valid name.)~~ +-- non-existing role +grant SELECT on schema::dbo to guest, babel_4344_u3; -- should fail +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Cannot find the principal 'babel_4344_u3', because it does not exist or you do not have permission.)~~ + -- tsql user=babel_4344_l1 password=12345678 -- User has OBJECT privileges, should be accessible. @@ -287,6 +319,12 @@ int 2 ~~END~~ +exec babel_4344_s1.babel_4344_p3; -- should fail, grant statement was rolled back +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: permission denied for procedure babel_4344_p3)~~ + select * from babel_4344_f1(); go ~~START~~ @@ -380,7 +418,7 @@ select * from babel_4344_s1.babel_4344_f1(); go ~~START~~ int -10 +11 ~~END~~ use master; @@ -424,14 +462,14 @@ int 2 ~~END~~ -exec babel_4344_s1.babel_4344_p1; -- TODO: should be accessible +exec babel_4344_s1.babel_4344_p1; go ~~START~~ int 2 ~~END~~ -select * from babel_4344_s1.babel_4344_f1(); -- TODO: should be accessible +select * from babel_4344_s1.babel_4344_f1(); go ~~START~~ int @@ -497,7 +535,7 @@ select * from babel_4344_s1.babel_4344_f2(); go ~~START~~ int -14 +15 ~~END~~ use master; @@ -562,7 +600,7 @@ select * from babel_4344_s1.babel_4344_f1(); go ~~START~~ int -14 +15 ~~END~~ select * from babel_4344_s2.babel_4344_t1; @@ -635,6 +673,54 @@ go use master; go +-- psql +-- grant object permission +grant select on babel_4344_s1.babel_4344_t1 to babel_4344_d1_babel_4344_u1; +go + +-- tsql +-- grant schema permission +use babel_4344_d1; +go +grant select on schema::babel_4344_s1 to babel_4344_u1; +go +use master +go + +-- tsql user=babel_4344_l1 password=12345678 +use babel_4344_d1; +go +select * from babel_4344_s1.babel_4344_t1; -- accessible +go +~~START~~ +int +2 +3 +3 +4 +5 +~~END~~ + +use master +go + +-- psql +-- revoke schema permission +revoke select on all tables in schema babel_4344_s1 from babel_4344_d1_babel_4344_u1; +go + +-- tsql user=babel_4344_l1 password=12345678 +use babel_4344_d1; +go +select * from babel_4344_s1.babel_4344_t1; -- not accessible +go +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: permission denied for table babel_4344_t1)~~ + +use master +go + -- tsql -- Drop objects use babel_4344_d1; @@ -673,6 +759,9 @@ go drop proc babel_4344_s1.babel_4344_p2; go +drop proc babel_4344_s1.babel_4344_p3; +go + drop function babel_4344_f1; go diff --git a/test/JDBC/input/GRANT_SCHEMA.mix b/test/JDBC/input/GRANT_SCHEMA.mix index a7175f2fd53..f4af9f7af76 100644 --- a/test/JDBC/input/GRANT_SCHEMA.mix +++ b/test/JDBC/input/GRANT_SCHEMA.mix @@ -57,13 +57,16 @@ go create proc babel_4344_s1.babel_4344_p1 as select 2; go +create proc babel_4344_s1.babel_4344_p3 as select 3; +go + CREATE FUNCTION babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.tables) END go CREATE FUNCTION babel_4344_s1.babel_4344_f1() RETURNS INT AS BEGIN RETURN (SELECT COUNT(*) FROM sys.objects) END go -grant SELECT on schema::babel_4344_S1 to public; +grant SELECT on schema::babel_4344_S1 to public, αιώνια; go grant select on schema::αγάπη to αιώνια; @@ -76,6 +79,9 @@ go select * from αγάπη.abc; go +select * from babel_4344_S1.babel_4344_t1; +go + use master; go @@ -94,7 +100,7 @@ go -- tsql use babel_4344_d1; go -revoke select on schema::babel_4344_s1 from public; +revoke select on schema::babel_4344_s1 from public, αιώνια; go -- tsql user=babel_4344_l1 password=12345678 @@ -145,6 +151,12 @@ grant execute on babel_4344_p1 to babel_4344_u1; go grant execute on babel_4344_s1.babel_4344_p1 to babel_4344_u1; go +-- inside a transaction, permission will not be granted since it is rolled back +begin transaction; +exec sp_executesql N'grant execute on babel_4344_s1.babel_4344_p3 to babel_4344_u1;'; +rollback transaction; +go + -- Mixed case grant Execute on Babel_4344_F1 to babel_4344_u1; go @@ -159,7 +171,15 @@ grant SELECT on schema::babel_4344_s2 to guest; -- should pass go grant select on schema::"" to guest; -- should fail go -grant select on schema::[] to guest; -- should fail +grant select on schema::non_existing_schema to guest; -- should fail +go +-- grant statement via a procedure +create procedure grant_perm_proc as begin exec('grant select on schema::[] to guest') end; +go +exec grant_perm_proc; -- should fail +go +-- non-existing role +grant SELECT on schema::dbo to guest, babel_4344_u3; -- should fail go -- tsql user=babel_4344_l1 password=12345678 @@ -184,6 +204,8 @@ exec babel_4344_p1; go exec babel_4344_s1.babel_4344_p1; go +exec babel_4344_s1.babel_4344_p3; -- should fail, grant statement was rolled back +go select * from babel_4344_f1(); go select * from babel_4344_s1.babel_4344_f1(); @@ -253,9 +275,9 @@ select * from babel_4344_s1.babel_4344_t3 -- not accessible go select * from babel_4344_s1.babel_4344_v1; go -exec babel_4344_s1.babel_4344_p1; -- TODO: should be accessible +exec babel_4344_s1.babel_4344_p1; go -select * from babel_4344_s1.babel_4344_f1(); -- TODO: should be accessible +select * from babel_4344_s1.babel_4344_f1(); go select * from babel_4344_s2.babel_4344_t1; go @@ -368,6 +390,41 @@ go use master; go +-- psql +-- grant object permission +grant select on babel_4344_s1.babel_4344_t1 to babel_4344_d1_babel_4344_u1; +go + +-- tsql +-- grant schema permission +use babel_4344_d1; +go +grant select on schema::babel_4344_s1 to babel_4344_u1; +go +use master +go + +-- tsql user=babel_4344_l1 password=12345678 +use babel_4344_d1; +go +select * from babel_4344_s1.babel_4344_t1; -- accessible +go +use master +go + +-- psql +-- revoke schema permission +revoke select on all tables in schema babel_4344_s1 from babel_4344_d1_babel_4344_u1; +go + +-- tsql user=babel_4344_l1 password=12345678 +use babel_4344_d1; +go +select * from babel_4344_s1.babel_4344_t1; -- not accessible +go +use master +go + -- tsql -- Drop objects use babel_4344_d1; @@ -406,6 +463,9 @@ go drop proc babel_4344_s1.babel_4344_p2; go +drop proc babel_4344_s1.babel_4344_p3; +go + drop function babel_4344_f1; go From c73c907dcb76529911ded3143f064f4281a55914 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Tue, 21 Nov 2023 03:50:01 +0000 Subject: [PATCH 13/14] Add a macro --- contrib/babelfishpg_tsql/src/catalog.c | 10 +++------- contrib/babelfishpg_tsql/src/catalog.h | 2 ++ contrib/babelfishpg_tsql/src/pl_exec-2.c | 8 ++++---- contrib/babelfishpg_tsql/src/pl_handler.c | 10 +++++----- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/catalog.c b/contrib/babelfishpg_tsql/src/catalog.c index 15865b30395..2d832e6a6e6 100644 --- a/contrib/babelfishpg_tsql/src/catalog.c +++ b/contrib/babelfishpg_tsql/src/catalog.c @@ -3134,13 +3134,9 @@ clean_up_bbf_schema(const char *schema_name, } /* - * CONTEXT - * REVOKE on SCHEMA: Revokes permission from all the objects belonging to that schema, - * but objects which has explicit OBJECT level permission should be accessible. - * - * grant_perms_to_objects_in_schema: - * For all objects belonging to that schema which has OBJECT level permission, - * It grants the permission explicitly after REVOKE has been executed. + * For all objects belonging to a schema which has OBJECT level permission, + * It grants the permission explicitly when REVOKE has been executed on that + * specific schema. */ void diff --git a/contrib/babelfishpg_tsql/src/catalog.h b/contrib/babelfishpg_tsql/src/catalog.h index 7b8ad195c27..f8250e9de25 100644 --- a/contrib/babelfishpg_tsql/src/catalog.h +++ b/contrib/babelfishpg_tsql/src/catalog.h @@ -299,6 +299,8 @@ typedef FormData_bbf_function_ext *Form_bbf_function_ext; #define Anum_bbf_schema_perms_grantee 5 #define Anum_bbf_schema_perms_object_type 6 +#define SCHEMA_PERMISSION_EXISTS "ALL" + extern Oid bbf_schema_perms_oid; extern Oid bbf_schema_perms_idx_oid; diff --git a/contrib/babelfishpg_tsql/src/pl_exec-2.c b/contrib/babelfishpg_tsql/src/pl_exec-2.c index c8412e193de..05632f753a7 100644 --- a/contrib/babelfishpg_tsql/src/pl_exec-2.c +++ b/contrib/babelfishpg_tsql/src/pl_exec-2.c @@ -3772,14 +3772,14 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) CommandCounterIncrement(); } /* Add entry for each grant statement. */ - if (stmt->is_grant && !check_bbf_schema_for_entry(stmt->schema_name, "ALL", priv_name, rolname)) - add_entry_to_bbf_schema(stmt->schema_name, "ALL", priv_name, rolname, NULL); + if (stmt->is_grant && !check_bbf_schema_for_entry(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname)) + add_entry_to_bbf_schema(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname, NULL); /* Remove entry for each revoke statement. */ - if (!stmt->is_grant && check_bbf_schema_for_entry(stmt->schema_name, "ALL", priv_name, rolname)) + if (!stmt->is_grant && check_bbf_schema_for_entry(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname)) { /* If any object in the schema has the OBJECT level permission. Then, internally grant that permission back. */ grant_perms_to_objects_in_schema(stmt->schema_name, priv_name, rolname); - del_from_bbf_schema(stmt->schema_name, "ALL", priv_name, rolname); + del_from_bbf_schema(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname); } pfree(rolname); } diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 285c4a2ad86..f629081efe4 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3666,7 +3666,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 3. If both schema and object permission exist, don't revoke any permission but delete object * entry from the catalog. */ - if (check_bbf_schema_for_entry(logical_schema, "ALL", permissions[i], rol_spec->rolename) && !has_schema_perms) + if (check_bbf_schema_for_entry(logical_schema, SCHEMA_PERMISSION_EXISTS, permissions[i], rol_spec->rolename) && !has_schema_perms) has_schema_perms = true; del_from_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename); } @@ -3713,7 +3713,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. */ - if ((logical_schema != NULL) && !check_bbf_schema_for_entry(logical_schema, "ALL", ap->priv_name, rol_spec->rolename)) + if ((logical_schema != NULL) && !check_bbf_schema_for_entry(logical_schema, SCHEMA_PERMISSION_EXISTS, ap->priv_name, rol_spec->rolename)) { if (prev_ProcessUtility) prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, @@ -3770,7 +3770,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, */ if (pstmt->stmt_len == 0) { - if(check_bbf_schema_for_schema(logicalschema, "ALL", "execute")) + if(check_bbf_schema_for_schema(logicalschema, SCHEMA_PERMISSION_EXISTS, "execute")) return; } @@ -3789,7 +3789,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); bool has_schema_perms = false; - if (check_bbf_schema_for_entry(logicalschema, "ALL", "execute", rol_spec->rolename) && !has_schema_perms) + if (check_bbf_schema_for_entry(logicalschema, SCHEMA_PERMISSION_EXISTS, "execute", rol_spec->rolename) && !has_schema_perms) has_schema_perms = true; del_from_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename); if (has_schema_perms) @@ -3830,7 +3830,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. */ - if (!check_bbf_schema_for_entry(logicalschema, "ALL", ap->priv_name, rol_spec->rolename)) + if (!check_bbf_schema_for_entry(logicalschema, SCHEMA_PERMISSION_EXISTS, ap->priv_name, rol_spec->rolename)) { /* Execute REVOKE statement. */ if (prev_ProcessUtility) From 22968787c9a3e30c8a5b1784ad70c3430167751d Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Tue, 21 Nov 2023 09:39:50 +0000 Subject: [PATCH 14/14] Change macro name --- contrib/babelfishpg_tsql/src/catalog.h | 2 +- contrib/babelfishpg_tsql/src/pl_exec-2.c | 8 ++++---- contrib/babelfishpg_tsql/src/pl_handler.c | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/catalog.h b/contrib/babelfishpg_tsql/src/catalog.h index f8250e9de25..c3a7de00859 100644 --- a/contrib/babelfishpg_tsql/src/catalog.h +++ b/contrib/babelfishpg_tsql/src/catalog.h @@ -299,7 +299,7 @@ typedef FormData_bbf_function_ext *Form_bbf_function_ext; #define Anum_bbf_schema_perms_grantee 5 #define Anum_bbf_schema_perms_object_type 6 -#define SCHEMA_PERMISSION_EXISTS "ALL" +#define PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA "ALL" extern Oid bbf_schema_perms_oid; extern Oid bbf_schema_perms_idx_oid; diff --git a/contrib/babelfishpg_tsql/src/pl_exec-2.c b/contrib/babelfishpg_tsql/src/pl_exec-2.c index 05632f753a7..3e98b678e85 100644 --- a/contrib/babelfishpg_tsql/src/pl_exec-2.c +++ b/contrib/babelfishpg_tsql/src/pl_exec-2.c @@ -3772,14 +3772,14 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) CommandCounterIncrement(); } /* Add entry for each grant statement. */ - if (stmt->is_grant && !check_bbf_schema_for_entry(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname)) - add_entry_to_bbf_schema(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname, NULL); + if (stmt->is_grant && !check_bbf_schema_for_entry(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, priv_name, rolname)) + add_entry_to_bbf_schema(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, priv_name, rolname, NULL); /* Remove entry for each revoke statement. */ - if (!stmt->is_grant && check_bbf_schema_for_entry(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname)) + if (!stmt->is_grant && check_bbf_schema_for_entry(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, priv_name, rolname)) { /* If any object in the schema has the OBJECT level permission. Then, internally grant that permission back. */ grant_perms_to_objects_in_schema(stmt->schema_name, priv_name, rolname); - del_from_bbf_schema(stmt->schema_name, SCHEMA_PERMISSION_EXISTS, priv_name, rolname); + del_from_bbf_schema(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, priv_name, rolname); } pfree(rolname); } diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index f629081efe4..ab83ccdf4af 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3666,7 +3666,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 3. If both schema and object permission exist, don't revoke any permission but delete object * entry from the catalog. */ - if (check_bbf_schema_for_entry(logical_schema, SCHEMA_PERMISSION_EXISTS, permissions[i], rol_spec->rolename) && !has_schema_perms) + if (check_bbf_schema_for_entry(logical_schema, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, permissions[i], rol_spec->rolename) && !has_schema_perms) has_schema_perms = true; del_from_bbf_schema(logical_schema, obj, permissions[i], rol_spec->rolename); } @@ -3713,7 +3713,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. */ - if ((logical_schema != NULL) && !check_bbf_schema_for_entry(logical_schema, SCHEMA_PERMISSION_EXISTS, ap->priv_name, rol_spec->rolename)) + if ((logical_schema != NULL) && !check_bbf_schema_for_entry(logical_schema, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, ap->priv_name, rol_spec->rolename)) { if (prev_ProcessUtility) prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, @@ -3770,7 +3770,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, */ if (pstmt->stmt_len == 0) { - if(check_bbf_schema_for_schema(logicalschema, SCHEMA_PERMISSION_EXISTS, "execute")) + if(check_bbf_schema_for_schema(logicalschema, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, "execute")) return; } @@ -3789,7 +3789,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); bool has_schema_perms = false; - if (check_bbf_schema_for_entry(logicalschema, SCHEMA_PERMISSION_EXISTS, "execute", rol_spec->rolename) && !has_schema_perms) + if (check_bbf_schema_for_entry(logicalschema, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, "execute", rol_spec->rolename) && !has_schema_perms) has_schema_perms = true; del_from_bbf_schema(logicalschema, funcname, "execute", rol_spec->rolename); if (has_schema_perms) @@ -3830,7 +3830,7 @@ bbf_ProcessUtility(PlannedStmt *pstmt, * 1. If GRANT on schema does not exist, execute REVOKE statement and remove the catalog entry if exists. * 2. If GRANT on schema exist, only remove the entry from the catalog if exists. */ - if (!check_bbf_schema_for_entry(logicalschema, SCHEMA_PERMISSION_EXISTS, ap->priv_name, rol_spec->rolename)) + if (!check_bbf_schema_for_entry(logicalschema, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, ap->priv_name, rol_spec->rolename)) { /* Execute REVOKE statement. */ if (prev_ProcessUtility)