Commit Graph

41 Commits

Author SHA1 Message Date
Nathan Bossart ecb0fd3372 Reintroduce MAINTAIN privilege and pg_maintain predefined role.
Roles with MAINTAIN on a relation may run VACUUM, ANALYZE, REINDEX,
REFRESH MATERIALIZE VIEW, CLUSTER, and LOCK TABLE on the relation.
Roles with privileges of pg_maintain may run those same commands on
all relations.

This was previously committed for v16, but it was reverted in
commit 151c22deee due to concerns about search_path tricks that
could be used to escalate privileges to the table owner.  Commits
2af07e2f74, 59825d1639, and c7ea3f4229 resolved these concerns by
restricting search_path when running maintenance commands.

Bumps catversion.

Reviewed-by: Jeff Davis
Discussion: https://postgr.es/m/20240305161235.GA3478007%40nathanxps13
2024-03-13 14:49:26 -05:00
Alvaro Herrera b0e96f3119
Catalog not-null constraints
We now create contype='n' pg_constraint rows for not-null constraints.

We propagate these constraints to other tables during operations such as
adding inheritance relationships, creating and attaching partitions and
creating tables LIKE other tables.  We also spawn not-null constraints
for inheritance child tables when their parents have primary keys.
These related constraints mostly follow the well-known rules of
conislocal and coninhcount that we have for CHECK constraints, with some
adaptations: for example, as opposed to CHECK constraints, we don't
match not-null ones by name when descending a hierarchy to alter it,
instead matching by column name that they apply to.  This means we don't
require the constraint names to be identical across a hierarchy.

For now, we omit them for system catalogs.  Maybe this is worth
reconsidering.  We don't support NOT VALID nor DEFERRABLE clauses
either; these can be added as separate features later (this patch is
already large and complicated enough.)

psql shows these constraints in \d+.

pg_dump requires some ad-hoc hacks, particularly when dumping a primary
key.  We now create one "throwaway" not-null constraint for each column
in the PK together with the CREATE TABLE command, and once the PK is
created, all those throwaway constraints are removed.  This avoids
having to check each tuple for nullness when the dump restores the
primary key creation.

pg_upgrading from an older release requires a somewhat brittle procedure
to create a constraint state that matches what would be created if the
database were being created fresh in Postgres 17.  I have tested all the
scenarios I could think of, and it works correctly as far as I can tell,
but I could have neglected weird cases.

This patch has been very long in the making.  The first patch was
written by Bernd Helmle in 2010 to add a new pg_constraint.contype value
('n'), which I (Álvaro) then hijacked in 2011 and 2012, until that one
was killed by the realization that we ought to use contype='c' instead:
manufactured CHECK constraints.  However, later SQL standard
development, as well as nonobvious emergent properties of that design
(mostly, failure to distinguish them from "normal" CHECK constraints as
well as the performance implication of having to test the CHECK
expression) led us to reconsider this choice, so now the current
implementation uses contype='n' again.  During Postgres 16 this had
already been introduced by commit e056c557ae, but there were some
problems mainly with the pg_upgrade procedure that couldn't be fixed in
reasonable time, so it was reverted.

In 2016 Vitaly Burovoy also worked on this feature[1] but found no
consensus for his proposed approach, which was claimed to be closer to
the letter of the standard, requiring an additional pg_attribute column
to track the OID of the not-null constraint for that column.
[1] https://postgr.es/m/CAKOSWNkN6HSyatuys8xZxzRCR-KL1OkHS5-b9qd9bf1Rad3PLA@mail.gmail.com

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Author: Bernd Helmle <mailings@oopsware.de>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
2023-08-25 13:31:24 +02:00
Nathan Bossart 151c22deee Revert MAINTAIN privilege and pg_maintain predefined role.
This reverts the following commits: 4dbdb82513, c2122aae63,
5b1a879943, 9e1e9d6560, ff9618e82a, 60684dd834, 4441fc704d,
and b5d6382496.  A role with the MAINTAIN privilege may be able to
use search_path tricks to escalate privileges to the table owner.
Unfortunately, it is too late in the v16 development cycle to apply
the proposed fix, i.e., restricting search_path when running
maintenance commands.

Bumps catversion.

Reviewed-by: Jeff Davis
Discussion: https://postgr.es/m/E1q7j7Y-000z1H-Hr%40gemulon.postgresql.org
Backpatch-through: 16
2023-07-07 11:25:13 -07:00
Nathan Bossart 4dbdb82513 Fix cache lookup hazards introduced by ff9618e82a.
ff9618e82a introduced has_partition_ancestor_privs(), which is used
to check whether a user has MAINTAIN on any partition ancestors.
This involves syscache lookups, and presently this function does
not take any relation locks, so it is likely subject to the same
kind of cache lookup failures that were fixed by 19de0ab23c.

To fix this problem, this commit partially reverts ff9618e82a.
Specifically, it removes the partition-related changes, including
the has_partition_ancestor_privs() function mentioned above.  This
means that MAINTAIN on a partitioned table is no longer sufficient
to perform maintenance commands on its partitions.  This is more
like how privileges for maintenance commands work on supported
versions.  Privileges are checked for each partition, so a command
that flows down to all partitions might refuse to process them
(e.g., if the current user doesn't have MAINTAIN on the partition).

In passing, adjust a few related comments and error messages, and
add a test for the privilege checks for CLUSTER on a partitioned
table.

Reviewed-by: Michael Paquier, Jeff Davis
Discussion: https://postgr.es/m/20230613211246.GA219055%40nathanxps13
2023-06-22 15:48:20 -07:00
Alvaro Herrera 9ce04b50e1
Revert "Catalog NOT NULL constraints" and fallout
This reverts commit e056c557ae and minor later fixes thereof.

There's a few problems in this new feature -- most notably regarding
pg_upgrade behavior, but others as well.  This new feature is not in any
way critical on its own, so instead of scrambling to fix it we revert it
and try again in early 17 with these issues in mind.

Discussion: https://postgr.es/m/3801207.1681057430@sss.pgh.pa.us
2023-04-12 19:29:21 +02:00
Alvaro Herrera e056c557ae
Catalog NOT NULL constraints
We now create pg_constaint rows for NOT NULL constraints with
contype='n'.

We propagate these constraints during operations such as adding
inheritance relationships, creating and attaching partitions, creating
tables LIKE other tables.  We mostly follow the well-known rules of
conislocal and coninhcount that we have for CHECK constraints, with some
adaptations; for example, as opposed to CHECK constraints, we don't
match NOT NULL ones by name when descending a hierarchy to alter it;
instead we match by column number.  This means we don't require the
constraint names to be identical across a hierarchy.

For now, we omit them from system catalogs.  Maybe this is worth
reconsidering.  We don't support NOT VALID nor DEFERRABLE clauses
either; these can be added as separate features later (this patch is
already large and complicated enough.)

This has been very long in the making.  The first patch was written by
Bernd Helmle in 2010 to add a new pg_constraint.contype value ('n'),
which I (Álvaro) then hijacked in 2011 and 2012, until that one was
killed by the realization that we ought to use contype='c' instead:
manufactured CHECK constraints.  However, later SQL standard
development, as well as nonobvious emergent properties of that design
(mostly, failure to distinguish them from "normal" CHECK constraints as
well as the performance implication of having to test the CHECK
expression) led us to reconsider this choice, so now the current
implementation uses contype='n' again.

In 2016 Vitaly Burovoy also worked on this feature[1] but found no
consensus for his proposed approach, which was claimed to be closer to
the letter of the standard, requiring additional pg_attribute columns to
track the OID of the NOT NULL constraint for that column.
[1] https://postgr.es/m/CAKOSWNkN6HSyatuys8xZxzRCR-KL1OkHS5-b9qd9bf1Rad3PLA@mail.gmail.com

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Author: Bernd Helmle <mailings@oopsware.de>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>

Discussion: https://postgr.es/m/CACA0E642A0267EDA387AF2B%40%5B172.26.14.62%5D
Discussion: https://postgr.es/m/AANLkTinLXMOEMz+0J29tf1POokKi4XDkWJ6-DDR9BKgU@mail.gmail.com
Discussion: https://postgr.es/m/20110707213401.GA27098@alvh.no-ip.org
Discussion: https://postgr.es/m/1343682669-sup-2532@alvh.no-ip.org
Discussion: https://postgr.es/m/CAKOSWNkN6HSyatuys8xZxzRCR-KL1OkHS5-b9qd9bf1Rad3PLA@mail.gmail.com
Discussion: https://postgr.es/m/20220817181249.q7qvj3okywctra3c@alvherre.pgsql
2023-04-07 19:59:57 +02:00
Jeff Davis ff9618e82a Fix MAINTAIN privileges for toast tables and partitions.
Commit 60684dd8 left loose ends when it came to maintaining toast
tables or partitions.

For toast tables, simply skip the privilege check if the toast table
is an indirect target of the maintenance command, because the main
table privileges have already been checked.

For partitions, allow the maintenance command if the user has the
MAINTAIN privilege on the partition or any parent.

Also make CLUSTER emit "skipping" messages when the user doesn't have
privileges, similar to VACUUM.

Author: Nathan Bossart
Reported-by: Pavel Luzanov
Reviewed-by: Pavel Luzanov, Ted Yu
Discussion: https://postgr.es/m/20230113231339.GA2422750@nathanxps13
2023-01-14 00:16:23 -08:00
Peter Geoghegan 8ab0ebb9a8 Fix CLUSTER tuplesorts on abbreviated expressions.
CLUSTER sort won't use the datum1 SortTuple field when clustering
against an index whose leading key is an expression.  This makes it
unsafe to use the abbreviated keys optimization, which was missed by the
logic that sets up SortSupport state.  Affected tuplesorts output tuples
in a completely bogus order as a result (the wrong SortSupport based
comparator was used for the leading attribute).

This issue is similar to the bug fixed on the master branch by recent
commit cc58eecc5d.  But it's a far older issue, that dates back to the
introduction of the abbreviated keys optimization by commit 4ea51cdfe8.

Backpatch to all supported versions.

Author: Peter Geoghegan <pg@bowt.ie>
Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA+hUKG+bA+bmwD36_oDxAoLrCwZjVtST2fqe=b4=qZcmU7u89A@mail.gmail.com
Backpatch: 10-
2022-04-20 17:17:43 -07:00
Alvaro Herrera 3f19e176ae
Have CLUSTER ignore partitions not owned by caller
If a partitioned table has partitions owned by roles other than the
owner of the partitioned table, don't include them in the to-be-
clustered list.  This is similar to what VACUUM FULL does (except we do
it sooner, because there is no reason to postpone it).  Add a simple
test to verify that only owned partitions are clustered.

While at it, change memory context switch-and-back to occur once per
partition instead of outside of the loop.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20220411140609.GF26620@telsasoft.com
2022-04-14 22:11:06 +02:00
Alvaro Herrera cfdd03f45e
Allow CLUSTER on partitioned tables
This is essentially the same as applying VACUUM FULL to a partitioned
table, which has been supported since commit 3c3bb99330 (March 2017).
While there's no great use case in applying CLUSTER to partitioned
tables, we don't have any strong reason not to allow it either.

For now, partitioned indexes cannot be marked clustered, so an index
must always be specified.

While at it, rename some variables that were RangeVars during the
development that led to 8bc717cb88 but never made it that way to the
source tree; there's no need to perpetuate names that have always been
more confusing than helpful.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/20201028003312.GU9241@telsasoft.com
Discussion: https://postgr.es/m/20200611153502.GT14879@telsasoft.com
2022-04-02 19:08:34 +02:00
Tom Lane 9d523119fd Avoid possible crash while finishing up a heap rewrite.
end_heap_rewrite was not careful to ensure that the target relation
is open at the smgr level before performing its final smgrimmedsync.
In ordinary cases this is no problem, because it would have been
opened earlier during the rewrite.  However a crash can be reproduced
by re-clustering an empty table with CLOBBER_CACHE_ALWAYS enabled.

Although that exact scenario does not crash in v13, I think that's
a chance result of unrelated planner changes, and the problem is
likely still reachable with other test cases.  The true proximate
cause of this failure is commit c6b92041d, which replaced a call to
heap_sync (which was careful about opening smgr) with a direct call
to smgrimmedsync.  Hence, back-patch to v13.

Amul Sul, per report from Neha Sharma; cosmetic changes
and test case by me.

Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com
2021-03-23 11:24:16 -04:00
Andres Freund cef82eda14 Fix CLUSTER on expression indexes.
Since the introduction of different slot types, in 1a0586de36, we
create a virtual slot in tuplesort_begin_cluster(). While that looks
right, it unfortunately doesn't actually work, as ExecStoreHeapTuple()
is used to store tuples in the slot. Unfortunately no regression tests
for CLUSTER on expression indexes existed so far.

Fix the slot type, and add bare bones tests for CLUSTER on expression
indexes.

Reported-By: Justin Pryzby
Author: Andres Freund
Discussion: https://postgr.es/m/20191011210320.GS10470@telsasoft.com
Backpatch: 12, like 1a0586de36
2019-10-15 10:40:13 -07:00
Alvaro Herrera 05fb5d6619 Ignore partitioned indexes where appropriate
get_relation_info() was too optimistic about opening indexes in
partitioned tables, which would raise errors when any queries were
planned on such tables.  Fix by ignoring any indexes of the partitioned
kind.

CLUSTER (and ALTER TABLE CLUSTER ON) had a similar problem.  Fix by
disallowing these commands in partitioned tables.

Fallout from 8b08f7d482.
2018-01-25 16:12:15 -03:00
Robert Haas 8b304b8b72 Remove replacement selection sort.
At the time replacement_sort_tuples was introduced, there were still
cases where replacement selection sort noticeably outperformed using
quicksort even for the first run.  However, those cases seem to have
evaporated as a result of further improvements made since that time
(and perhaps also advances in CPU technology).  So remove replacement
selection and the controlling GUC entirely.  This makes tuplesort.c
noticeably simpler and probably paves the way for further
optimizations someone might want to do later.

Peter Geoghegan, with review and testing by Tomas Vondra and me.

Discussion: https://postgr.es/m/CAH2-WzmmNjG_K0R9nqYwMq3zjyJJK+hCbiZYNGhAy-Zyjs64GQ@mail.gmail.com
2017-09-29 10:25:44 -04:00
Tom Lane 18555b1323 Establish conventions about global object names used in regression tests.
To ensure that "make installcheck" can be used safely against an existing
installation, we need to be careful about what global object names
(database, role, and tablespace names) we use; otherwise we might
accidentally clobber important objects.  There's been a weak consensus that
test databases should have names including "regression", and that test role
names should start with "regress_", but we didn't have any particular rule
about tablespace names; and neither of the other rules was followed with
any consistency either.

This commit moves us a long way towards having a hard-and-fast rule that
regression test databases must have names including "regression", and that
test role and tablespace names must start with "regress_".  It's not
completely there because I did not touch some test cases in rolenames.sql
that test creation of special role names like "session_user".  That will
require some rethinking of exactly what we want to test, whereas the intent
of this patch is just to hit all the cases in which the needed renamings
are cosmetic.

There is no enforcement mechanism in this patch either, but if we don't
add one we can expect that the tests will soon be violating the convention
again.  Again, that's not such a cosmetic change and it will require
discussion.  (But I did use a quick-hack enforcement patch to find these
cases.)

Discussion: <16638.1468620817@sss.pgh.pa.us>
2016-07-17 18:42:43 -04:00
Tom Lane 56a9974133 Minor test adjustment.
Dept of second thoughts: given the RESET SESSION AUTHORIZATION that
was just added by commit cec550139, we don't need the reconnection
that used to be here.  Might as well buy back a few microseconds.
2016-07-13 15:36:25 -04:00
Tom Lane cec5501394 Add a regression test case to improve code coverage for tuplesort.
Test the external-sort code path in CLUSTER for two different scenarios:
multiple-pass external sorting, and the best case for replacement
selection, where only one run is produced, so that no merge is required.
This test would have caught the bug fixed in commit 1b0fc8507, at
least when run with valgrind enabled.

In passing, add a short-circuit test in plan_cluster_use_sort() to make
dead certain that it selects sorting when enable_indexscan is off.  As
things stand, that would happen anyway, but it seems like good future
proofing for this test.

Peter Geoghegan

Discussion: <CAM3SWZSgxehDkDMq1FdiW2A0Dxc79wH0hz1x-TnGy=1BXEL+nw@mail.gmail.com>
2016-07-13 15:23:56 -04:00
Robert Haas d7c734841b Reduce messages about implicit indexes and sequences to DEBUG1.
Per recent discussion on pgsql-hackers, these messages are too
chatty for most users.
2012-07-04 20:35:29 -04:00
Peter Eisentraut fc946c39ae Remove useless whitespace at end of lines 2010-11-23 22:34:55 +02:00
Tom Lane 98e314550c Fix timing-sensitive regression test result I just created :-( --- the
DROP USER at the end of the cluster.sql test could fail, if the temp
table created in the previous session hadn't finished getting dropped.
Unluckily, I didn't see this in several repetitions of the parallel
regression tests, but it's popping up on quite a few buildfarm machines.
2010-02-03 03:21:25 +00:00
Tom Lane c98157d693 CLUSTER specified the wrong namespace when renaming toast tables of temporary
relations (they don't live in pg_toast).  This caused an Assert failure in
assert-enabled builds.  So far as I can see, in a non-assert build it would
only have messed up the checks for conflicting names, so a failure would be
quite improbable but perhaps not impossible.
2010-02-02 19:12:29 +00:00
Tom Lane 6f4a5462b4 Add an ORDER BY to nail down the expected row order from a query that's
been shown to be sensitive to concurrent autovacuum.  Per Alvaro.
2007-09-03 01:28:40 +00:00
Tom Lane 7b78474da3 Make CLUSTER MVCC-safe. Heikki Linnakangas 2007-04-08 01:26:33 +00:00
Bruce Momjian e55c8e36ae Support syntax "CLUSTER table USING index", which is more logical.
Holger Schurig
2007-04-08 00:26:34 +00:00
Tom Lane 59d1b3d99e Track dependencies on shared objects (which is to say, roles; we already
have adequate mechanisms for tracking the contents of databases and
tablespaces).  This solves the longstanding problem that you can drop a
user who still owns objects and/or has access permissions.
Alvaro Herrera, with some kibitzing from Tom Lane.
2005-07-07 20:40:02 +00:00
Tom Lane 45616f5bbb Clean up generation of default names for constraints, indexes, and serial
sequences, as per recent discussion.  All these names are now of the
form table_column_type, with digits added if needed to make them unique.
Default constraint names are chosen to be unique across their whole schema,
not just within the parent object, so as to be more SQL-spec-compatible
and make the information schema views more useful.
2004-06-10 17:56:03 +00:00
Bruce Momjian 1cdc58722c OK, here's the final version of ALTER TABLE ... SET WITHOUT CLUSTER.
Has docs + regression test.

Christopher Kings-Lynne
2004-06-02 21:01:10 +00:00
Peter Eisentraut 5b806ecf55 Remove NOTICE about foreign key creating implicit triggers, because it no
longer conveys useful information.
2003-10-02 06:32:46 +00:00
Peter Eisentraut feb4f44d29 Message editing: remove gratuitous variations in message wording, standardize
terms, add some clarifications, fix some untranslatable attempts at dynamic
message building.
2003-09-25 06:58:07 +00:00
Tom Lane 56f87688c4 Error message editing for foreign-key triggers. 2003-07-22 22:14:57 +00:00
Tom Lane da4ed8bfdd Another round of error message editing, covering backend/commands/. 2003-07-20 21:56:35 +00:00
Tom Lane a56ff9a0bd Another round of error message editing, covering backend/parser/. 2003-07-19 20:20:53 +00:00
Bruce Momjian 432b9b0f75 Add ALTER TABLE <tablename> CLUSTER ON <indexname>
Alvaro Herrera
2003-03-20 18:52:48 +00:00
Tom Lane 48dfa0d057 Arrange to print the relevant key values when reporting a foreign-key
violation.  Also, factor out some duplicate code in the RI triggers.
Patch by Dmitry Tkach, reviewed by Stephan Szabo and Tom Lane.
2003-03-15 21:19:40 +00:00
Tom Lane a03c0d93d5 Code review for CLUSTER ALL patch. Fix bogus locking, incorrect transaction
stop/start nesting, other infelicities.
2002-12-30 18:42:17 +00:00
Bruce Momjian 988dbc58a2 Update regression for ALL missing from CLUSTER. 2002-11-19 17:39:00 +00:00
Bruce Momjian 00e59b5ec2 Fix CLUSTER regression failure caused by new patch, from Neil Conway 2002-11-16 03:25:22 +00:00
Bruce Momjian 8bc717cb88 New version attached. The following is implemented:
- CLUSTER ALL clusters all the tables that have some index with
  indisclustered set and the calling user owns.
- CLUSTER tablename clusters the named table, using the index with
  indisclustered set.  If no index has the bit set, throws elog(ERROR).
- The multi-relation version (CLUSTER ALL) uses a multitransaction
  approach, similar to what VACUUM does.

Alvaro Herrera
2002-11-15 03:09:39 +00:00
Bruce Momjian c0ad595386 I attach a little patch to make CLUSTER set and reset the indisclustered
bit on the indexes.

I also attach clusterdb and clusterdb.sgml; both of them are blatant
rips of vacuumdb and vacuumdb.sgml, but get the job done.  Please review
them, as I'm probably making a lot of mistakes with SGML and I can't
compile it here.

vacuumdb itself is not very comfortable to use when the databases have
passwords, because it has to connect once for each table (I can probably
make it connect only once for each database; should I?).  Because of
this I added a mention of PGPASSWORDFILE in the documentation, but I
don't know if that is the correct place for that.

Alvaro Herrera
2002-08-27 03:38:28 +00:00
Tom Lane e44beef712 Code review of CLUSTER patch. Clean up problems with relcache getting
confused, toasted data getting lost, etc.
2002-08-11 21:17:35 +00:00
Tom Lane 74ce5c93c7 Make cluster regress test functional. 2002-08-11 02:06:32 +00:00