aboutsummaryrefslogtreecommitdiff
path: root/src/backend/commands
Commit message (Collapse)AuthorAge
* Support MERGE into updatable views.Dean Rasheed2024-02-29
| | | | | | | | | | | | | | | | | | | This allows the target relation of MERGE to be an auto-updatable or trigger-updatable view, and includes support for WITH CHECK OPTION, security barrier views, and security invoker views. A trigger-updatable view must have INSTEAD OF triggers for every type of action (INSERT, UPDATE, and DELETE) mentioned in the MERGE command. An auto-updatable view must not have any INSTEAD OF triggers. Mixing auto-update and trigger-update actions (i.e., having a partial set of INSTEAD OF triggers) is not supported. Rule-updatable views are also not supported, since there is no rewriter support for non-SELECT rules with MERGE operations. Dean Rasheed, reviewed by Jian He and Alvaro Herrera. Discussion: https://postgr.es/m/CAEZATCVcB1g0nmxuEc-A+gGB0HnfcGQNGYH7gS=7rq0u0zOBXA@mail.gmail.com
* Improve performance of subsystems on top of SLRUAlvaro Herrera2024-02-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | More precisely, what we do here is make the SLRU cache sizes configurable with new GUCs, so that sites with high concurrency and big ranges of transactions in flight (resp. multixacts/subtransactions) can benefit from bigger caches. In order for this to work with good performance, two additional changes are made: 1. the cache is divided in "banks" (to borrow terminology from CPU caches), and algorithms such as eviction buffer search only affect one specific bank. This forestalls the problem that linear searching for a specific buffer across the whole cache takes too long: we only have to search the specific bank, whose size is small. This work is authored by Andrey Borodin. 2. Change the locking regime for the SLRU banks, so that each bank uses a separate LWLock. This allows for increased scalability. This work is authored by Dilip Kumar. (A part of this was previously committed as d172b717c6f4.) Special care is taken so that the algorithms that can potentially traverse more than one bank release one bank's lock before acquiring the next. This should happen rarely, but particularly clog.c's group commit feature needed code adjustment to cope with this. I (Álvaro) also added lots of comments to make sure the design is sound. The new GUCs match the names introduced by bcdfa5f2e2f2 in the pg_stat_slru view. The default values for these parameters are similar to the previous sizes of each SLRU. commit_ts, clog and subtrans accept value 0, which means to adjust by dividing shared_buffers by 512 (so 2MB for every 1GB of shared_buffers), with a cap of 8MB. (A new slru.c function SimpleLruAutotuneBuffers() was added to support this.) The cap was previously 1MB for clog, so for sites with more than 512MB of shared memory the total memory used increases, which is likely a good tradeoff. However, other SLRUs (notably multixact ones) retain smaller sizes and don't support a configured value of 0. These values based on shared_buffers may need to be revisited, but that's an easy change. There was some resistance to adding these new GUCs: it would be better to adjust to memory pressure automatically somehow, for example by stealing memory from shared_buffers (where the caches can grow and shrink naturally). However, doing that seems to be a much larger project and one which has made virtually no progress in several years, and because this is such a pain point for so many users, here we take the pragmatic approach. Author: Andrey Borodin <x4mmm@yandex-team.ru> Author: Dilip Kumar <dilipbalaut@gmail.com> Reviewed-by: Amul Sul, Gilles Darold, Anastasia Lubennikova, Ivan Lazarev, Robert Haas, Thomas Munro, Tomas Vondra, Yura Sokolov, Васильев Дмитрий (Dmitry Vasiliev). Discussion: https://postgr.es/m/2BEC2B3F-9B61-4C1D-9FB5-5FAB0F05EF86@yandex-team.ru Discussion: https://postgr.es/m/CAFiTN-vzDvNz=ExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A@mail.gmail.com
* Rename SLRU elements in view pg_stat_slruAlvaro Herrera2024-02-28
| | | | | | | | | | | | | | The new names are intended to match those in an upcoming patch that adds a few GUCs to configure the SLRU buffer sizes. Backwards compatibility concern: this changes the accepted names for function pg_stat_slru_rest(). Since this function recognizes "any other string" as a request to reset the entry for "other", this means that calling it with the old names would silently reset "other" instead of doing nothing or throwing an error. Reviewed-by: Andrey M. Borodin <x4mmm@yandex-team.ru> Discussion: https://postgr.es/m/202402261616.dlriae7b6emv@alvherre.pgsql
* Group more closely cache updates for backends in sequence.cMichael Paquier2024-02-26
| | | | | | | | | | | | | | | | | | | Information of sequences is cached for each backend for currval() and nextval(), and the update of some cached information was mixed in the middle of computations based on the other properties of a sequence, for the increment value in nextval() and the cached state when altering a sequence. Grouping them makes the code easier to follow and to refactor in the future, when splitting the computation and the SeqTable change parts. Note that the cached data is untouched between the areas where these cache updates are moved. Issue noticed while doing some refactoring of the sequence code. Author: Michael Paquier Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/ZWlohtKAs0uVVpZ3@paquier.xyz
* Introduce sequence_*() access functionsMichael Paquier2024-02-26
| | | | | | | | | | | | | | | | | | Similarly to tables and indexes, these functions are able to open relations with a sequence relkind, which is useful to make a distinction with the other relation kinds. Previously, commands/sequence.c used a mix of table_{close,open}() and relation_{close,open}() routines when manipulating sequence relations, so this clarifies the code. A direct effect of this change is to align the error messages produced when attempting DDLs for sequences on relations with an unexpected relkind, like a table or an index with ALTER SEQUENCE, providing an extra error detail about the relkind of the relation used in the DDL query. Author: Michael Paquier Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/ZWlohtKAs0uVVpZ3@paquier.xyz
* Remove extra check_stack_depth() from dropconstraint_internal()Alexander Korotkov2024-02-21
| | | | | | | | The second check was added by d57b7cc33 without taking into account there is already a check since b0f7dd915. Reported-by: Ashutosh Bapat, Alexander Lakhin Discussion: https://postgr.es/m/CAExHW5sBZWDjeBUFs_ehEDM%2BuhWxTiBkPbLiat7ZjWkb-DWQWw%40mail.gmail.com
* Revert "Improve compression and storage support with inheritance"Peter Eisentraut2024-02-20
| | | | | | | | | This reverts commit 0413a556990ba628a3de8a0b58be020fd9a14ed0. pg_dump cannot currently dump all the structures that are allowed by this patch. This needs more work in pg_dump and more test coverage. Discussion: https://www.postgresql.org/message-id/flat/24656cec-d6ef-4d15-8b5b-e8dfc9c833a7@eisentraut.org
* Add missing check_stack_depth() to some recursive functionsAlexander Korotkov2024-02-16
| | | | | Reported-by: Egor Chindyaskin, Alexander Lakhin Discussion: https://postgr.es/m/1672760457.940462079%40f306.i.mail.ru
* Improve compression and storage support with inheritancePeter Eisentraut2024-02-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A child table can specify a compression or storage method different from its parents. This was previously an error. (But this was inconsistently enforced because for example the settings could be changed later using ALTER TABLE.) This now also allows an explicit override if multiple parents have different compression or storage settings, which was previously an error that could not be overridden. The compression and storage properties remains unchanged in a child inheriting from parent(s) after its creation, i.e., when using ALTER TABLE ... INHERIT. (This is not changed.) Before this change, the error detail would mention the first pair of conflicting parent compression or storage methods. But with this change it waits till the child specification is considered by which time we may have encountered many such conflicting pairs. Hence the error detail after this change does not include the conflicting compression/storage methods. Those can be obtained from parent definitions if necessary. The code to maintain list of all conflicting methods or even the first conflicting pair does not seem worth the convenience it offers. This change is inline with what we do with conflicting default values. Before this commit, the specified storage method could be stored in ColumnDef::storage (CREATE TABLE ... LIKE) or ColumnDef::storage_name (CREATE TABLE ...). This caused the MergeChildAttribute() and MergeInheritedAttribute() to ignore a storage method specified in the child definition since it looked only at ColumnDef::storage. This commit removes ColumnDef::storage and instead uses ColumnDef::storage_name to save any storage method specification. This is similar to how compression method specification is handled. Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/24656cec-d6ef-4d15-8b5b-e8dfc9c833a7@eisentraut.org
* Fix multiranges to behave more like dependent types.Tom Lane2024-02-14
| | | | | | | | | | | | | | | | | | | | | For most purposes, multiranges act like dependent objects of the associated range type: you can't create them separately or drop them separately. This is like the way that autogenerated array types behave. However, a couple of points were overlooked: array types automatically track the ownership of their base type, and array types do not have their own permissions but use those of the base type, while multiranges didn't emulate those behaviors. This is fairly broken, mainly because pg_dump doesn't think it needs to worry about multiranges as separate objects, and thus it fails to dump/restore ownership or permissions of multiranges. There's no apparent value in letting a multirange diverge from its parent's ownership or permissions, so let's make them act like arrays in these respects. However, we continue to let multiranges be renamed or moved to a different schema independently of their parent, since that doesn't break anything. Discussion: https://postgr.es/m/1580383.1705343264@sss.pgh.pa.us
* Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"Michael Paquier2024-02-14
| | | | | | | | | | | | | | This reverts commit 95fb5b49024, for reasons similar to what led to 1aa8324b81fa. In this case, the callback was called once per row, which is less worse than the previous callback introduced for COPY TO called once per argument for each row, still the patch set discussed to plug in custom routines to the COPY paths would be able to know which subroutine to use depending on its CopyFromState, so this led to a suboptimal approach at the end. For now, this part is reverted to consider better which approach to use. Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de
* Remove unnecessary smgropen() callsHeikki Linnakangas2024-02-12
| | | | | | | | 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
* Use heap_inplace_update() to unset pg_database.dathasloginevtAlexander Korotkov2024-02-12
| | | | | | | | | | | | | 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
* Fix indentation of copyto.cMichael Paquier2024-02-09
| | | | | | Issue introduced by b619852086ed. Per buildfarm member koel.
* Improve COPY TO performance when server and client encodings matchMichael Paquier2024-02-09
| | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix gcc >= 10 warningAlexander Korotkov2024-02-08
| | | | | Reported-by: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACVQoFXxFm2kCmhHcdM7DjA84_bOjoM8HVAKHbE%2BKrZ1uA%40mail.gmail.com
* Clean-ups for 776621a5e4 and 7329240437.Amit Kapila2024-02-07
| | | | | | | | | | | | | | | | 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
* Simplify signature of CopyAttributeOutCSV() in copyto.cMichael Paquier2024-02-07
| | | | | | | | 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
* Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"Michael Paquier2024-02-07
| | | | | | | | | | | | 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
* Fix assertion if index is dropped during REFRESH CONCURRENTLYHeikki Linnakangas2024-02-05
| | | | | | | | | | 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
* Run REFRESH MATERIALIZED VIEW CONCURRENTLY in right security contextHeikki Linnakangas2024-02-05
| | | | | | | | | | | | | | | | | | | | | | | | | 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
* Enhance libpqrcv APIs to support slot synchronization.Amit Kapila2024-02-05
| | | | | | | | | | | | | | | | | 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
* Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TOMichael Paquier2024-02-05
| | | | | | | | | | | | | | | | | | 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
* Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROMMichael Paquier2024-02-05
| | | | | | | | | | | | | 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
* Sync PG_VERSION file in CREATE DATABASE.Noah Misch2024-02-01
| | | | | | | | | | | 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
* Handle interleavings between CREATE DATABASE steps and base backup.Noah Misch2024-02-01
| | | | | | | | | | | | | | | | | 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
* Give SMgrRelation pointers a well-defined lifetime.Heikki Linnakangas2024-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix various issues with ALTER TEXT SEARCH CONFIGURATIONMichael Paquier2024-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Add a failover option to subscriptions.Amit Kapila2024-01-30
| | | | | | | | | | | | | | | | | | | | | 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
* Doc: mention foreign keys can reference unique indexesDavid Rowley2024-01-30
| | | | | | | | | | | | | | | | 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
* Add EXPLAIN (MEMORY) to report planner memory consumptionAlvaro Herrera2024-01-29
| | | | | | | | | | | | | | | | | | | | 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
* Allow setting failover property in the replication command.Amit Kapila2024-01-29
| | | | | | | | | | | | | | | | 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
* Fix DROP ROLE when specifying duplicated rolesMichael Paquier2024-01-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Split some code out from MergeAttributes()Peter Eisentraut2024-01-26
| | | | | | | | | | | | | | | | | | | | | | - 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
* MergeAttributes code deduplicationPeter Eisentraut2024-01-26
| | | | | | | | 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
* MergeAttributes: convert pg_attribute back to ColumnDef before comparingPeter Eisentraut2024-01-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Add progress reporting of skipped tuples during COPY FROM.Masahiko Sawada2024-01-25
| | | | | | | | | | | | | | | | 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
* Add temporal PRIMARY KEY and UNIQUE constraintsPeter Eisentraut2024-01-24
| | | | | | | | | | | | 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
* Fix ALTER TABLE .. ADD COLUMN with complex inheritance treesMichael Paquier2024-01-24
| | | | | | | | | | | | | | | | 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
* Fix some typosMichael Paquier2024-01-22
| | | | | Author: Yongtao Huang Discussion: https://postgr.es/m/CAOe1Go1F99o5JsphtXdDC5bxm7AzetU8q3AxLh4AAVGKu1AzEQ@mail.gmail.com
* Rename COPY option from SAVE_ERROR_TO to ON_ERRORAlexander Korotkov2024-01-19
| | | | | | | | | 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
* Improve some documentation about the bootstrap superuser.Nathan Bossart2024-01-18
| | | | | | | | | | | | | | 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
* Error message capitalisationPeter Eisentraut2024-01-18
| | | | | | | 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
* Fix spelling in noticeAlexander Korotkov2024-01-17
| | | | | Reported-by: Atsushi Torikoshi Discussion: https://postgr.es/m/762d7dd4d5aa9e5ecffec2ae6a255a28%40oss.nttdata.com
* Fix format specifier for NOTICE in copyfrom.cAlexander Korotkov2024-01-17
| | | | It's incorrect to use %lz for 64-bit numbers on 32-bit machine.
* Add new COPY option SAVE_ERROR_TOAlexander Korotkov2024-01-16
| | | | | | | | | | | | | | | | | | | | | 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
* Support identity columns in partitioned tablesPeter Eisentraut2024-01-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Assert that partition inherits from only one parent in MergeAttributes()Peter Eisentraut2024-01-16
| | | | | | | | | | | 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
* Prevent access to an unpinned buffer in BEFORE ROW UPDATE triggers.Tom Lane2024-01-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Remove useless AssertPeter Eisentraut2024-01-14
| | | | | It's already included in the subsequent intVal() call. Fixup for 4f622503d6.