Skip to content

Allow seeing query texts for inherited roles in pg_stat_statements (#20) #685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

dimitri
Copy link

@dimitri dimitri commented Jul 16, 2025

Today, if you don't have pg_monitor (pg_read_all_stats, specifically), you can only see your own query texts in pg_stat_statements. This limits the usefulness quite a bit because we're not ready to give out pg_monitor yet.

pg_stat_activity however is more lax and lets you see the query text of roles you inherit the privileges of. See
here.

This PR aligns the behavior of pg_stat_statements with pg_stat_activity. Later in PrPr when we give out pg_monitor, we can choose whether we keep this or not.

I manually verified the new behavior. Will follow up with an automated test as a fast-follow to PrPr for the final behavior when we give out pg_monitor. Manual test:

SET ROLE cloud_admin
-- Create a role u1 simulating endpoint creator.
CREATE ROLE u1 WITH CREATEROLE;
GRANT databricks_superuser TO u1;
GRANT ALL PRIVILEGES ON SCHEMA public TO u1 WITH GRANT OPTION;

-- Now, as endpoint creator, create a second user.
SET ROLE u1;
CREATE ROLE u2;
GRANT ALL PRIVILEGES ON SCHEMA public to u2;

-- As the second user, create a dingding table.
SET ROLE u2;
CREATE TABLE tbl_dingding(a INT);
INSERT INTO tbl_dingding VALUES (1);

-- Now, as endpoint creator again, try to see what u2 did.
-- This is NOT visible because u1 doesn't have u2's privileges.
SET ROLE u1;
CREATE EXTENSION pg_stat_statements
SELECT * FROM pg_stat_statements WHERE query LIKE '%tbl_dingding%'	

-- But since u1 created u2, u1 has ADMIN on u2 and can grant itself its privileges.
-- Now u1 can see u2's dingding query. This is only possible because of the change in this PR.
GRANT u2 to u1;
SELECT * FROM pg_stat_statements WHERE query LIKE '%tbl_dingding%'

Today, if you don't have pg_monitor (pg_read_all_stats, specifically),
you can only see your own query texts in pg_stat_statements. This limits
the usefulness quite a bit because we're not ready to give out
pg_monitor yet.

pg_stat_activity however is more lax and lets you see the query text of
roles you inherit the privileges of. See
[here](https://github.com/databricks-eng/postgres/blob/REL_16_STABLE_hadron/src/backend/utils/adt/pgstatfuncs.c#L39).

This PR aligns the behavior of pg_stat_statements with pg_stat_activity.
Later in PrPr when we give out pg_monitor, we can choose whether we keep
this or not.

I manually verified the new behavior. Will follow up with an automated
test as a fast-follow to PrPr for the final behavior when we give out
pg_monitor. Manual test:

```sql
SET ROLE cloud_admin
-- Create a role u1 simulating endpoint creator.
CREATE ROLE u1 WITH CREATEROLE;
GRANT databricks_superuser TO u1;
GRANT ALL PRIVILEGES ON SCHEMA public TO u1 WITH GRANT OPTION;

-- Now, as endpoint creator, create a second user.
SET ROLE u1;
CREATE ROLE u2;
GRANT ALL PRIVILEGES ON SCHEMA public to u2;

-- As the second user, create a dingding table.
SET ROLE u2;
CREATE TABLE tbl_dingding(a INT);
INSERT INTO tbl_dingding VALUES (1);

-- Now, as endpoint creator again, try to see what u2 did.
-- This is NOT visible because u1 doesn't have u2's privileges.
SET ROLE u1;
CREATE EXTENSION pg_stat_statements
SELECT * FROM pg_stat_statements WHERE query LIKE '%tbl_dingding%'	

-- But since u1 created u2, u1 has ADMIN on u2 and can grant itself its privileges.
-- Now u1 can see u2's dingding query. This is only possible because of the change in this PR.
GRANT u2 to u1;
SELECT * FROM pg_stat_statements WHERE query LIKE '%tbl_dingding%'
```

---------

Co-authored-by: Hans Norheim <[email protected]>
@dimitri dimitri requested review from MMeent and tristan957 July 16, 2025 11:39
@MMeent
Copy link

MMeent commented Jul 16, 2025

Have you proposed to apply this in upstream as well?

I'm a bit hesitant to change behaviour of popular extensions like this without at least polling the upstreamability of those changes.

@dimitri
Copy link
Author

dimitri commented Jul 17, 2025

This is no longer needed as we can give pg_monitor to neon_superuser instead.

@dimitri dimitri closed this Jul 17, 2025
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.

3 participants