1
0
mirror of https://github.com/postgres/postgres.git synced 2025-04-29 13:56:47 +03:00

1115 Commits

Author SHA1 Message Date
Michael Paquier
9f87da1cff Refactor some code for ALTER TABLE SET LOGGED/UNLOGGED in tablecmds.c
Both sub-commands use the same routine to switch the relpersistence of a
relation, duplicated the same checks, and used a style inconsistent with
access methods and tablespaces.

SET LOGEED/UNLOGGED is refactored to avoid any duplication, setting the
reason why a relation rewrite happens within ATPrepChangePersistence().
This shaves some code.

Discussion: https://postgr.es/m/ZiiyGFTBNkqcMQi_@paquier.xyz
2024-08-29 15:31:30 +09:00
Alexander Korotkov
3890d90c15 Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) commands
This commit reverts 1adf16b8fb, 87c21bb941, and subsequent fixes and
improvements including df64c81ca9, c99ef1811a, 9dfcac8e15, 885742b9f8,
842c9b2705, fcf80c5d5f, 96c7381c4c, f4fc7cb54b, 60ae37a8bc, 259c96fa8f,
449cdcd486, 3ca43dbbb6, 2a679ae94e, 3a82c689fd, fbd4321fd5, d53a4286d7,
c086896625, 4e5d6c4091, 04158e7fa3.

The reason for reverting is security issues related to repeatable name lookups
(CVE-2014-0062).  Even though 04158e7fa3 solved part of the problem, there
are still remaining issues, which aren't feasible to even carefully analyze
before the RC deadline.

Reported-by: Noah Misch, Robert Haas
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Backpatch-through: 17
2024-08-24 18:48:48 +03:00
Alexander Korotkov
04158e7fa3 Avoid repeated table name lookups in createPartitionTable()
Currently, createPartitionTable() opens newly created table using its name.
This approach is prone to privilege escalation attack, because we might end
up opening another table than we just created.

This commit address the issue above by opening newly created table by its
OID.  It appears to be tricky to get a relation OID out of ProcessUtility().
We have to extend TableLikeClause with new newRelationOid field, which is
filled within ProcessUtility() to be further accessed by caller.

Security: CVE-2014-0062
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Reviewed-by: Pavel Borisov, Dmitry Koval
2024-08-22 09:50:48 +03:00
Richard Guo
9bb842f95e Small code simplification
Apply the same code simplification to ATExecAddColumn as was done in
7ff9afbbd: apply GETSTRUCT() once instead of doing it repeatedly in
the same function.

Author: Tender Wang
Discussion: https://postgr.es/m/CAHewXNkO9+U437jvKT14s0MCu6Qpf6G-p2mZK5J9mAi4cHDgpQ@mail.gmail.com
2024-08-22 11:41:08 +09:00
Peter Eisentraut
7ff9afbbd1 Small code simplification
Apply GETSTRUCT() once instead of doing it repeatedly in the same
function.  This simplifies the notation and makes the function's
structure more similar to the surrounding ones.

Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org
2024-08-21 09:21:25 +02:00
Alvaro Herrera
a90bdd7a44
Refuse ATTACH of a table referenced by a foreign key
Trying to attach a table as a partition which is already on the
referenced side of a foreign key on the partitioned table that it is
being attached to, leads to strange behavior: we try to clone the
foreign key from the parent to the partition, but this new FK points to
the partition itself, and the mix of pg_constraint rows and triggers
doesn't behave well.

Rather than trying to untangle the mess (which might be possible given
sufficient time), I opted to forbid the ATTACH.  This doesn't seem a
problematic restriction, given that we already fail to create the
foreign key if you do it the other way around, that is, having the
partition first and the FK second.

Backpatch to all supported branches.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org
2024-08-08 19:35:13 -04:00
Heikki Linnakangas
1e35951e71 Turn a few 'validnsps' static variables into locals
There was no need for these to be static buffers, local variables work
just as well. I think they were marked as 'static' to imply that they
are read-only, but 'const' is more appropriate for that, so change
them to const.

To make it possible to mark the variables as 'const', also add 'const'
decorations to the transformRelOptions() signature.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/54c29fb0-edf2-48ea-9814-44e918bbd6e8@iki.fi
2024-08-06 23:03:43 +03:00
Tom Lane
5d1d8b3c82 Clarify error message and documentation related to typed tables.
We restrict typed tables (those declared as "OF composite_type")
to be based on stand-alone composite types, not composite types
that are the implicitly-created rowtypes of other tables.
But if you tried to do that, you got the very confusing error
message "type foo is not a composite type".  Provide a more specific
message for that case.  Also clarify related documentation in the
CREATE TABLE man page.

Erik Wienhold and David G. Johnston, per complaint from Hannu Krosing.

Discussion: https://postgr.es/m/CAMT0RQRysCb_Amy5CTENSc5GfsvXL1a4qX3mv_hx31_v74P==g@mail.gmail.com
2024-07-26 12:39:45 -04:00
Fujii Masao
c086896625 Fix tablespace handling in MERGE/SPLIT partition commands.
As commit ca4103025d stated, new partitions without a specified tablespace
should inherit the parent relation's tablespace. However, previously,
ALTER TABLE MERGE PARTITIONS and ALTER TABLE SPLIT PARTITION commands
always created new partitions in the default tablespace, ignoring
the parent's tablespace. This commit ensures new partitions inherit
the parent's tablespace.

Backpatch to v17 where these commands were introduced.

Author: Fujii Masao
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/abaf390b-3320-40a5-8815-ef476db5cfe7@oss.nttdata.com
2024-07-15 13:11:51 +09:00
Alvaro Herrera
8391779138
Fix ALTER TABLE DETACH for inconsistent indexes
When a partitioned table has an index that doesn't support a constraint,
but a partition has an equivalent index that does, then a DETACH
operation would misbehave: a crash in assertion-enabled systems (because
we fail to find the constraint in the parent that we expect to), or a
broken coninhcount value (-1) in production systems (because we blindly
believe that we've successfully detached the parent).

While we should reject an ATTACH of a partition with such an index, we
have failed to do so in existing releases, so adding an error in stable
releases might break the (unlikely) existing applications that rely on
this behavior.  At this point I don't even want to reject them in
master, because it'd break pg_upgrade if such databases exist, and there
would be no easy way to fix existing databases without expensive index
rebuilds.

(Later on we could add ALTER TABLE ... ADD CONSTRAINT USING INDEX to
partitioned tables, which would allow the user to fix such patterns.  At
that point we could add more restrictions to prevent the problem from
its root.)

Also, add a test case that leaves one table in this condition, so that
we can verify that pg_upgrade continues to work if we later decide to
change the policy on the master branch.

Backpatch to all supported branches.

Co-authored-by: Tender Wang <tndrwang@gmail.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/18500-62948b6fe5522f56@postgresql.org
2024-07-12 12:54:01 +02:00
Michael Paquier
b81a71aa05 Assign error codes where missing for user-facing failures
All the errors triggered in the code paths patched here would cause the
backend to issue an internal_error errcode, which is a state that should
be used only for "can't happen" situations.  However, these code paths
are reachable by the regression tests, and could be seen by users in
valid cases.  Some regression tests expect internal errcodes as they
manipulate the backend state to cause corruption (like checksums), or
use elog() because it is more convenient (like injection points), these
have no need to change.

This reduces the number of internal failures triggered in a check-world
by more than half, while providing correct errcodes for these valid
cases.

Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/Zic_GNgos5sMxKoa@paquier.xyz
2024-07-04 09:48:40 +09:00
Peter Eisentraut
e26d313bad Remove useless code
BuildDescForRelation() goes out of its way to fill in
->constr->has_not_null, but that value is not used for anything later,
so this code can all be removed.  Note that BuildDescForRelation()
doesn't make any effort to fill in the rest of ->constr, so there is
no claim that that structure is completely filled in.

Reviewed-by: Tomasz Rybak <tomasz.rybak@post.pl>
Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org
2024-07-01 08:50:29 +02:00
Noah Misch
0cecc908e9 Lock before setting relhassubclass on RELKIND_PARTITIONED_INDEX.
Commit 5b562644fec696977df4a82790064e8287927891 added a comment that
SetRelationHasSubclass() callers must hold this lock.  When commit
17f206fbc824d2b4b14480199ca9ff7dea417eda extended use of this column to
partitioned indexes, it didn't take the lock.  As the latter commit
message mentioned, we currently never reset a partitioned index to
relhassubclass=f.  That largely avoids harm from the lock omission.  The
cause for fixing this now is to unblock introducing a rule about locks
required to heap_update() a pg_class row.  This might cause more
deadlocks.  It gives minor user-visible benefits:

- If an ALTER INDEX SET TABLESPACE runs concurrently with ALTER TABLE
  ATTACH PARTITION or CREATE PARTITION OF, one transaction blocks
  instead of failing with "tuple concurrently updated".  (Many cases of
  DDL concurrency still fail that way.)

- Match ALTER INDEX ATTACH PARTITION in choosing to lock the index.

While not user-visible today, we'll need this if we ever make something
set the flag to false for a partitioned index, like ANALYZE does today
for tables.  Back-patch to v12 (all supported versions), the plan for
the commit relying on the new rule.  In back branches, add
LockOrStrongerHeldByMe() instead of adding a LockHeldByMe() parameter.

Reviewed (in an earlier version) by Robert Haas.

Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
2024-06-27 19:21:05 -07:00
Peter Geoghegan
6207f08f70 Harmonize function parameter names for Postgres 17.
Make sure that function declarations use names that exactly match the
corresponding names from function definitions in a few places.  These
inconsistencies were all introduced during Postgres 17 development.

pg_bsd_indent still has a couple of similar inconsistencies, which I
(pgeoghegan) have left untouched for now.

This commit was written with help from clang-tidy, by mechanically
applying the same rules as similar clean-up commits (the earliest such
commit was commit 035ce1fe).
2024-06-12 17:01:51 -04:00
Tom Lane
7ed8ce8a46 Reject modifying a temp table of another session with ALTER TABLE.
Normally this case isn't even reachable by non-superusers, since
permissions checks prevent naming such a table.  However, it is
possible to make it happen by altering a parent table whose child
is another session's temp table.

We definitely can't support any such ALTER that requires modifying
the contents of such a table, since we lack access to the other
session's temporary-buffer pool.  But there seems no good reason
to allow it even if it'd only require changing catalog contents.
One reason not to allow it is that we'd rather not expose the
implementation-dependent behavior of whether a specific ALTER
requires touching the table contents.  Another is that there may
be (in future, even if not today) optimizations that assume that
a session's own temp tables won't be modified by other sessions.

Hence, add a RELATION_IS_OTHER_TEMP() check to all the places
where ALTER TABLE currently does CheckTableNotInUse().  (I looked
through all other callers of CheckTableNotInUse(), and they seem
OK already.)

Per bug #18492 from Alexander Lakhin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18492-c7a2634bf4968763@postgresql.org
2024-06-07 14:50:09 -04:00
Alexander Korotkov
fbd4321fd5 Don't copy extended statistics during MERGE/SPLIT partition operations
When MERGE/SPLIT created new partitions, it was cloning the extended
statistics of the parent table.

However, extended stats on partitioned tables don't behave like
indexes on partitioned tables (which exist only to create physical
indexes on child tables).  Rather, extended stats on a parent 1) cause
extended stats to be collected and computed across the whole partition
hierarchy, and 2) do not cause extended stats to be computed for the
individual partitions.

"CREATE TABLE ... PARTITION OF" command doesn't copy extended
statistics.  This commit makes createPartitionTable() behave
consistently.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/ZiJW1g2nbQs9ekwK%40pryzbyj2023
Author: Alexander Korotkov, Justin Pryzby
2024-05-23 02:22:41 +03:00
Alexander Korotkov
3a82c689fd Fix the name collision detection in MERGE/SPLIT partition operations
Both MERGE and SPLIT partition operations support the case when the name of the
new partition matches the name of the existing partition to be merged/split.
But the name collision detection doesn't always work as intended.  The SPLIT
partition operation finds the namespace to search for an existing partition
without taking into account the parent's persistence.  The MERGE partition
operation checks for the name collision with simple equal() on RangeVar's
simply ignoring the search_path.

This commit fixes this behavior as follows.
 1. The SPLIT partition operation now finds the namespace to search for
    an existing partition according to the parent's persistence.
 2. The MERGE partition operation now checks for the name collision similarly
    to the SPLIT partition operation using
    RangeVarGetAndCheckCreationNamespace() and get_relname_relid().

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/86b4f1e3-0b5d-315c-9225-19860d64d685%40gmail.com
Author: Dmitry Koval, Alexander Korotkov
2024-05-23 02:21:00 +03:00
Peter Eisentraut
8aee330af5 Revert temporal primary keys and foreign keys
This feature set did not handle empty ranges correctly, and it's now
too late for PostgreSQL 17 to fix it.

The following commits are reverted:

    6db4598fcb8 Add stratnum GiST support function
    46a0cd4cefb Add temporal PRIMARY KEY and UNIQUE constraints
    86232a49a43 Fix comment on gist_stratnum_btree
    030e10ff1a3 Rename pg_constraint.conwithoutoverlaps to conperiod
    a88c800deb6 Use daterange and YMD in without_overlaps tests instead of tsrange.
    5577a71fb0c Use half-open interval notation in without_overlaps tests
    34768ee3616 Add temporal FOREIGN KEY contraints
    482e108cd38 Add test for REPLICA IDENTITY with a temporal key
    c3db1f30cba doc:  clarify PERIOD and WITHOUT OVERLAPS in CREATE TABLE
    144c2ce0cc7 Fix ON CONFLICT DO NOTHING/UPDATE for temporal indexes

Discussion: https://www.postgresql.org/message-id/d0b64a7a-dfe4-4b84-a906-c7dedfa40a3e@eisentraut.org
2024-05-16 08:17:46 +02:00
Alvaro Herrera
6f8bb7c1e9
Revert structural changes to not-null constraints
There are some problems with the new way to handle these constraints
that were detected at the last minute, and require fixes that appear too
invasive to be doing this late in the cycle.  Revert this (again) for
now, we'll try again with these problems fixed.

The following commits are reverted:

    b0e96f311985  Catalog not-null constraints
    9b581c534186  Disallow changing NO INHERIT status of a not-null constraint
    d0ec2ddbe088  Fix not-null constraint test
    ac22a9545ca9  Move privilege check to the right place
    b0f7dd915bca  Check stack depth in new recursive functions
    3af721794272  Update information_schema definition for not-null constraints
    c3709100be73  Fix propagating attnotnull in multiple inheritance
    d9f686a72ee9  Fix restore of not-null constraints with inheritance
    d72d32f52d26  Don't try to assign smart names to constraints
    0cd711271d42  Better handle indirect constraint drops
    13daa33fa5a6  Disallow NO INHERIT not-null constraints on partitioned tables
    d45597f72fe5  Disallow direct change of NO INHERIT of not-null constraints
    21ac38f498b3  Fix inconsistencies in error messages

Discussion: https://postgr.es/m/202405110940.joxlqcx4dogd@alvherre.pgsql
2024-05-13 11:31:09 +02:00
Tom Lane
9effc4608e Repair ALTER EXTENSION ... SET SCHEMA.
It turns out that we broke this in commit e5bc9454e, because
the code was assuming that no dependent types would appear
among the extension's direct dependencies, and now they do.

This isn't terribly hard to fix: just skip dependent types,
expecting that we will recurse to them when we process the parent
object (which should also be among the direct dependencies).
But a little bit of refactoring is needed so that we can avoid
duplicating logic about what is a dependent type.

Although there is some testing of ALTER EXTENSION SET SCHEMA,
it failed to cover interesting cases, so add more tests.

Discussion: https://postgr.es/m/930191.1715205151@sss.pgh.pa.us
2024-05-09 12:19:52 -04:00
Alvaro Herrera
21ac38f498
Fix inconsistencies in error messages
Reported by Kyotaro Horiguchi

Also some comments mentioning wrong version numbers, spotted by Justin
Pryzby.

Discussion: https://postgr.es/m/20240507.171724.750916195320223609.horikyota.ntt@gmail.com
Discussion: https://postgr.es/m/Zh0aAH7tbZb-9HbC@pryzbyj2023
2024-05-09 13:31:22 +02:00
Peter Eisentraut
509199587d Fix assorted bugs related to identity column in partitioned tables
When changing the data type of a column of a partitioned table, craft
the ALTER SEQUENCE command only once.  Partitions do not have identity
sequences of their own and thus do not need a ALTER SEQUENCE command
for each partition.

Fix getIdentitySequence() to fetch the identity sequence associated
with the top-level partitioned table when a Relation of a partition is
passed to it.  While doing so, translate the attribute number of the
partition into the attribute number of the partitioned table.

Author: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com>
Discussion: https://www.postgresql.org/message-id/3b8a9dc1-bbc7-0ef5-6863-c432afac7d59@gmail.com
2024-05-07 22:50:00 +02:00
Peter Eisentraut
8f0a97dfff Fix segmentation fault in MergeInheritedAttribute()
While converting a pg_attribute tuple into a ColumnDef,
ColumnDef::compression remains NULL if there is no compression method
set fot the attribute.  Calling strcmp() with NULL
ColumnDef::compression, when comparing compression methods of parents,
causes segmentation fault in MergeInheritedAttribute().  Skip
comparing compression methods if either of them is NULL.

Author: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://www.postgresql.org/message-id/b22a6834-aacb-7b18-0424-a3f5fe889667%40gmail.com
2024-05-03 11:10:40 +02:00
Tom Lane
91e7115b17 Throw a more on-point error for publications depending on columns.
Same as 42b041243, except that the trouble case is a publication
WHERE clause that depends on a column.

Again reported by Alexander Lakhin.  Back-patch to v15 where
we added publication WHERE clauses.

Discussion: https://postgr.es/m/548a47bc-87ae-b3df-c6a2-60b9966f808b@gmail.com
2024-05-02 17:36:31 -04:00
Alvaro Herrera
d45597f72f
Disallow direct change of NO INHERIT of not-null constraints
We support changing NO INHERIT constraint to INHERIT for constraints in
child relations when adding a constraint to some ancestor relation, and
also during pg_upgrade's schema restore; but other than those special
cases, command ALTER TABLE ADD CONSTRAINT should not be allowed to
change an existing constraint from NO INHERIT to INHERIT, as that would
require to process child relations so that they also acquire an
appropriate constraint, which we may not be in a position to do.  (It'd
also be surprising behavior.)

It is conceivable that we want to allow ALTER TABLE SET NOT NULL to make
such a change; but in that case some more code is needed to implement it
correctly, so for now I've made that throw the same error message.

Also, during the prep phase of ALTER TABLE ADD CONSTRAINT, acquire locks
on all descendant tables; otherwise we might operate on child tables on
which no locks are held, particularly in the mode where a primary key
causes not-null constraints to be created on children.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/7d923a66-55f0-3395-cd40-81c142b5448b@gmail.com
2024-05-02 17:26:30 +02:00
Alexander Korotkov
259c96fa8f Inherit parent's AM for partition MERGE/SPLIT operations
This commit makes new partitions created by ALTER TABLE ... SPLIT PARTITION
and ALTER TABLE ... MERGE PARTITIONS commands inherit the paret table access
method.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/84ada05b-be5c-473e-6d1c-ebe5dd21b190%40gmail.com
Reviewed-by: Pavel Borisov
2024-04-30 12:00:39 +03:00
Alexander Korotkov
fcf80c5d5f Make new partitions with parent's persistence during MERGE/SPLIT
The createPartitionTable() function is responsible for creating new partitions
for ALTER TABLE ... MERGE PARTITIONS, and ALTER TABLE ... SPLIT PARTITION
commands.  It emulates the behaviour of CREATE TABLE ... (LIKE ...), where
new table persistence should be specified by the user.  In the table
partitioning persistent of the partition and its parent must match.  So, this
commit makes createPartitionTable() copy the persistence of the parent
partition.

Also, this commit makes createPartitionTable() recheck the persistence after
the new table creation.  This is needed because persistence might be affected
by pg_temp in search_path.

This commit also changes the signature of createPartitionTable() making it
take the parent's Relation itself instead of the name of the parent relation,
and return the Relation of new partition.  That doesn't lead to
complications, because both callers have the parent table open and need to
open the new partition.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com
Author: Dmitry Koval
Reviewed-by: Alexander Korotkov, Robert Haas, Justin Pryzby, Pavel Borisov
2024-04-30 12:00:15 +03:00
Alexander Korotkov
885742b9f8 Change the way ATExecMergePartitions() handles the name collision
The name collision happens when the name of the new partition is the same as
the name of one of the merging partitions.  Currently, ATExecMergePartitions()
first gives the new partition a temporary name and then renames it when old
partitions are deleted.  That negatively influences the naming of related
objects like indexes and constrains, which could inherit a temporary name.

This commit changes the implementation in the following way.  A merging
partition gets renamed first, then the new partition is created with the
right name immediately.  This resolves the issue of the naming of related
objects.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/edfbd846-dcc1-42d1-ac26-715691b687d3%40postgrespro.ru
Author: Dmitry Koval, Alexander Korotkov
Reviewed-by: Robert Haas, Justin Pryzby, Pavel Borisov
2024-04-30 11:54:42 +03:00
Tom Lane
42b041243c Throw a more on-point error for functions depending on columns.
ALTER COLUMN TYPE wasn't expecting to find any pg_proc objects
depending on the column whose type is to be altered.  That indeed
wasn't possible when this code was written, but it is possible
since we introduced new-style SQL function bodies.

It's about as difficult to fix this case as it is to fix dependent
views, and we've been punting on those for years, so I don't feel
too awful about punting for functions too.  (I sure wouldn't risk
back-patching such code.)  So just throw a more user-facing error.
Also, adjust some of the existing comments to reflect that these
are all pretty much the same issue.

(This patch also fixes it so we will tolerate finding such a
dependency during ALTER COLUMN SET EXPRESSION; in that, we need
not do anything to the function, so no error is wanted.  That
problem is new in HEAD.)

Per bug #18449 from Alexander Lakhin.  Back-patch to v14 where
we added new-style SQL functions.

Discussion: https://postgr.es/m/18449-f8248467aaa294d5@postgresql.org
2024-04-28 14:34:21 -04:00
Alvaro Herrera
0cd711271d
Better handle indirect constraint drops
It is possible for certain cases to remove not-null constraints without
maintaining the attnotnull in its correct state; for example if you drop
a column that's part of the primary key, and the other columns of the PK don't
have not-null constraints, then we should reset the attnotnull flags for
those other columns; up to this commit, we didn't.  Handle those cases
better by doing the attnotnull reset in RemoveConstraintById() instead
of in dropconstraint_internal().

However, there are some cases where we must not do so.  For example if
those other columns are in replica identity indexes or are generated
identity columns, we must keep attnotnull set, even though it results in
the catalog inconsistency that no not-null constraint supports that.

Because the attnotnull reset now happens in more places than before, for
instance when a column of the primary key changes type, we need an
additional trick to reinstate it as necessary.  Introduce a new
alter-table pass that does this, which needs simply reschedule some
AT_SetAttNotNull subcommands that were already being generated and
ignored.

Because of the exceptions in which attnotnull is not reset noted above,
we also include a pg_dump hack to include a not-null constraint when the
attnotnull flag is set even if no pg_constraint row exists.  This part
is undesirable but necessary, because failing to handle the case can
result in unrestorable dumps.

Reported-by: Tender Wang <tndrwang@gmail.com>
Co-authored-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CAHewXN=hMbNa3d43NOR=OCgdgpTt18S-1fmueCoEGesyeK4bqw@mail.gmail.com
2024-04-19 12:37:33 +02:00
Daniel Gustafsson
950d4a2cb1 Fix typos and duplicate words
This fixes various typos, duplicated words, and tiny bits of whitespace
mainly in code comments but also in docs.

Author: Daniel Gustafsson <daniel@yesql.se>
Author: Heikki Linnakangas <hlinnaka@iki.fi>
Author: Alexander Lakhin <exclusion@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
2024-04-18 21:28:07 +02:00
Alvaro Herrera
d9f686a72e
Fix restore of not-null constraints with inheritance
In tables with primary keys, pg_dump creates tables with primary keys by
initially dumping them with throw-away not-null constraints (marked "no
inherit" so that they don't create problems elsewhere), to later drop
them once the primary key is restored.  Because of a unrelated
consideration, on tables with children we add not-null constraints to
all columns of the primary key when it is created.

If both a table and its child have primary keys, and pg_dump happens to
emit the child table first (and its throw-away not-null) and later its
parent table, the creation of the parent's PK will fail because the
throw-away not-null constraint collides with the permanent not-null
constraint that the PK wants to add, so the dump fails to restore.

We can work around this problem by letting the primary key "take over"
the child's not-null.  This requires no changes to pg_dump, just two
changes to ALTER TABLE: first, the ability to convert a no-inherit
not-null constraint into a regular inheritable one (including recursing
down to children, if there are any); second, the ability to "drop" a
constraint that is defined both directly in the table and inherited from
a parent (which simply means to mark it as no longer having a local
definition).

Secondarily, change ATPrepAddPrimaryKey() to acquire locks all the way
down the inheritance hierarchy, in case we need to recurse when
propagating constraints.

These two changes allow pg_dump to reproduce more cases involving
inheritance from versions 16 and older.

Lastly, make two changes to pg_dump: 1) do not try to drop a not-null
constraint that's marked as inherited; this allows a dump to restore
with no errors if a table with a PK inherits from another which also has
a PK; 2) avoid giving inherited constraints throwaway names, for the
rare cases where such a constraint survives after the restore.

Reported-by: Andrew Bille <andrewbille@gmail.com>
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/CAJnzarwkfRu76_yi3dqVF_WL-MpvT54zMwAxFwJceXdHB76bOA@mail.gmail.com
Discussion: https://postgr.es/m/Zh0aAH7tbZb-9HbC@pryzbyj2023
2024-04-18 15:35:15 +02:00
Alvaro Herrera
cee8db3f68
ATTACH PARTITION: Don't match a PK with a UNIQUE constraint
When matching constraints in AttachPartitionEnsureIndexes() we weren't
testing the constraint type, which could make a UNIQUE key lacking a
not-null constraint incorrectly satisfy a primary key requirement.  Fix
this by testing that the constraint types match.  (Other possible
mismatches are verified by comparing index properties.)

Discussion: https://postgr.es/m/202402051447.wimb4xmtiiyb@alvherre.pgsql
2024-04-15 15:07:47 +02:00
Alexander Korotkov
9dfcac8e15 Grammar fixes for split/merge partitions code
The fixes relate to comments, error messages, and corresponding expected output
of regression tests.

Discussion: https://postgr.es/m/CAMbWs49DDsknxyoycBqiE72VxzL_sYHF6zqL8dSeNehKPJhkKg%40mail.gmail.com
Discussion: https://postgr.es/m/86bfd241-a58c-479a-9a72-2c67a02becf8%40postgrespro.ru
Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com
Author: Richard Guo, Dmitry Koval, Tender Wang
2024-04-15 16:00:02 +03:00
Alvaro Herrera
c3709100be
Fix propagating attnotnull in multiple inheritance
In one of the many strange corner cases of multiple inheritance being
used, commit b0e96f311985 missed a CommandCounterIncrement() call after
updating the attnotnull flag during ALTER TABLE ADD COLUMN, which caused
a catalog tuple to be update attempted twice in the same command, giving
rise to a "tuple already updated by self" error.  Add the missing call
to solve that, and a test case that reproduces the scenario.

As a (perhaps surprising) secondary effect, this CCI addition triggers
another behavior change: when a primary key is added to a parent
partitioned table and the column in an existing partition does not have
a not-null constraint, we no longer error out.  This will probably be a
welcome change by some users, and I think it's unlikely that anybody
will miss the old behavior.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: http://postgr.es/m/045dec3f-9b3d-aa44-0c99-85f6992306c7@gmail.com
2024-04-15 12:20:56 +02:00
Alexander Korotkov
8dd0bb84da Revert: Allow table AM tuple_insert() method to return the different slot
This commit reverts c35a3fb5e0 per review by Andres Freund.

Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
2024-04-11 16:02:45 +03:00
Alexander Korotkov
da841aa4dc Revert: Let table AM insertion methods control index insertion
This commit reverts b1484a3f19 per review by Andres Freund.

Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
2024-04-11 16:01:30 +03:00
Alexander Korotkov
bc1e2092eb Revert: Custom reloptions for table AM
This commit reverts 9bd99f4c26 and 422041542f per review by Andres Freund.

Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
2024-04-11 15:46:35 +03:00
Alexander Korotkov
c99ef1811a Checks for ALTER TABLE ... SPLIT/MERGE PARTITIONS ... commands
Check that the target partition actually belongs to the parent table.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/cd842601-cf1a-9806-f7b7-d2509b93ba61%40gmail.com
Author: Dmitry Koval
2024-04-10 01:47:21 +03:00
Alexander Korotkov
df64c81ca9 Fix some grammer errors from error messages and codes comments
Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com
Author: Tender Wang
2024-04-08 14:39:41 +03:00
Alexander Korotkov
9bd99f4c26 Custom reloptions for table AM
Let table AM define custom reloptions for its tables. This allows specifying
AM-specific parameters by the WITH clause when creating a table.

The reloptions, which could be used outside of table AM, are now extracted
into the CommonRdOptions data structure.  These options could be by decision
of table AM directly specified by a user or calculated in some way.

The new test module test_tam_options evaluates the ability to set up custom
reloptions and calculate fields of CommonRdOptions on their base.

The code may use some parts from prior work by Hao Wu.

Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Discussion: https://postgr.es/m/AMUA1wBBBxfc3tKRLLdU64rb.1.1683276279979.Hmail.wuhao%40hashdata.cn
Reviewed-by: Reviewed-by: Pavel Borisov, Matthias van de Meent, Jess Davis
2024-04-08 11:23:28 +03:00
Alexander Korotkov
87c21bb941 Implement ALTER TABLE ... SPLIT PARTITION ... command
This new DDL command splits a single partition into several parititions.
Just like ALTER TABLE ... MERGE PARTITIONS ... command, new patitions are
created using createPartitionTable() function with parent partition as the
template.

This commit comprises quite naive implementation which works in single process
and holds the ACCESS EXCLUSIVE LOCK on the parent table during all the
operations including the tuple routing.  This is why this new DDL command
can't be recommended for large partitioned tables under a high load.  However,
this implementation come in handy in certain cases even as is.
Also, it could be used as a foundation for future implementations with lesser
locking and possibly parallel.

Discussion: https://postgr.es/m/c73a1746-0cd0-6bdd-6b23-3ae0b7c0c582%40postgrespro.ru
Author: Dmitry Koval
Reviewed-by: Matthias van de Meent, Laurenz Albe, Zhihong Yu, Justin Pryzby
Reviewed-by: Alvaro Herrera, Robert Haas, Stephane Tachoires
2024-04-07 01:18:44 +03:00
Alexander Korotkov
1adf16b8fb Implement ALTER TABLE ... MERGE PARTITIONS ... command
This new DDL command merges several partitions into the one partition of the
target table.  The target partition is created using new
createPartitionTable() function with parent partition as the template.

This commit comprises quite naive implementation which works in single process
and holds the ACCESS EXCLUSIVE LOCK on the parent table during all the
operations including the tuple routing.  This is why this new DDL command
can't be recommended for large partitioned tables under a high load.  However,
this implementation come in handy in certain cases even as is.
Also, it could be used as a foundation for future implementations with lesser
locking and possibly parallel.

Discussion: https://postgr.es/m/c73a1746-0cd0-6bdd-6b23-3ae0b7c0c582%40postgrespro.ru
Author: Dmitry Koval
Reviewed-by: Matthias van de Meent, Laurenz Albe, Zhihong Yu, Justin Pryzby
Reviewed-by: Alvaro Herrera, Robert Haas, Stephane Tachoires
2024-04-07 01:18:43 +03:00
Alexander Korotkov
867cc7b6dd Revert "Custom reloptions for table AM"
This reverts commit c95c25f9af4bc77f2f66a587735c50da08c12b37 due to multiple
design issues spotted after commit.

Reported-by: Jeff Davis
Discussion: https://postgr.es/m/11550b536211d5748bb2865ed6cb3502ff073bf7.camel%40j-davis.com
2024-04-02 11:29:00 +03:00
Alexander Korotkov
b1484a3f19 Let table AM insertion methods control index insertion
Previously, the executor did index insert unconditionally after calling
table AM interface methods tuple_insert() and multi_insert().  This commit
introduces the new parameter insert_indexes for these two methods.  Setting
'*insert_indexes' to true saves the current logic.  Setting it to false
indicates that table AM cares about index inserts itself and doesn't want the
caller to do that.

Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Reviewed-by: Pavel Borisov, Matthias van de Meent, Mark Dilger
2024-03-30 22:53:56 +02:00
Alexander Korotkov
c95c25f9af Custom reloptions for table AM
Let table AM define custom reloptions for its tables.  This allows to
specify AM-specific parameters by WITH clause when creating a table.

The code may use some parts from prior work by Hao Wu.

Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Discussion: https://postgr.es/m/AMUA1wBBBxfc3tKRLLdU64rb.1.1683276279979.Hmail.wuhao%40hashdata.cn
Reviewed-by: Reviewed-by: Pavel Borisov, Matthias van de Meent
2024-03-30 22:36:25 +02:00
Alvaro Herrera
e2395cdbe8
ALTER TABLE: rework determination of access method ID
Avoid setting an access method OID for relation kinds that don't take
one.  Code review for new feature added in 374c7a229042.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/e5516ac1-5264-c3c0-d822-9e6f614ea93b@gmail.com
2024-03-28 16:51:20 +01:00
Tom Lane
fad3b5b5ac Fix failure of ALTER FOREIGN TABLE SET SCHEMA to move sequences.
Ordinary ALTER TABLE SET SCHEMA will also move any owned sequences
into the new schema.  We failed to do likewise for foreign tables,
because AlterTableNamespaceInternal believed that only certain
relkinds could have indexes, owned sequences, or constraints.
We could simply add foreign tables to that relkind list, but it
seems likely that the same oversight could be made again in
future.  Instead let's remove the relkind filter altogether.
These functions shouldn't cost much when there are no objects
that they need to process, and surely this isn't an especially
performance-critical case anyway.

Per bug #18407 from Vidushi Gupta.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18407-4fd07373d252c6a0@postgresql.org
2024-03-26 15:28:31 -04:00
Peter Eisentraut
89e5ef7e21 Remove ObjectClass type
ObjectClass is an enum whose values correspond to catalog OIDs.  But
the extra layer of redirection, which is used only in small parts of
the code, and the similarity to ObjectType, are confusing and
cumbersome.

One advantage has been that some switches processing the OCLASS enum
don't have "default:" cases.  This is so that the compiler tells us
when we fail to add support for some new object class.  But you can
also handle that with some assertions and proper test coverage.  It's
not even clear how strong this benefit is.  For example, in
AlterObjectNamespace_oid(), you could still put a new OCLASS into the
"ignore object types that don't have schema-qualified names" case, and
it might or might not be wrong.  Also, there are already various
OCLASS switches that do have a default case, so it's not even clear
what the preferred coding style should be.

Reviewed-by: jian he <jian.universality@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/CAGECzQT3caUbcCcszNewCCmMbCuyP7XNAm60J3ybd6PN5kH2Dw%40mail.gmail.com
2024-03-26 10:08:56 +01:00
Alvaro Herrera
374c7a2290
Allow specifying an access method for partitioned tables
It's now possible to specify a table access method via
CREATE TABLE ... USING for a partitioned table, as well change it with
ALTER TABLE ... SET ACCESS METHOD.  Specifying an AM for a partitioned
table lets the value be used for all future partitions created under it,
closely mirroring the behavior of the TABLESPACE option for partitioned
tables.  Existing partitions are not modified.

For a partitioned table with no AM specified, any new partitions are
created with the default_table_access_method.

Also add ALTER TABLE ... SET ACCESS METHOD DEFAULT, which reverts to the
original state of using the default for new partitions.

The relcache of partitioned tables is not changed: rd_tableam is not
set, even if a partitioned table has a relam set.

Author: Justin Pryzby <pryzby@telsasoft.com>
Author: Soumyadeep Chakraborty <soumyadeep2007@gmail.com>
Author: Michaël Paquier <michael@paquier.xyz>
Reviewed-by: The authors themselves
Discussion: https://postgr.es/m/CAE-ML+9zM4wJCGCBGv01k96qQ3gFv4WFcFy=zqPHKeaEFwwv6A@mail.gmail.com
Discussion: https://postgr.es/m/20210308010707.GA29832%40telsasoft.com
2024-03-25 16:30:36 +01:00