New Object Access Type hooks
Mark Dilger <mark.dilger@enterprisedb.com>
From: Mark Dilger <mark.dilger@enterprisedb.com>
To: PostgreSQL-development <pgsql-hackers@postgresql.org>,
Joshua Brindle <joshua.brindle@crunchydata.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
Tom Lane <tgl@sss.pgh.pa.us>,
Andrew Dunstan <andrew@dunslane.net>,
Jeff Davis <pgsql@j-davis.com>,
Joe Conway <joe@crunchydata.com>
Date: 2022-03-18T03:21:56Z
Lists: pgsql-hackers
Attachments
- v1-0001-Add-String-object-access-hooks.patch (application/octet-stream) patch v1-0001
- v1-0002-Add-regression-tests-of-Object-Access-Type-hooks.patch (application/octet-stream) patch v1-0002
- (unnamed) (text/plain)
Hackers, Over in [1], Joshua proposed a new set of Object Access Type hooks based on strings rather than Oids. His patch was written to be applied atop my patch for granting privileges on gucs. On review of his patch, I became uncomfortable with the complete lack of regression test coverage. To be fair, he did paste a bit of testing logic to the thread, but it appears to be based on pgaudit, and it is unclear how to include such a test in the core project, where pgaudit is not assumed to be installed. First, I refactored his patch to work against HEAD and not depend on my GUCs patch. Find that as v1-0001. The refactoring exposed a bit of a problem. To call the new hook for SET and ALTER SYSTEM commands, I need to pass in the Oid of a catalog table. But since my GUC patch isn't applied yet, there isn't any such table (pg_setting_acl or whatnot) to pass. So I'm passing InvalidOid, but I don't know if that is right. In any event, if we want a new API like this, we should think a bit harder about whether it can be used to check operations where no table Oid is applicable. Second, I added a new test directory, src/test/modules/test_oat_hooks, which includes a new loadable module with hook implementations and a regression test for testing the object access hooks. The main point of the test is to log which hooks get called in which order, and which hooks do or do not get called when other hooks allow or deny access. That information shows up in the expected output as NOTICE messages. This second patch has gotten a little long, and I'd like another pair of eyes on this before spending a second day on the effort. Please note that this is a quick WIP patch in response to the patch Joshua posted earlier today. Sorry for sometimes missing function comments, etc. The goal, if this design seems acceptable, is to polish this, hopefully with Joshua's assistance, and get it committed *before* my GUCs patch, so that my patch can be rebased to use it. Otherwise, if this is rejected, I can continue on the GUC patch without this. (FYI, I got a test failure from src/test/recovery/t/013_crash_restart.pl when testing v1-0001. I'm not sure yet what that is about.)
Commits
-
Update some tests in 013_crash_restart.pl.
- 8803df4ea960 14.3 landed
- 7a43a1fc52d0 15.0 landed
-
Avoid freeing objects during json aggregate finalization
- 4eb979887968 15.0 cited
-
SQL/JSON constructors
- f4fb45d15c59 15.0 cited
-
Force NO_LOCALE / UTF8 for test_oat_hooks tests
- 7c51b7f7cc08 15.0 landed
-
Temporarily disable installcheck for test_oat_hooks module
- 5b29a9f77297 15.0 landed
-
Fix new Object Access hooks test
- f0206d99520e 15.0 landed
-
Add String object access hooks
- d11e84ea466b 15.0 landed
-
Add a test module for Object Access hooks
- 90efa2f5565d 15.0 landed