Bug fixes and performance overhead reduction for GRANT .. ON SCHEMA #2020
Conversation
|
|
||
| bbf_schema_rel = table_open(get_bbf_schema_perms_oid(), | ||
| AccessShareLock); | ||
| ScanKeyInit(&key[0], |
There was a problem hiding this comment.
This all usage of ScanKeyInit will choose "C" collation (default collation) for the lookup. Do we have sufficient test cases to prove that it works fine in term of case-insensitive searches/update/delete?
There was a problem hiding this comment.
We are always storing the downcase string in the catalog. The search/update/delete always happen on the downcase value. We have tested for Upper case, down case and mixed case string.
| CStringGetDatum(grantee)); | ||
|
|
||
| scan = table_beginscan_catalog(bbf_schema_rel, 4, key); | ||
| tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection); |
There was a problem hiding this comment.
do we need to free this tuple?
| 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); | ||
| int16 old_priv = get_bbf_schema_privilege(logical_schema, obj,rol_spec->rolename); | ||
| if(old_priv!= 31 || (old_priv|(31)) != old_priv) |
| int i = 0; | ||
| char *permissions[] = {"select", "insert", "update", "references", "delete"}; | ||
| for(i = 0; i < 5; i++) | ||
| if((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0) && !check_bbf_schema_for_entry(logical_schema, obj,rol_spec->rolename)) |
There was a problem hiding this comment.
Please add a comment what the following code is trying to do.
| if ((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0) && !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 if((rol_spec->rolename != NULL) && (strcmp(rol_spec->rolename, "public") != 0)) | ||
| { | ||
| 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_entry(logical_schema, obj, privilege_maskInt, old_priv, rol_spec->rolename, obj_type, grant->is_grant); |
There was a problem hiding this comment.
This code appears in 7-8 places in the code. Please generalize and decouple this in a separate function (probably inline).
test/JDBC/expected/BABEL-GRANT.out
Outdated
| @@ -106,19 +106,19 @@ GO | |||
| REVOKE ALL ON OBJECT::t1 TO guest; | |||
| GO | |||
|
|
|||
| REVOKE ALL ON OBJECT::seq_tinyint TO guest; | |||
| REVOKE ALL ON OBJECT::seq_tinyint FROM guest; | |||
There was a problem hiding this comment.
Why changing from TO to FROM? This should both be supported right?
Description
Bug fixes and performance overhead reduction for GRANT .. ON SCHEMA
schema::[]schema::""Issues Resolved
JIRA: BABEL-4344
Co-authored-by: Anju Bharti
Signed-off-by: Shalini Lohia [email protected]
Test Scenarios Covered
Use case based - Added
Boundary conditions -
Arbitrary inputs -
Negative test cases -
Minor version upgrade tests -
Major version upgrade tests -
Performance tests -
Tooling impact -
Client tests -
Check List
By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.