From 119a217781a44ce7912e9ca14d360e39f3dfebdf Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Tue, 21 Nov 2023 20:22:18 +0000 Subject: [PATCH] Removed unnecessary checks --- contrib/babelfishpg_tsql/sql/ownership.sql | 2 +- .../babelfishpg_tsql--3.3.0--3.4.0.sql | 2 +- contrib/babelfishpg_tsql/src/catalog.c | 17 +- contrib/babelfishpg_tsql/src/pl_exec-2.c | 37 +-- contrib/babelfishpg_tsql/src/pl_handler.c | 216 +++++++++--------- 5 files changed, 139 insertions(+), 135 deletions(-) diff --git a/contrib/babelfishpg_tsql/sql/ownership.sql b/contrib/babelfishpg_tsql/sql/ownership.sql index 1b0af43e69..a5e831a41a 100644 --- a/contrib/babelfishpg_tsql/sql/ownership.sql +++ b/contrib/babelfishpg_tsql/sql/ownership.sql @@ -19,7 +19,7 @@ CREATE TABLE sys.babelfish_schema_permissions ( dbid smallint NOT NULL, schema_name NAME NOT NULL COLLATE sys.database_default, object_name NAME NOT NULL COLLATE sys.database_default, - permission SMALLINT, + permission SMALLINT NOT NULL, grantee NAME NOT NULL COLLATE sys.database_default, object_type NAME COLLATE sys.database_default, PRIMARY KEY(dbid, schema_name, object_name, 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 0f33dff48b..d80b44411a 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 @@ -894,7 +894,7 @@ CREATE TABLE IF NOT EXISTS sys.babelfish_schema_permissions ( dbid smallint NOT NULL, schema_name NAME NOT NULL COLLATE sys.database_default, object_name NAME NOT NULL COLLATE sys.database_default, - permission SMALLINT, + permission SMALLINT NOT NULL, grantee NAME NOT NULL COLLATE sys.database_default, object_type NAME COLLATE sys.database_default, PRIMARY KEY(dbid, schema_name, object_name, grantee) diff --git a/contrib/babelfishpg_tsql/src/catalog.c b/contrib/babelfishpg_tsql/src/catalog.c index 16909df537..53f713d8ca 100644 --- a/contrib/babelfishpg_tsql/src/catalog.c +++ b/contrib/babelfishpg_tsql/src/catalog.c @@ -3042,8 +3042,6 @@ update_bbf_schema_privilege(const char *schema_name, table_endscan(scan); table_close(bbf_schema_rel, RowExclusiveLock); - - CommandCounterIncrement(); } /* Check if the catalog entry exists. */ @@ -3113,10 +3111,11 @@ check_bbf_schema_for_schema(const char *schema_name, { Relation bbf_schema_rel; HeapTuple tuple_bbf_schema; - ScanKeyData key[3]; - SysScanDesc scan; - int16 dbid = get_cur_db_id(); - int16 priv = 0; + ScanKeyData key[3]; + SysScanDesc scan; + int16 dbid = get_cur_db_id(); + int16 priv = 0; + bool permission_exists_on_schema = false; bbf_schema_rel = table_open(get_bbf_schema_perms_oid(), AccessShareLock); @@ -3144,19 +3143,19 @@ check_bbf_schema_for_schema(const char *schema_name, true, NULL, 3, key); tuple_bbf_schema = systable_getnext(scan); - while (HeapTupleIsValid(tuple_bbf_schema)) + if (HeapTupleIsValid(tuple_bbf_schema)) { Form_bbf_schema_perms schemaform; schemaform = (Form_bbf_schema_perms) GETSTRUCT(tuple_bbf_schema); priv = schemaform->permission; if((permission & priv) == permission) - return true; + permission_exists_on_schema = true; } systable_endscan(scan); table_close(bbf_schema_rel, AccessShareLock); - return false; + return permission_exists_on_schema; } void diff --git a/contrib/babelfishpg_tsql/src/pl_exec-2.c b/contrib/babelfishpg_tsql/src/pl_exec-2.c index f9252bf593..3d051c30e0 100644 --- a/contrib/babelfishpg_tsql/src/pl_exec-2.c +++ b/contrib/babelfishpg_tsql/src/pl_exec-2.c @@ -3786,23 +3786,32 @@ exec_stmt_grantschema(PLtsql_execstate *estate, PLtsql_stmt_grantschema *stmt) if((privilege_maskInt & PRIVILEGE_BIT_FOR_REFERENCES) == PRIVILEGE_BIT_FOR_REFERENCES) exec_gen_grantschema(schema_name, rolname, stmt, "references"); - /* Add entry for each grant statement. */ - if (stmt->is_grant && !check_bbf_schema_for_entry(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rolname)) - add_entry_to_bbf_schema(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, privilege_maskInt, rolname, NULL); - else if(stmt->is_grant) + /* Add/update an entry for a GRANT Statement. */ + if (stmt->is_grant) { - int16 old_priv = get_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rolname); - if(old_priv!= privilege_maskInt || ((old_priv|privilege_maskInt) != old_priv)) - update_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, privilege_maskInt, old_priv, rolname, NULL, stmt->is_grant); + /* Add an entry if it doesn't exist already. */ + if (!check_bbf_schema_for_entry(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rolname)) + add_entry_to_bbf_schema(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, privilege_maskInt, rolname, NULL); + else + { + /* Update the entry if it exists already. */ + int16 old_priv = get_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rolname); + if(old_priv!= privilege_maskInt || ((old_priv|privilege_maskInt) != old_priv)) + update_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, privilege_maskInt, old_priv, rolname, NULL, stmt->is_grant); + } } - /* Remove entry for each revoke statement. */ - else if (!stmt->is_grant && check_bbf_schema_for_entry(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rolname)) + /* Either update the entry or do nothing for REVOKE Statement. */ + else { - /* 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, privilege_maskInt, rolname); - old_priv = get_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rolname); - if(old_priv!= privilege_maskInt || ((old_priv)&(~privilege_maskInt)) != old_priv) - update_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, privilege_maskInt, old_priv, rolname, NULL, stmt->is_grant); + /* Update the entry if it exists already. Otherwise, do nothing. */ + if (check_bbf_schema_for_entry(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, 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, privilege_maskInt, rolname); + old_priv = get_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rolname); + if(old_priv!= privilege_maskInt || ((old_priv)&(~privilege_maskInt)) != old_priv) + update_bbf_schema_privilege(stmt->schema_name, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, privilege_maskInt, old_priv, rolname, NULL, stmt->is_grant); + } } pfree(rolname); } diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index e5463c61a9..4c4369e6bf 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -3619,137 +3619,133 @@ bbf_ProcessUtility(PlannedStmt *pstmt, break; else if (grant->objtype == OBJECT_TABLE) { - /* Ignore CREATE database subcommands */ - if (strcmp("(CREATE LOGICAL DATABASE )", queryString) != 0) + RangeVar *rv = (RangeVar *) linitial(grant->objects); + const char *logical_schema = NULL; + char *obj = rv->relname; + ListCell *lc; + const char *obj_type = "r"; + if (rv->schemaname != NULL) + logical_schema = get_logical_schema_name(rv->schemaname, true); + else + logical_schema = get_authid_user_ext_schema_name(dbname, current_user); + /* If ALL PRIVILEGES is granted/revoked. */ + if (list_length(grant->privileges) == 0) { - RangeVar *rv = (RangeVar *) linitial(grant->objects); - const char *logical_schema = NULL; - char *obj = rv->relname; - ListCell *lc; - const char *obj_type = "r"; - if (rv->schemaname != NULL) - logical_schema = get_logical_schema_name(rv->schemaname, true); - else - logical_schema = get_authid_user_ext_schema_name(dbname, current_user); - /* If ALL PRIVILEGES is granted/revoked. */ - if (list_length(grant->privileges) == 0) - { - if (grant->is_grant) - { - foreach(lc, grant->grantees) - { - RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - if (!check_bbf_schema_for_entry(logical_schema, obj,rol_spec->rolename)) - { - add_entry_to_bbf_schema(logical_schema, obj, ALL_PRIVILEGE_EXCEPT_EXECUTE, rol_spec->rolename, obj_type); - } - else - { - int16 old_priv = get_bbf_schema_privilege(logical_schema, obj,rol_spec->rolename); - if(old_priv!= ALL_PRIVILEGE_EXCEPT_EXECUTE || (old_priv|(ALL_PRIVILEGE_EXCEPT_EXECUTE)) != old_priv) - update_bbf_schema_privilege(logical_schema, obj, ALL_PRIVILEGE_EXCEPT_EXECUTE, old_priv, rol_spec->rolename, obj_type, grant->is_grant); - } - } - break; - } - else - { - foreach(lc, grant->grantees) - { - RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - bool has_schema_perms = false; - if (check_bbf_schema_for_entry(logical_schema, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rol_spec->rolename) && !has_schema_perms) - has_schema_perms = true; - if (check_bbf_schema_for_entry(logical_schema, obj, rol_spec->rolename)) - { - int16 old_priv = get_bbf_schema_privilege(logical_schema, obj,rol_spec->rolename); - if(old_priv!= ALL_PRIVILEGE_EXCEPT_EXECUTE || ((old_priv)&(~(ALL_PRIVILEGE_EXCEPT_EXECUTE))) != old_priv) - update_bbf_schema_privilege(logical_schema, obj, ALL_PRIVILEGE_EXCEPT_EXECUTE, old_priv, rol_spec->rolename, obj_type, grant->is_grant); - } - if (has_schema_perms) - return; - } - break; - } - } if (grant->is_grant) { - /* - * 1. Execute the GRANT statement. - * 2. Add its corresponding entry in the catalog, if doesn't exist already. - * 3. Don't add an entry, if the permission is granted on column list. - */ - if (prev_ProcessUtility) - prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, - queryEnv, dest, qc); - else - standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, - queryEnv, dest, qc); foreach(lc, grant->grantees) { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - bool flag = false; - foreach(lc1, grant->privileges) + if (!check_bbf_schema_for_entry(logical_schema, obj,rol_spec->rolename)) { - AccessPriv *ap = (AccessPriv *) lfirst(lc1); - if(ap->cols != NULL) - { - flag = true; - break; - } + add_entry_to_bbf_schema(logical_schema, obj, ALL_PRIVILEGE_EXCEPT_EXECUTE, rol_spec->rolename, obj_type); } - if(flag) - break; - if (!check_bbf_schema_for_entry(logical_schema, obj, rol_spec->rolename)) - add_entry_to_bbf_schema(logical_schema, obj, privilege_maskInt, rol_spec->rolename, obj_type); else { - int16 old_priv = get_bbf_schema_privilege(logical_schema, obj, rol_spec->rolename); - if(old_priv!= privilege_maskInt || ((old_priv)|(privilege_maskInt)) != old_priv) - update_bbf_schema_privilege(logical_schema, obj, privilege_maskInt, old_priv, rol_spec->rolename, obj_type, grant->is_grant); + int16 old_priv = get_bbf_schema_privilege(logical_schema, obj,rol_spec->rolename); + if(old_priv!= ALL_PRIVILEGE_EXCEPT_EXECUTE || (old_priv|(ALL_PRIVILEGE_EXCEPT_EXECUTE)) != old_priv) + update_bbf_schema_privilege(logical_schema, obj, ALL_PRIVILEGE_EXCEPT_EXECUTE, old_priv, rol_spec->rolename, obj_type, grant->is_grant); } } + break; } else { foreach(lc, grant->grantees) { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); - bool flag = false; - /* - * 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, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rol_spec->rolename)) + bool has_schema_perms = false; + if (check_bbf_schema_for_entry(logical_schema, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, rol_spec->rolename) && !has_schema_perms) + has_schema_perms = true; + if (check_bbf_schema_for_entry(logical_schema, obj, 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); + int16 old_priv = get_bbf_schema_privilege(logical_schema, obj,rol_spec->rolename); + if(old_priv!= ALL_PRIVILEGE_EXCEPT_EXECUTE || ((old_priv)&(~(ALL_PRIVILEGE_EXCEPT_EXECUTE))) != old_priv) + update_bbf_schema_privilege(logical_schema, obj, ALL_PRIVILEGE_EXCEPT_EXECUTE, old_priv, rol_spec->rolename, obj_type, grant->is_grant); } - foreach(lc1, grant->privileges) + if (has_schema_perms) + return; + } + break; + } + } + if (grant->is_grant) + { + /* + * 1. Execute the GRANT statement. + * 2. Add its corresponding entry in the catalog, if doesn't exist already. + * 3. Don't add an entry, if the permission is granted on column list. + */ + if (prev_ProcessUtility) + prev_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, + queryEnv, dest, qc); + else + standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, + queryEnv, dest, qc); + foreach(lc, grant->grantees) + { + RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); + bool permission_is_granted_on_column = false; + foreach(lc1, grant->privileges) + { + AccessPriv *ap = (AccessPriv *) lfirst(lc1); + if(ap->cols != NULL) { - AccessPriv *ap = (AccessPriv *) lfirst(lc1); - if(ap->cols != NULL) - { - flag = true; - break; - } + permission_is_granted_on_column = true; + break; } - if(flag) - continue; - if (check_bbf_schema_for_entry(logical_schema, obj, rol_spec->rolename)) + } + if(permission_is_granted_on_column) + break; + if (!check_bbf_schema_for_entry(logical_schema, obj, rol_spec->rolename)) + add_entry_to_bbf_schema(logical_schema, obj, privilege_maskInt, rol_spec->rolename, obj_type); + else + { + int16 old_priv = get_bbf_schema_privilege(logical_schema, obj, rol_spec->rolename); + if(old_priv!= privilege_maskInt || ((old_priv)|(privilege_maskInt)) != old_priv) + update_bbf_schema_privilege(logical_schema, obj, privilege_maskInt, old_priv, rol_spec->rolename, obj_type, grant->is_grant); + } + } + } + else + { + foreach(lc, grant->grantees) + { + RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); + bool permission_is_granted_on_column = false; + /* + * 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, PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA, 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); + } + foreach(lc1, grant->privileges) + { + AccessPriv *ap = (AccessPriv *) lfirst(lc1); + if(ap->cols != NULL) { - int16 old_priv = get_bbf_schema_privilege(logical_schema, obj,rol_spec->rolename); - if(old_priv!= privilege_maskInt || ((old_priv)&(~(privilege_maskInt))) != old_priv) - update_bbf_schema_privilege(logical_schema, obj, privilege_maskInt, old_priv, rol_spec->rolename, obj_type, grant->is_grant); + permission_is_granted_on_column = true; + break; } } - } - return; + if(permission_is_granted_on_column) + continue; + if (check_bbf_schema_for_entry(logical_schema, obj, rol_spec->rolename)) + { + int16 old_priv = get_bbf_schema_privilege(logical_schema, obj,rol_spec->rolename); + if(old_priv!= privilege_maskInt || ((old_priv)&(~(privilege_maskInt))) != old_priv) + update_bbf_schema_privilege(logical_schema, obj, privilege_maskInt, old_priv, rol_spec->rolename, obj_type, grant->is_grant); + } + } } + return; } else if ((grant->objtype == OBJECT_PROCEDURE) || (grant->objtype == OBJECT_FUNCTION)) { @@ -3795,12 +3791,12 @@ bbf_ProcessUtility(PlannedStmt *pstmt, { RoleSpec *rol_spec = (RoleSpec *) lfirst(lc); if (!check_bbf_schema_for_entry(logicalschema, funcname, rol_spec->rolename)) - add_entry_to_bbf_schema(logicalschema, funcname, 32, rol_spec->rolename, obj_type); + add_entry_to_bbf_schema(logicalschema, funcname, PRIVILEGE_BIT_FOR_EXECUTE, rol_spec->rolename, obj_type); else { int16 old_priv = get_bbf_schema_privilege(logicalschema, funcname, rol_spec->rolename); - if(old_priv!= 32 || ((old_priv)|(32)) != old_priv) - update_bbf_schema_privilege(logicalschema, funcname, 32, old_priv, rol_spec->rolename, obj_type, grant->is_grant); + if(old_priv!= PRIVILEGE_BIT_FOR_EXECUTE || ((old_priv)|(PRIVILEGE_BIT_FOR_EXECUTE)) != old_priv) + update_bbf_schema_privilege(logicalschema, funcname, PRIVILEGE_BIT_FOR_EXECUTE, old_priv, rol_spec->rolename, obj_type, grant->is_grant); } } } @@ -3816,8 +3812,8 @@ bbf_ProcessUtility(PlannedStmt *pstmt, if (check_bbf_schema_for_entry(logicalschema, funcname, rol_spec->rolename)) { int16 old_priv = get_bbf_schema_privilege(logicalschema, funcname,rol_spec->rolename); - if(old_priv!= 32 || ((old_priv)&(~(32))) != old_priv) - update_bbf_schema_privilege(logicalschema, funcname, 32, old_priv, rol_spec->rolename, obj_type, grant->is_grant); + if(old_priv!= PRIVILEGE_BIT_FOR_EXECUTE || ((old_priv)&(~(PRIVILEGE_BIT_FOR_EXECUTE))) != old_priv) + update_bbf_schema_privilege(logicalschema, funcname, PRIVILEGE_BIT_FOR_EXECUTE, old_priv, rol_spec->rolename, obj_type, grant->is_grant); } if (has_schema_perms) return;