4673 Commits

Author SHA1 Message Date
Heikki Linnakangas
9f35e42e7d Remove unnecessary smgropen() calls
Now that RelationCreateStorage() returns the SmgrRelation (since
commit 5c1560606dc), use that.

Author: Japin Li
Discussion: https://www.postgresql.org/message-id/ME3P282MB316600FA62F6605477F26F6AB6742@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
2024-02-12 10:59:45 +02:00
Alexander Korotkov
8be93177c4 Use heap_inplace_update() to unset pg_database.dathasloginevt
Doing this instead of regular updates serves two purposes. First, that avoids
possible waiting on the row-level lock.  Second, that avoids dealing with
TOAST.

It's known that changes made by heap_inplace_update() may be lost due to
concurrent normal updates.  However, we are OK with that.  The subsequent
connections will still have a chance to set "dathasloginevt" to false.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/e2a0248e-5f32-af0c-9832-a90d303c2c61%40gmail.com
2024-02-12 00:52:25 +02:00
Michael Paquier
49e7c6f78e Fix indentation of copyto.c
Issue introduced by b619852086ed.

Per buildfarm member koel.
2024-02-09 11:05:01 +09:00
Michael Paquier
b619852086 Improve COPY TO performance when server and client encodings match
This commit fixes an oversight introduced in c61a2f58418e, where COPY TO
would attempt to do encoding conversions even if the encodings of the
client and the server matched for multi-byte encodings.  All conversions
go through pg_any_to_server() that makes the conversion a no-op when the
encodings of the client and the server match, even for multi-byte
encodings.

The logic was fine, but setting CopyToStateData->need_transcoding would
cause strlen() to be called for nothing for each attribute of all the
rows copied, and that was showing high in some profiles (more attributes
make that easier to reach).  This change improves the runtime of some
worst-case COPY TO queries by 15%~ (number present at least here).

This is a performance improvement, so no backpatch is done out of
caution as this is not a regression.

Reported-by: Andres Freund
Analyzed-by: Andres Freund
Author: Michael Paquier
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/20240206020504.edijzczkgd25ek6z@awork3.anarazel.de
2024-02-09 09:30:53 +09:00
Alexander Korotkov
d859fdb36f Fix gcc >= 10 warning
Reported-by: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVQoFXxFm2kCmhHcdM7DjA84_bOjoM8HVAKHbE%2BKrZ1uA%40mail.gmail.com
2024-02-08 21:59:28 +02:00
Amit Kapila
22f7e61a63 Clean-ups for 776621a5e4 and 7329240437.
Following are a few clean-ups related to failover option support in slots:
1. Improve the documentation in create_subscription.sgml.
2. Remove the spurious blank line in subscriptioncmds.c.
3. Remove the NOTICE for alter_replication_slot in subscriptioncmds.c as
we would sometimes print it even when nothing has changed. One can find
the change by enabling log_replication_commands on the publisher.
4. Optimize ReplicationSlotAlter() function to prevent disk flushing when
the slot's data remains unchanged.

Author: Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/OS0PR01MB57164904651FB588A518E98894472@OS0PR01MB5716.jpnprd01.prod.outlook.com
2024-02-07 10:04:04 +05:30
Michael Paquier
b9d6038d70 Simplify signature of CopyAttributeOutCSV() in copyto.c
This has come up in 2889fd23be56, reverted later on, and is still useful
on its own to reduce a bit the differences between the code paths
dedicated to CSV and text.

Discussion: https://postgr.es/m/ZcCKwAeFrlOqPBuN@paquier.xyz
2024-02-07 12:28:55 +09:00
Michael Paquier
1aa8324b81 Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"
This reverts commit 2889fd23be56, following a discussion with Andres
Freund as this callback, being called once per attribute when sending a
relation's row, can involve a lot of indirect function calls (more
attributes to deal with means more impact).  The effects of a dispatch
at this level would become more visible when improving the per-row code
execution of COPY TO, impacting future potential performance
improvements.

Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de
2024-02-07 08:04:26 +09:00
Heikki Linnakangas
b96115acb8 Fix assertion if index is dropped during REFRESH CONCURRENTLY
When assertions are disabled, the built SQL statement is invalid and
you get a "syntax error". So this isn't a serious problem, but let's
avoid the assertion failure.

Backpatch to all supported versions.

Reviewed-by: Noah Misch
2024-02-05 11:01:30 +02:00
Heikki Linnakangas
5a9167c397 Run REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context
The internal commands in REFRESH MATERIALIZED VIEW CONCURRENTLY are
correctly executed in SECURITY_RESTRICTED_OPERATION mode, except for
creating the temporary "diff" table, because you cannot create
temporary tables in SRO mode. But creating the temporary "diff" table
is a pretty complex CTAS command that selects from another temporary
table created earlier in the command. If you can cajole that CTAS
command to execute code defined by the table owner, the table owner
can run code with the privileges of the user running the REFRESH
command.

The proof-of-concept reported to the security team relied on CREATE
RULE to convert the internally-built temp table to a view. That's not
possible since commit b23cd185fd, and I was not able to find a
different way to turn the SELECT on the temp table into code
execution, so as far as I know this is only exploitable in v15 and
below. That's a fiddly assumption though, so apply this patch to
master and all stable versions.

Thanks to Pedro Gallegos for the report.

Security: CVE-2023-5869
Reviewed-by: Noah Misch
2024-02-05 11:01:23 +02:00
Amit Kapila
dafbfed9ef Enhance libpqrcv APIs to support slot synchronization.
This patch provides support for regular (non-replication) connections in
libpqrcv_connect(). This can be used to execute SQL statements on the
primary server without starting a walsender.

A new API libpqrcv_get_dbname_from_conninfo() is also added to extract the
database name from the given connection-info.

Note that this patch doesn't change any existing functionality but later
patches implementing the slot synchronization will use this functionality
to connect to the primary server to fetch required slot information.

Author: Shveta Malik, Hou Zhijie, Ajin Cherian
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
2024-02-05 10:54:06 +05:30
Michael Paquier
2889fd23be Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO
These routines are used by the text and CSV formats to send an output
representation of a string, applying quotes if required.  This is
similar to 95fb5b49024a, reducing the number of "if" branches that need
to be checked on a per-row basis when sending representation of fields
in text or CSV mode.

While on it, this simplifies the signature of CopyAttributeOutCSV() as
it is possible to know that an attribute is alone on a line thanks to
CopyToState.  Headers should not use quotes, even if forced at query
level.

Extracted from a larger patch by the same author.

Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com
2024-02-05 11:12:37 +09:00
Michael Paquier
95fb5b4902 Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM
CopyReadAttributes{CSV,Text}() are used to parse lines for text and CSV
format.  This reduces the number of "if" branches that need to be
checked when parsing fields in CSV and text mode when dealing with a
COPY FROM, something that can become more noticeable with more
attributes and more lines to process.

Extracted from a larger patch by the same author.

Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com
2024-02-05 09:46:02 +09:00
Noah Misch
0b6517a3b7 Sync PG_VERSION file in CREATE DATABASE.
An OS crash could leave PG_VERSION empty or missing.  The same symptom
appeared in a backup by block device snapshot, taken after the next
checkpoint and before the OS flushes the PG_VERSION blocks.  Device
snapshots are not a documented backup method, however.  Back-patch to
v15, where commit 9c08aea6a3090a396be334cc58c511edab05776a introduced
STRATEGY=WAL_LOG and made it the default.

Discussion: https://postgr.es/m/20240130195003.0a.nmisch@google.com
2024-02-01 13:44:19 -08:00
Noah Misch
df220714e5 Handle interleavings between CREATE DATABASE steps and base backup.
Restoring a base backup taken in the middle of CreateDirAndVersionFile()
or write_relmap_file() would lose the function's effects.  The symptom
was absence of the database directory, PG_VERSION file, or
pg_filenode.map.  If missing the directory, recovery would fail.  Either
missing file would not fail recovery but would render the new database
unusable.  Fix CreateDirAndVersionFile() with the transam/README "action
first and then write a WAL entry" strategy.  That has a side benefit of
moving filesystem mutations out of a critical section, reducing the ways
to PANIC.  Fix the write_relmap_file() call with a lock acquisition, so
it interacts with checkpoints like non-CREATE DATABASE calls do.
Back-patch to v15, where commit 9c08aea6a3090a396be334cc58c511edab05776a
introduced STRATEGY=WAL_LOG and made it the default.

Discussion: https://postgr.es/m/20240130195003.0a.nmisch@google.com
2024-02-01 13:44:19 -08:00
Heikki Linnakangas
21d9c3ee4e Give SMgrRelation pointers a well-defined lifetime.
After calling smgropen(), it was not clear how long you could continue
to use the result, because various code paths including cache
invalidation could call smgrclose(), which freed the memory.

Guarantee that the object won't be destroyed until the end of the
current transaction, or in recovery, the commit/abort record that
destroys the underlying storage.

smgrclose() is now just an alias for smgrrelease(). It closes files
and forgets all state except the rlocator, but keeps the SMgrRelation
object valid.

A new smgrdestroy() function is used by rare places that know there
should be no other references to the SMgrRelation.

The short version:

 * smgrclose() is now just an alias for smgrrelease(). It releases
   resources, but doesn't destroy until EOX
 * smgrdestroy() now frees memory, and should rarely be used.

Existing code should be unaffected, but it is now possible for code that
has an SMgrRelation object to use it repeatedly during a transaction as
long as the storage hasn't been physically dropped.  Such code would
normally hold a lock on the relation.

This also replaces the "ownership" mechanism of SMgrRelations with a
pin counter.  An SMgrRelation can now be "pinned", which prevents it
from being destroyed at end of transaction.  There can be multiple pins
on the same SMgrRelation.  In practice, the pin mechanism is only used
by the relcache, so there cannot be more than one pin on the same
SMgrRelation.  Except with swap_relation_files XXX

Author: Thomas Munro, Heikki Linnakangas
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://www.postgresql.org/message-id/CA%2BhUKGJ8NTvqLHz6dqbQnt2c8XCki4r2QvXjBQcXpVwxTY_pvA@mail.gmail.com
2024-01-31 12:31:02 +02:00
Michael Paquier
3e91dba8b0 Fix various issues with ALTER TEXT SEARCH CONFIGURATION
This commit addresses a set of issues when changing token type mappings
in a text search configuration when using duplicated token names:
- ADD MAPPING would fail on insertion because of a constraint failure
after inserting the same mapping.
- ALTER MAPPING with an "overridden" configuration failed with "tuple
already updated by self" when the token mappings are removed.
- DROP MAPPING failed with "tuple already updated by self", like
previously, but in a different code path.

The code is refactored so the token names (with their numbers) are
handled as a List with unique members rather than an array with numbers,
ensuring that no duplicates mess up with the catalog inserts, updates
and deletes.  The list is generated by getTokenTypes(), with the same
error handling as previously while duplicated tokens are discarded from
the list used to work on the catalogs.

Regression tests are expanded to cover much more ground for the cases
fixed by this commit, as there was no coverage for the code touched in
this commit.  A bit more is done regarding the fact that a token name
not supported by a configuration's parser should result in an error even
if IF EXISTS is used in a DROP MAPPING clause.  This is implied in the
code but there was no coverage for that, and it was very easy to miss.

These issues exist since at least their introduction in core with
140d4ebcb46e, so backpatch all the way down.

Reported-by: Alexander Lakhin
Author: Tender Wang, Michael Paquier
Discussion: https://postgr.es/m/18310-1eb233c5908189c8@postgresql.org
Backpatch-through: 12
2024-01-31 13:15:21 +09:00
Amit Kapila
776621a5e4 Add a failover option to subscriptions.
This commit introduces a new subscription option named 'failover', which
provides users with the ability to set the failover property of the
replication slot on the publisher when creating or altering a
subscription.

This uses the replication commands introduced by commit 7329240437 to
enable the failover option for a logical replication slot.

If the failover option is set to true, the associated replication slots
(i.e. the main slot and the table sync slots) in the upstream database are
enabled to be synchronized to the standbys. Note that the capability to
sync the replication slots will be added in subsequent commits.

Thanks to Masahiko Sawada for the design inputs.

Author: Shveta Malik, Hou Zhijie, Ajin Cherian
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
2024-01-30 16:49:28 +05:30
David Rowley
c85977d8fe Doc: mention foreign keys can reference unique indexes
We seem to have only documented a foreign key can reference the columns of
a primary key or unique constraint.  Here we adjust the documentation
to mention columns in a non-partial unique index can be mentioned too.

The header comment for transformFkeyCheckAttrs() also didn't mention
unique indexes, so fix that too.  In passing make that header comment
reflect reality in the various other aspects where it deviated from it.

Bug: 18295
Reported-by: Gilles PARC
Author: Laurenz Albe, David Rowley
Discussion: https://www.postgresql.org/message-id/18295-0ed0fac5c9f7b17b%40postgresql.org
Backpatch-through: 12
2024-01-30 10:15:17 +13:00
Alvaro Herrera
5de890e361
Add EXPLAIN (MEMORY) to report planner memory consumption
This adds a new "Memory:" line under the "Planning:" group (which
currently only has "Buffers:") when the MEMORY option is specified.

In order to make the reporting reasonably accurate, we create a separate
memory context for planner activities, to be used only when this option
is given.  The total amount of memory allocated by that context is
reported as "allocated"; we subtract memory in the context's freelists
from that and report that result as "used".  We use
MemoryContextStatsInternal() to obtain the quantities.

The code structure to show buffer usage during planning was not in
amazing shape, so I (Álvaro) modified the patch a bit to clean that up
in passing.

Author: Ashutosh Bapat
Reviewed-by: David Rowley, Andrey Lepikhov, Jian He, Andy Fan
Discussion: https://www.postgresql.org/message-id/CAExHW5sZA=5LJ_ZPpRO-w09ck8z9p7eaYAqq3Ks9GDfhrxeWBw@mail.gmail.com
2024-01-29 17:53:03 +01:00
Amit Kapila
7329240437 Allow setting failover property in the replication command.
This commit implements a new replication command called
ALTER_REPLICATION_SLOT and a corresponding walreceiver API function named
walrcv_alter_slot. Additionally, the CREATE_REPLICATION_SLOT command has
been extended to support the failover option.

These new additions allow the modification of the failover property of a
replication slot on the publisher. A subsequent commit will make use of
these commands in subscription commands and will add the tests as well to
cover the functionality added/changed by this commit.

Author: Hou Zhijie, Shveta Malik
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda, Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
2024-01-29 09:37:23 +05:30
Michael Paquier
50b797dc99 Fix DROP ROLE when specifying duplicated roles
This commit fixes failures with "tuple already updated by self" when
listing twice the same role and in a DROP ROLE query.

This is an oversight in 6566133c5f52, that has introduced a two-phase
logic in DropRole() where dependencies of all the roles to drop are
removed in a first phase, with the roles themselves removed from
pg_authid in a second phase.

The code is simplified to not rely on a List of ObjectAddress built in
the first phase used to remove the pg_authid entries in the second
phase, switching to a list of OIDs.  Duplicated OIDs can be simply
avoided in the first phase thanks to that.  Using ObjectAddress was not
necessary for the roles as they are not used for anything specific to
dependency.c, building all the ObjectAddress in the List with
AuthIdRelationId as class ID.

In 15 and older versions, where a single phase is used, DROP ROLE with
duplicated role names would fail on "role \"blah\" does not exist" for
the second entry after the CCI() done by the first deletion.  This is
not really incorrect, but it does not seem worth changing based on a
lack of complaints.

Reported-by: Alexander Lakhin
Reviewed-by: Tender Wang
Discussion: https://postgr.es/m/18310-1eb233c5908189c8@postgresql.org
Backpatch-through: 16
2024-01-29 08:05:59 +09:00
Peter Eisentraut
f7cf9494ba Split some code out from MergeAttributes()
- Separate function to merge a child attribute into matching inherited
  attribute: The logic to merge a child attribute into matching
  inherited attribute in MergeAttribute() is only applicable to
  regular inheritance child.  The code is isolated and coherent enough
  that it can be separated into a function of its own.

- Separate function to merge next parent attribute: Partitions inherit
  from only a single parent.  The logic to merge an attribute from the
  next parent into the corresponding attribute inherited from previous
  parents in MergeAttribute() is only applicable to regular
  inheritance children.  This code is isolated enough that it can be
  separate into a function by itself.

These separations makes MergeAttribute() more readable by making it
easier to follow high level logic without getting entangled into
details.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
2024-01-26 13:52:05 +01:00
Peter Eisentraut
64444ce071 MergeAttributes code deduplication
The code handling NOT NULL constraints is duplicated in blocks merging
the attribute definition incrementally.  Deduplicate that code.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
2024-01-26 11:05:10 +01:00
Peter Eisentraut
4d969b2f85 MergeAttributes: convert pg_attribute back to ColumnDef before comparing
MergeAttributes() has a loop to merge multiple inheritance parents
into a column column definition.  The merged-so-far definition is
stored in a ColumnDef node.  If we have to merge columns from multiple
inheritance parents (if the name matches), then we have to check
whether various column properties (type, collation, etc.) match.  The
current code extracts the pg_attribute value of the
currently-considered inheritance parent and compares it against the
merged-so-far ColumnDef value.  If the currently considered column
doesn't match any previously inherited column, we make a new ColumnDef
node from the pg_attribute information and add it to the column list.

This patch rearranges this so that we create the ColumnDef node first
in either case.  Then the code that checks the column properties for
compatibility compares ColumnDef against ColumnDef (instead of
ColumnDef against pg_attribute).  This makes the code more symmetric
and easier to follow.  Also, later in MergeAttributes(), there is a
similar but separate loop that merges the new local column definition
with the combination of the inheritance parents established in the
first loop.  That comparison is already ColumnDef-vs-ColumnDef.  With
this change, both of these can use similar-looking logic.  (A future
project might be to extract these two sets of code into a common
routine that encodes all the knowledge of whether two column
definitions are compatible.  But this isn't currently straightforward
because we want to give different error messages in the two cases.)
Furthermore, by avoiding the use of Form_pg_attribute here, we make it
easier to make changes in the pg_attribute layout without having to
worry about the local needs of tablecmds.c.

Because MergeAttributes() is hugely long, it's sometimes hard to know
where in the function you are currently looking.  To help with that, I
also renamed some variables to make it clearer where you are currently
looking.  The first look is "prev" vs. "new", the second loop is "inh"
vs. "new".

Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
2024-01-25 11:30:01 +01:00
Masahiko Sawada
729439607a Add progress reporting of skipped tuples during COPY FROM.
9e2d870119 enabled the COPY command to skip malformed data, however
there was no visibility into how many tuples were actually skipped
during the COPY FROM.

This commit adds a new "tuples_skipped" column to
pg_stat_progress_copy view to report the number of tuples that were
skipped because they contain malformed data.

Bump catalog version.

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/d12fd8c99adcae2744212cb23feff6ed%40oss.nttdata.com
2024-01-25 10:57:41 +09:00
Peter Eisentraut
46a0cd4cef Add temporal PRIMARY KEY and UNIQUE constraints
Add WITHOUT OVERLAPS clause to PRIMARY KEY and UNIQUE constraints.
These are backed by GiST indexes instead of B-tree indexes, since they
are essentially exclusion constraints with = for the scalar parts of
the key and && for the temporal part.

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
2024-01-24 16:34:37 +01:00
Michael Paquier
bb812ab091 Fix ALTER TABLE .. ADD COLUMN with complex inheritance trees
This command, when used to add a column on a parent table with a complex
inheritance tree, tried to update multiple times the same tuple in
pg_attribute for a child table when incrementing attinhcount, causing
failures with "tuple already updated by self" because of a missing
CommandCounterIncrement() between two updates.

This exists for a rather long time, so backpatch all the way down.

Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/18297-b04cd83a55b51e35@postgresql.org
Backpatch-through: 12
2024-01-24 14:20:01 +09:00
Michael Paquier
b199eb89c6 Fix some typos
Author: Yongtao Huang
Discussion: https://postgr.es/m/CAOe1Go1F99o5JsphtXdDC5bxm7AzetU8q3AxLh4AAVGKu1AzEQ@mail.gmail.com
2024-01-22 13:55:25 +09:00
Alexander Korotkov
b725b7eec4 Rename COPY option from SAVE_ERROR_TO to ON_ERROR
The option names now are "stop" (default) and "ignore".  The future options
could be "file 'filename.log'" and "table 'tablename'".

Discussion: https://postgr.es/m/20240117.164859.2242646601795501168.horikyota.ntt%40gmail.com
Author: Jian He
Reviewed-by: Atsushi Torikoshi
2024-01-19 15:15:51 +02:00
Nathan Bossart
d891dcc065 Improve some documentation about the bootstrap superuser.
This commit adds some notes about the inability to remove superuser
privileges from the bootstrap superuser.  This has been blocked
since commit e530be2c5c, but it wasn't intended be a supported
feature before that, either.

In passing, change "bootstrap user" to "bootstrap superuser" in a
couple places.

Author: Yurii Rashkovskii
Reviewed-by: Vignesh C, David G. Johnston
Discussion: https://postgr.es/m/CA%2BRLCQzSx_eTC2Fch0EzeNHD3zFUcPvBYOoB%2BpPScFLch1DEQw%40mail.gmail.com
2024-01-18 21:39:51 -06:00
Peter Eisentraut
ed1e0a6512 Error message capitalisation
per style guidelines

Author: Peter Smith <peter.b.smith@fujitsu.com>
Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPtzstExQ4%3DvFH%2BWzZ4g4xEx2JA%3DqxussxOdxVEwJce6bw%40mail.gmail.com
2024-01-18 09:35:12 +01:00
Alexander Korotkov
58fbbc9d68 Fix spelling in notice
Reported-by: Atsushi Torikoshi
Discussion: https://postgr.es/m/762d7dd4d5aa9e5ecffec2ae6a255a28%40oss.nttdata.com
2024-01-17 22:59:09 +02:00
Alexander Korotkov
8ad1f7db87 Fix format specifier for NOTICE in copyfrom.c
It's incorrect to use %lz for 64-bit numbers on 32-bit machine.
2024-01-17 00:59:44 +02:00
Alexander Korotkov
9e2d870119 Add new COPY option SAVE_ERROR_TO
Currently, when source data contains unexpected data regarding data type or
range, the entire COPY fails. However, in some cases, such data can be ignored
and just copying normal data is preferable.

This commit adds a new option SAVE_ERROR_TO, which specifies where to save the
error information. When this option is specified, COPY skips soft errors and
continues copying.

Currently, SAVE_ERROR_TO only supports "none". This indicates error information
is not saved and COPY just skips the unexpected data and continues running.

Later works are expected to add more choices, such as 'log' and 'table'.

Author: Damir Belyalov, Atsushi Torikoshi, Alex Shulgin, Jian He
Discussion: https://postgr.es/m/87k31ftoe0.fsf_-_%40commandprompt.com
Reviewed-by: Pavel Stehule, Andres Freund, Tom Lane, Daniel Gustafsson,
Reviewed-by: Alena Rybakina, Andy Fan, Andrei Lepikhov, Masahiko Sawada
Reviewed-by: Vignesh C, Atsushi Torikoshi
2024-01-16 23:08:53 +02:00
Peter Eisentraut
6995863157 Support identity columns in partitioned tables
Previously, identity columns were disallowed on partitioned tables.
(The reason was mainly that no one had gotten around to working
through all the details to make it work.)  This makes it work now.

Some details on the behavior:

* A newly created partition inherits identity property

  The partitions of a partitioned table are integral part of the
  partitioned table.  A partition inherits identity columns from the
  partitioned table.  An identity column of a partition shares the
  identity space with the corresponding column of the partitioned
  table.  In other words, the same identity column across all
  partitions of a partitioned table share the same identity space.
  This is effected by sharing the same underlying sequence.

  When INSERTing directly into a partition, the sequence associated
  with the topmost partitioned table is used to calculate the value of
  the corresponding identity column.

  In regular inheritance, identity columns and their properties in a
  child table are independent of those in its parent tables.  A child
  table does not inherit identity columns or their properties
  automatically from the parent.  (This is unchanged.)

* Attached partition inherits identity column

  A table being attached as a partition inherits the identity property
  from the partitioned table.  This should be fine since we expect
  that the partition table's column has the same type as the
  partitioned table's corresponding column.  If the table being
  attached is a partitioned table, the identity properties are
  propagated down its partition hierarchy.

  An identity column in the partitioned table is also marked as NOT
  NULL.  The corresponding column in the partition needs to be marked
  as NOT NULL for the attach to succeed.

* Drop identity property when detaching partition

  A partition's identity column shares the identity space
  (i.e. underlying sequence) as the corresponding column of the
  partitioned table.  If a partition is detached it can longer share
  the identity space as before.  Hence the identity columns of the
  partition being detached loose their identity property.

  When identity of a column of a regular table is dropped it retains
  the NOT NULL constraint that came with the identity property.
  Similarly the columns of the partition being detached retain the NOT
  NULL constraints that came with identity property, even though the
  identity property itself is lost.

  The sequence associated with the identity property is linked to the
  partitioned table (and not the partition being detached).  That
  sequence is not dropped as part of detach operation.

* Partitions with their own identity columns are not allowed.

* The usual ALTER operations (add identity column, add identity
  property to existing column, alter properties of an indentity
  column, drop identity property) are supported for partitioned
  tables.  Changing a column only in a partitioned table or a
  partition is not allowed; the change needs to be applied to the
  whole partition hierarchy.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/CAExHW5uOykuTC+C6R1yDSp=o8Q83jr8xJdZxgPkxfZ1Ue5RRGg@mail.gmail.com
2024-01-16 17:24:52 +01:00
Peter Eisentraut
d22d98c341 Assert that partition inherits from only one parent in MergeAttributes()
A partition inherits only from one partitioned table and thus inherits
a column definition only once.  Assert the same in MergeAttributes()
and simplify a condition accordingly.

Similar definition exists about line 3068 in the same function.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAExHW5uOykuTC+C6R1yDSp=o8Q83jr8xJdZxgPkxfZ1Ue5RRGg@mail.gmail.com
2024-01-16 08:57:35 +01:00
Tom Lane
6545ba96cb Prevent access to an unpinned buffer in BEFORE ROW UPDATE triggers.
When ExecBRUpdateTriggers switches to a new target tuple as a result
of the EvalPlanQual logic, it must form a new proposed update tuple.
Since commit 86dc90056, that tuple (the result of
ExecGetUpdateNewTuple) has been a virtual tuple that might contain
pointers to by-ref fields of the new target tuple (in "oldslot").
However, immediately after that we materialize oldslot, causing it to
drop its buffer pin, whereupon the by-ref pointers are unsafe to use.
This is a live bug only when the new target tuple is in a different
page than the original target tuple, since we do still hold a pin on
the original one.  (Before 86dc90056, there was no bug because the
EPQ plantree would hold a pin on the new target tuple; but now that's
not assured.)  To fix, forcibly materialize the new tuple before we
materialize oldslot.  This costs nothing since we would have done that
shortly anyway.

The real-world impact of this is probably minimal.  A visible failure
could occur if the new target tuple's buffer were recycled for some
other page in the short interval before we materialize newslot within
the trigger-calling loop; but that's quite unlikely given that we'd
just touched that page.  There's a larger hazard that some other
process could prune and repack that page within the window.  We have
lock on the new target tuple, but that wouldn't prevent it being moved
on the page.

Alexander Lakhin and Tom Lane, per bug #17798 from Alexander Lakhin.
Back-patch to v14 where 86dc90056 came in.

Discussion: https://postgr.es/m/17798-0907404928dcf0dd@postgresql.org
2024-01-14 12:38:41 -05:00
Peter Eisentraut
e3aa802e6f Remove useless Assert
It's already included in the subsequent intVal() call.  Fixup for
4f622503d6.
2024-01-14 07:29:45 +01:00
Peter Eisentraut
4f622503d6 Make attstattarget nullable
This changes the pg_attribute field attstattarget into a nullable
field in the variable-length part of the row.  If no value is set by
the user for attstattarget, it is now null instead of previously -1.
This saves space in pg_attribute and tuple descriptors for most
practical scenarios.  (ATTRIBUTE_FIXED_PART_SIZE is reduced from 108
to 104.)  Also, null is the semantically more correct value.

The ANALYZE code internally continues to represent the default
statistics target by -1, so that that code can avoid having to deal
with null values.  But that is now contained to the ANALYZE code.
Only the DDL code deals with attstattarget possibly null.

For system columns, the field is now always null.  The ANALYZE code
skips system columns anyway.

To set a column's statistics target to the default value, the new
command form ALTER TABLE ... SET STATISTICS DEFAULT can be used.  (SET
STATISTICS -1 still works.)

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184c76@eisentraut.org
2024-01-13 18:14:53 +01:00
Peter Eisentraut
a1604237a6 Refactor ATExecAddColumn() to use BuildDescForRelation()
BuildDescForRelation() has all the knowledge for converting a
ColumnDef into pg_attribute/tuple descriptor.  ATExecAddColumn() can
make use of that, instead of duplicating all that logic.  We just pass
a one-element list of ColumnDef and we get back exactly the data
structure we need.  Note that we don't even need to touch
BuildDescForRelation() to make this work.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
2024-01-12 16:11:29 +01:00
Peter Eisentraut
5d06e99a3c ALTER TABLE command to change generation expression
This adds a new ALTER TABLE subcommand ALTER COLUMN ... SET EXPRESSION
that changes the generation expression of a generated column.

The syntax is not standard but was adapted from other SQL
implementations.

This command causes a table rewrite, using the usual ALTER TABLE
mechanisms.  The implementation is similar to and makes use of some of
the infrastructure of the SET DATA TYPE subcommand (for example,
rebuilding constraints and indexes afterwards).  The new command
requires a new pass in AlterTablePass, and the ADD COLUMN pass had to
be moved earlier so that combinations of ADD COLUMN and SET EXPRESSION
can work.

Author: Amul Sul <sulamul@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b94yyJeGA-5M951_Lr+KfZokOp-2kXicpmEhi5FXhBeTog@mail.gmail.com
2024-01-04 16:28:54 +01:00
Bruce Momjian
29275b1d17 Update copyright for 2024
Reported-by: Michael Paquier

Discussion: https://postgr.es/m/ZZKTDPxBBMt3C0J9@paquier.xyz

Backpatch-through: 12
2024-01-03 20:49:05 -05:00
Peter Eisentraut
d4e66a39eb Refactor: separate function to find all objects depending on a column
Move code from ATExecAlterColumnType() that finds the all the objects
that depend on the column to a separate function.  A future patch will
reuse this code.

Author: Amul Sul <sulamul@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b94yyJeGA-5M951_Lr+KfZokOp-2kXicpmEhi5FXhBeTog@mail.gmail.com
2024-01-03 08:50:37 +01:00
Robert Haas
0d9937d118 Fix typos in comments and in one isolation test.
Dagfinn Ilmari Mannsåker, reviewed by Shubham Khanna. Some subtractions
by me.

Discussion: http://postgr.es/m/87le9fmi01.fsf@wibble.ilmari.org
2024-01-02 12:05:41 -05:00
Amit Kapila
9a17be1e24 Allow upgrades to preserve the full subscription's state.
This feature will allow us to replicate the changes on subscriber nodes
after the upgrade.

Previously, only the subscription metadata information was preserved.
Without the list of relations and their state, it's not possible to
re-enable the subscriptions without missing some records as the list of
relations can only be refreshed after enabling the subscription (and
therefore starting the apply worker).  Even if we added a way to refresh
the subscription while enabling a publication, we still wouldn't know
which relations are new on the publication side, and therefore should be
fully synced, and which shouldn't.

To preserve the subscription relations, this patch teaches pg_dump to
restore the content of pg_subscription_rel from the old cluster by using
binary_upgrade_add_sub_rel_state SQL function. This is supported only
in binary upgrade mode.

The subscription's replication origin is needed to ensure that we don't
replicate anything twice.

To preserve the replication origins, this patch teaches pg_dump to update
the replication origin along with creating a subscription by using
binary_upgrade_replorigin_advance SQL function to restore the
underlying replication origin remote LSN. This is supported only in
binary upgrade mode.

pg_upgrade will check that all the subscription relations are in 'i'
(init) or in 'r' (ready) state and will error out if that's not the case,
logging the reason for the failure. This helps to avoid the risk of any
dangling slot or origin after the upgrade.

Author: Vignesh C, Julien Rouhaud, Shlok Kyal
Reviewed-by: Peter Smith, Masahiko Sawada, Michael Paquier, Amit Kapila, Hayato Kuroda
Discussion: https://postgr.es/m/20230217075433.u5mjly4d5cr4hcfe@jrouhaud
2024-01-02 08:08:46 +05:30
Peter Eisentraut
cea89c93a1 Turn AT_PASS_* macros into an enum
This make this code simpler and easier to follow.  Also, patches that
want to change the passes won't have to renumber the whole list.

Reviewed-by: Amul Sul <sulamul@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b94yyJeGA-5M951_Lr+KfZokOp-2kXicpmEhi5FXhBeTog@mail.gmail.com
2024-01-01 19:17:18 +01:00
Tom Lane
59bd34c2fa Fix bugs in manipulation of large objects.
In v16 and up (since commit afbfc0298), large object ownership
checking has been broken because object_ownercheck() didn't take care
of the discrepancy between our object-address representation of large
objects (classId == LargeObjectRelationId) and the catalog where their
ownership info is actually stored (LargeObjectMetadataRelationId).
This resulted in failures such as "unrecognized class ID: 2613"
when trying to update blob properties as a non-superuser.

Poking around for related bugs, I found that AlterObjectOwner_internal
would pass the wrong classId to the PostAlterHook in the no-op code
path where the large object already has the desired owner.  Also,
recordExtObjInitPriv checked for the wrong classId; that bug is only
latent because the stanza is dead code anyway, but as long as we're
carrying it around it should be less wrong.  These bugs are quite old.

In HEAD, we can reduce the scope for future bugs of this ilk by
changing AlterObjectOwner_internal's API to let the translation happen
inside that function, rather than requiring callers to know about it.

A more bulletproof fix, perhaps, would be to start using
LargeObjectMetadataRelationId as the dependency and object-address
classId for blobs.  However that has substantial risk of breaking
third-party code; even within our own code, it'd create hassles
for pg_dump which would have to cope with a version-dependent
representation.  For now, keep the status quo.

Discussion: https://postgr.es/m/2650449.1702497209@sss.pgh.pa.us
2023-12-15 13:55:05 -05:00
Heikki Linnakangas
049ef3398d Don't try to open visibilitymap when analyzing a foreign table
It's harmless, visibilitymap_count() returns 0 if the file doesn't
exist. But it's also very pointless. I noticed this when I added an
assertion in smgropen() that the relnumber is valid.

Discussion: https://www.postgresql.org/message-id/621a52fd-3cd8-4f5d-a561-d510b853bbaf@iki.fi
2023-12-08 09:16:21 +02:00
Michael Paquier
d43bd090a8 Improve some error messages with invalid indexes for REINDEX CONCURRENTLY
An invalid index is skipped when doing REINDEX CONCURRENTLY at table
level, with INDEX_CORRUPTED used as errcode.  This is confusing,
because an invalid index could exist after an interruption.  The errcode
is switched to OBJECT_NOT_IN_PREREQUISITE_STATE instead, as per a
suggestion from Andres Freund.

While on it, the error messages are reworded, and a hint is added,
telling how to rebuild an invalid index in this case.  This has been
suggested by Noah Misch.

Discussion: https://postgr.es/m/20231118230958.4fm3fhk4ypshxopa@awork3.anarazel.de
2023-12-07 14:27:54 +09:00