Fix bugs for GRANT/REVOKE on SCHEMA #2031

shalinilohia50 · 2023-11-17T08:03:43Z

Description

Bug fixes for GRANT/REVOKE .. ON SCHEMA

Server crash when using empty bracketed name. schema::[]
Server crash when using empty quoted identifier. schema::""
When granting permission to yourself, we need a different error message.
Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords
Not-supported object type in error message should be in uppercase
GRANT ON SCHEMA:: TO PUBLIC does not take effect
Specified collation for column names using NAME datatype

Issues Resolved

JIRA: BABEL-4344
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

Commits are signed per the DCO using --signoff

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.

contrib/babelfishpg_tsql/sql/ownership.sql

contrib/babelfishpg_tsql/src/pl_exec-2.c

contrib/babelfishpg_tsql/src/tsqlIface.cpp

contrib/babelfishpg_tsql/sql/ownership.sql

contrib/babelfishpg_tsql/src/pl_exec-2.c

contrib/babelfishpg_tsql/sql/ownership.sql

Deepesh125

Collations changes looks okay to me!

contrib/babelfishpg_tsql/src/catalog.c

contrib/babelfishpg_tsql/src/pl_exec-2.c

KushaalShroff · 2023-11-20T08:26:23Z

contrib/babelfishpg_tsql/src/pl_exec-2.c

 				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)


How will this work for multi-db?
(strcmp(rolname, user) == 0)

Can you suggest a case which might fail?
I have ran the test framework in both single-db and multi-db mode.

How is user = public ever possible?

That is not possible. User will not be public.

rolname can be public, but not user.

contrib/babelfishpg_tsql/src/tsqlIface.cpp

contrib/babelfishpg_tsql/src/pl_handler.c

contrib/babelfishpg_tsql/src/catalog.c

contrib/babelfishpg_tsql/src/pl_handler.c

contrib/babelfishpg_tsql/src/catalog.h

KushaalShroff · 2023-11-21T09:07:55Z

contrib/babelfishpg_tsql/src/pl_exec-2.c

 				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)


How is user = public ever possible?

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

…#2031)" This reverts commit dd23da9.

This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

…#2031)" (babelfish-for-postgresql#2051) This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

…#2031)" (babelfish-for-postgresql#2051) This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

…#2031)" (babelfish-for-postgresql#2051) This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

…#2031)" (babelfish-for-postgresql#2051) This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

…#2031)" (babelfish-for-postgresql#2051) This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

…#2031)" (babelfish-for-postgresql#2051) This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Bug fixes for GRANT/REVOKE .. ON SCHEMA 1. Server crash when using empty bracketed name. `schema::[]` 2. Server crash when using empty quoted identifier. `schema::""` 3. When granting permission to yourself, we need a different error message. 4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords 5. Not-supported object type in error message should be in uppercase 6. GRANT ON SCHEMA:: TO PUBLIC does not take effect 7. Specified collation for column names using NAME datatype Issues Resolved : BABEL-4344 Signed-off-by: Shalini Lohia <[email protected]>

This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

lohia-shalini added 2 commits November 17, 2023 08:00

Fix bugs for GRANT/REVOKE on SCHEMA

60bab74

Add collate

c765748

shalinilohia50 requested review from Deepesh125, kuntalghosh and xhfanhe and removed request for Deepesh125 November 17, 2023 08:37

lohia-shalini added 2 commits November 17, 2023 09:19

Update collation to sys.database_default

72155d5

Merge branch 'BABEL_3_X_DEV' into fix-bugs-4344

8d36d50

Deepesh125 requested changes Nov 17, 2023

View reviewed changes

contrib/babelfishpg_tsql/sql/ownership.sql Outdated Show resolved Hide resolved

lohia-shalini added 2 commits November 17, 2023 10:45

Address comments

26f1408

Empty commit

762b4b9

xhfanhe reviewed Nov 17, 2023

View reviewed changes

contrib/babelfishpg_tsql/src/pl_exec-2.c Show resolved Hide resolved

contrib/babelfishpg_tsql/src/tsqlIface.cpp Outdated Show resolved Hide resolved

Use pstrdup

bf3844d

shalinilohia50 requested review from xhfanhe and Deepesh125 November 17, 2023 18:26

xhfanhe reviewed Nov 17, 2023

View reviewed changes

contrib/babelfishpg_tsql/sql/ownership.sql Show resolved Hide resolved

Add test

836f8af

xhfanhe previously approved these changes Nov 17, 2023

View reviewed changes

Fix sql validation test

9edc046

shalinilohia50 dismissed xhfanhe’s stale review via 9edc046 November 17, 2023 21:21

xhfanhe previously approved these changes Nov 17, 2023

View reviewed changes

Deepesh125 requested changes Nov 20, 2023

View reviewed changes

contrib/babelfishpg_tsql/src/pl_exec-2.c Show resolved Hide resolved

Empty commit

32307b1

shalinilohia50 requested a review from Deepesh125 November 20, 2023 05:59

KushaalShroff requested changes Nov 20, 2023

View reviewed changes

contrib/babelfishpg_tsql/sql/ownership.sql Show resolved Hide resolved

Deepesh125 previously approved these changes Nov 20, 2023

View reviewed changes

KushaalShroff requested changes Nov 20, 2023

View reviewed changes

kuntalghosh requested changes Nov 20, 2023

View reviewed changes

Address comments

4da4b77

Add a macro

c73c907

shalinilohia50 requested review from kuntalghosh, xhfanhe and KushaalShroff November 21, 2023 04:32

kuntalghosh previously approved these changes Nov 21, 2023

View reviewed changes

KushaalShroff reviewed Nov 21, 2023

View reviewed changes

Change macro name

2296878

shalinilohia50 dismissed kuntalghosh’s stale review via 2296878 November 21, 2023 09:40

shalinilohia50 requested a review from KushaalShroff November 21, 2023 10:02

KushaalShroff approved these changes Nov 21, 2023

View reviewed changes

shardgupta merged commit dd23da9 into babelfish-for-postgresql:BABEL_3_X_DEV Nov 21, 2023
29 checks passed

shardgupta deleted the fix-bugs-4344 branch November 21, 2023 11:15

shalinilohia50 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 22, 2023

Revert "Fix bugs for GRANT/REVOKE on SCHEMA (babelfish-for-postgresql…

52eb46e

…#2031)" This reverts commit dd23da9.

forestkeeper pushed a commit that referenced this pull request Nov 22, 2023

Revert "Fix bugs for GRANT/REVOKE on SCHEMA (#2031)" (#2051)

0cabf83

This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

KushaalShroff mentioned this pull request Nov 28, 2023

Bug fixes and performance overhead reduction for GRANT .. ON SCHEMA #2020

Closed

1 task

priyansx pushed a commit that referenced this pull request Dec 29, 2023

Revert "Fix bugs for GRANT/REVOKE on SCHEMA (#2031)" (#2051)

45171e1

This reverts commit dd23da9. Co-authored-by: Shalini Lohia <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bugs for GRANT/REVOKE on SCHEMA #2031

Fix bugs for GRANT/REVOKE on SCHEMA #2031

shalinilohia50 commented Nov 17, 2023 •

edited

Loading

Deepesh125 left a comment

KushaalShroff Nov 20, 2023

shalinilohia50 Nov 20, 2023

KushaalShroff Nov 21, 2023

shalinilohia50 Nov 21, 2023

shalinilohia50 Nov 21, 2023

KushaalShroff Nov 21, 2023

Fix bugs for GRANT/REVOKE on SCHEMA #2031

Fix bugs for GRANT/REVOKE on SCHEMA #2031

Conversation

shalinilohia50 commented Nov 17, 2023 • edited Loading

Description

Issues Resolved

Test Scenarios Covered

Check List

Deepesh125 left a comment

Choose a reason for hiding this comment

KushaalShroff Nov 20, 2023

Choose a reason for hiding this comment

shalinilohia50 Nov 20, 2023

Choose a reason for hiding this comment

KushaalShroff Nov 21, 2023

Choose a reason for hiding this comment

shalinilohia50 Nov 21, 2023

Choose a reason for hiding this comment

shalinilohia50 Nov 21, 2023

Choose a reason for hiding this comment

KushaalShroff Nov 21, 2023

Choose a reason for hiding this comment

shalinilohia50 commented Nov 17, 2023 •

edited

Loading