Skip to content

Spr 805 policy tables#543

Merged
garthgoodson merged 51 commits into
mainfrom
SPR-805-policy-tables
Aug 28, 2025
Merged

Spr 805 policy tables#543
garthgoodson merged 51 commits into
mainfrom
SPR-805-policy-tables

Conversation

@garthgoodson

@garthgoodson garthgoodson commented Jul 14, 2025

Copy link
Copy Markdown
Contributor
  • Added row level security columns to table names system table rls_enabled and rls_forced

  • Added sync for roles, role membership and table policies

  • Added sync for table ownership only for tables that have row policies -- NO TEST FOR THIS YET.

  • Other cleanup to some of the other tests

  • Random other cleanup

@linear

linear Bot commented Jul 14, 2025

Copy link
Copy Markdown
SPR-805 Policy tables and trigger functions to support Row level security + policies

  • Add new system table for policies
  • Add new table column for flag enabling/disabling row level security
  • Patch Postgres to enable policies for FDW
  • Add function to trigger to update internal policy table and to sync policy changes

@garthgoodson garthgoodson requested a review from craigsoules July 22, 2025 23:12
@garthgoodson garthgoodson marked this pull request as ready for review July 22, 2025 23:13

@craigsoules craigsoules left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor comments.

Comment thread include/pg_fdw/pg_ddl_mgr.hh Outdated
Comment on lines +123 to +124
const std::unordered_map<uint64_t, std::unordered_map<uint64_t,
std::pair<std::string, std::string>>> &user_types);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe define some types for these?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

* @param cmd SQL command
* @return true if result is successful; false otherwise; check result separately
*/
bool exec_no_throw(const std::string &cmd, bool use_savepoint = true);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these can't throw exceptions, let's mark them as noexcept?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is badly named, but it won't throw an exception on a failed query, but other exceptions may be thrown. The normal version of exec() throws an exception on a query failure.

Comment thread src/pg_fdw/pg_ddl_mgr.cc Outdated
static constexpr char CREATE_FDW_USER[] =
"CREATE USER {} WITH LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD '{}'";
static constexpr char CREATE_USER[] =
"CREATE USER {} WITH LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD '{}' IN ROLE pg_read_all_data";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is pg_read_all_data a role that we create elsewhere? Or is it guaranteed to exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a postgres system role; so it exists.

@garthgoodson garthgoodson Aug 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do need to verify that this works for new table creates. But will need the user mgmt through the proxy to verify, so will do that in a separate PR.

@sonarqubecloud

Copy link
Copy Markdown

@garthgoodson garthgoodson merged commit 753f990 into main Aug 28, 2025
6 checks passed
@garthgoodson garthgoodson deleted the SPR-805-policy-tables branch August 28, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants