allow_system_table_mods stuff

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: allow_system_table_mods stuff
Date: 2019-06-21 09:12:38
Message-ID: 8b00ea5e-28a7-88ba-e848-21528b632354@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

After the earlier thread [0] that dealt with ALTER TABLE on system
catalogs, I took a closer look at the allow_system_table_mods setting.
I found a few oddities, and it seems there is some room for improvement.

Attached are some patches to get the discussion rolling: One patch makes
allow_system_table_mods settable at run time by superuser, the second
one is a test suite that documents the current behavior that I gathered
after analyzing the source code, the third one removes some code that
was found useless by the tests. (The first patch might be useful on its
own, but right now it's just to facilitate the test suite.)

Some observations:

- For the most part, a_s_t_m establishes an additional level of access
control on top of superuserdom for doing DDL on system catalogs. That
seems like a useful definition.

- But enabling a_s_t_m also allows a non-superuser to do DML on system
catalogs. That seems like an entirely unrelated and surprising behavior.

- Some checks are redundant with the pinning concept of the dependency
system. For example, you can't drop a system catalog even with a_s_t_m
on. That seems useful, of course, but as a result there is a bit of
dead or useless code around. (The dependency system is newer than a_s_t_m.)

- The source code comments indicate that SET STATISTICS on system
catalogs is supposed to be allowed without a_s_t_m, but it actually
doesn't work.

Proposals and discussion points:

- Having a test suite like this seems useful.

- The behavior that a_s_t_m allows non-superusers to do DML on system
catalogs should be removed. (Regular permissions can be used for that.)

- Things that are useful in normal use, for example SET STATISTICS, some
or all reloptions, should always be allowed (subject to other access
control).

- There is currently no support in pg_dump to preserve any of those
changes. Maybe that's not a big problem.

- Dead code or code that is redundant with pinning should be removed.

Any other thoughts?

[0]:
https://www.postgresql.org/message-id/flat/e49f825b-fb25-0bc8-8afc-d5ad895c7975%402ndquadrant.com

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
v1-0001-Change-allow_system_table_mods-to-SUSET.patch text/plain 1.3 KB
v1-0002-Add-tests-for-allow_system_table_mods.patch text/plain 12.1 KB
v1-0003-Disable-dead-code.patch text/plain 1.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-06-21 10:25:50 Re: O(N^2) when building multi-column MCV lists
Previous Message Dean Rasheed 2019-06-21 07:50:33 Re: Choosing values for multivariate MCV lists