aboutsummaryrefslogtreecommitdiff
path: root/src/backend/commands
Commit message (Collapse)AuthorAge
...
* Make the order of the header file includes consistentPeter Eisentraut2024-03-13
| | | | | | | | Similar to commit 7e735035f20. Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4-WhpCFMbXCjtJ%2BFzmjfPrp7Hw1pk4p%2BZpU95Kh3ofZ1A%40mail.gmail.com
* Improve support for ExplainOneQuery() hookMichael Paquier2024-03-11
| | | | | | | | | | | | | | | | | | | | There is a hook called ExplainOneQuery_hook that gives modules the possibility to plug into this code path, but, like utility.c for utility statement execution, there is no corresponding "standard" routine in the case of EXPLAIN executed for one Query. This commit adds a new standard_ExplainOneQuery() in explain.c, which is able to run explain on a non-utility Query without calling its hook. Per the feedback received from a couple of hackers, this change gives the possibility to cut a few hundred lines of code in some of the popular out-of-core modules as these maintained a copy of ExplainOneQuery(), adding custom extra information at the beginning or the end of the EXPLAIN output. Author: Mats Kindahl Reviewed-by: Aleksander Alekseev, Jelte Fennema-Nio, Andrei Lepikhov Discussion: https://postgr.es/m/CA+14427V_B4EAoC_o-iYYucRdMSOTfpuH9k-QbexffY1HYJBiA@mail.gmail.com
* Catalog changes preparing for builtin collation provider.Jeff Davis2024-03-09
| | | | | | | | | | | | Rename pg_collation.colliculocale to colllocale, and pg_database.daticulocale to datlocale. These names reflects that the fields will be useful for the upcoming builtin provider as well, not just for ICU. This is purely a rename; no changes to the meaning of the fields. Discussion: https://postgr.es/m/ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com Reviewed-by: Peter Eisentraut
* Add support for DEFAULT in ALTER TABLE .. SET ACCESS METHODMichael Paquier2024-03-08
| | | | | | | | | | | | | | | | This option can be used to switch a relation to use the access method set by default_table_access_method when running the command. This has come up when discussing the possibility to support setting pg_class.relam for partitioned tables (left out here as future work), while being useful on its own for relations with physical storage as these must have an access method set. Per suggestion from Justin Pryzby. Author: Michael Paquier Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/ZeCZ89xAVFeOmrQC@pryzbyj2023
* Rename pg_constraint.conwithoutoverlaps to conperiodPeter Eisentraut2024-03-05
| | | | | | | | | | | | | | | | | pg_constraint.conwithoutoverlaps was recently added to support primary keys and unique constraints with the WITHOUT OVERLAPS clause. An upcoming patch provides the foreign-key side of this functionality, but the syntax there is different and uses the keyword PERIOD. It would make sense to use the same pg_constraint field for both of these, but then we should pick a more general name that conveys "this constraint has a temporal/period-related feature". conperiod works for that and is nicely compact. Changing this now avoids possibly having to introduce versioning into clients. Note there are still some "without overlaps" variables left, which deal specifically with the parsing of the primary key/unique constraint feature. Author: Paul A. Jungwirth <pj@illuminatedcomputing.com> Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
* Fix buildfarm failures from 2af07e2f74.Jeff Davis2024-03-04
| | | | | | | | | | Use GUC_ACTION_SAVE rather than GUC_ACTION_SET, necessary for working with parallel query. Now that the call requires more arguments, wrap the call in a new function to avoid code duplication and offer a place for a comment. Discussion: https://postgr.es/m/E1rhJpO-0027Wf-9L@gemulon.postgresql.org
* Fix search_path to a safe value during maintenance operations.Jeff Davis2024-03-04
| | | | | | | | | | | | | | | | | | | | | While executing maintenance operations (ANALYZE, CLUSTER, REFRESH MATERIALIZED VIEW, REINDEX, or VACUUM), set search_path to 'pg_catalog, pg_temp' to prevent inconsistent behavior. Functions that are used for functional indexes, in index expressions, or in materialized views and depend on a different search path must be declared with CREATE FUNCTION ... SET search_path='...'. This change was previously committed as 05e1737351, then reverted in commit 2fcc7ee7af because it was too late in the cycle. Preparation for the MAINTAIN privilege, which was previously reverted due to search_path manipulation hazards. Discussion: https://postgr.es/m/d4ccaf3658cb3c281ec88c851a09733cd9482f22.camel@j-davis.com Discussion: https://postgr.es/m/E1q7j7Y-000z1H-Hr%40gemulon.postgresql.org Discussion: https://postgr.es/m/e44327179e5c9015c8dda67351c04da552066017.camel%40j-davis.com Reviewed-by: Greg Stark, Nathan Bossart, Noah Misch
* Explicitly list dependent types as extension members in pg_depend.Tom Lane2024-03-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Auto-generated array types, multirange types, and relation rowtypes are treated as dependent objects: they can't be dropped separately from the base object, nor can they have their own ownership or permissions. We previously felt that, for objects that are in an extension, only the base object needs to be listed as an extension member in pg_depend. While that's sufficient to prevent inappropriate drops, it results in undesirable answers if someone asks whether a dependent type belongs to the extension. It looks like the dependent type is just some random separately-created object that happens to depend on the base object. Notably, this results in postgres_fdw concluding that expressions involving an array type are not shippable to the remote server, even when the defining extension has been whitelisted. To fix, cause GenerateTypeDependencies to make extension dependencies for dependent types as well as their base objects, and adjust ExecAlterExtensionContentsStmt so that object addition and removal operations recurse to dependent types. The latter change means that pg_upgrade of a type-defining extension will end with the dependent type(s) now also listed as extension members, even if they were not that way in the source database. Normally we want pg_upgrade to precisely reproduce the source extension's state, but it seems desirable to make an exception here. This is arguably a bug fix, but we can't back-patch it since it causes changes in the expected contents of pg_depend. (Because it does, I've bumped catversion, even though there's no change in the immediate post-initdb catalog contents.) Tom Lane and David Geier Discussion: https://postgr.es/m/4a847c55-489f-4e8d-a664-fc6b1cbe306f@gmail.com
* Remove unused #include's from backend .c filesPeter Eisentraut2024-03-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | as determined by include-what-you-use (IWYU) While IWYU also suggests to *add* a bunch of #include's (which is its main purpose), this patch does not do that. In some cases, a more specific #include replaces another less specific one. Some manual adjustments of the automatic result: - IWYU currently doesn't know about includes that provide global variable declarations (like -Wmissing-variable-declarations), so those includes are being kept manually. - All includes for port(ability) headers are being kept for now, to play it safe. - No changes of catalog/pg_foo.h to catalog/pg_foo_d.h, to keep the patch from exploding in size. Note that this patch touches just *.c files, so nothing declared in header files changes in hidden ways. As a small example, in src/backend/access/transam/rmgr.c, some IWYU pragma annotations are added to handle a special case there. Discussion: https://www.postgresql.org/message-id/flat/af837490-6b2f-46df-ba05-37ea6a6653fc%40eisentraut.org
* Use MyBackendType in more places to check what process this isHeikki Linnakangas2024-03-04
| | | | | | | | | | Remove IsBackgroundWorker, IsAutoVacuumLauncherProcess(), IsAutoVacuumWorkerProcess(), and IsLogicalSlotSyncWorker() in favor of new Am*Process() macros that use MyBackendType. For consistency with the existing Am*Process() macros. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/f3ecd4cb-85ee-4e54-8278-5fabfb3a4ed0@iki.fi
* Replace BackendIds with 0-based ProcNumbersHeikki Linnakangas2024-03-03
| | | | | | | | | | | | | | | | | | Now that BackendId was just another index into the proc array, it was redundant with the 0-based proc numbers used in other places. Replace all usage of backend IDs with proc numbers. The only place where the term "backend id" remains is in a few pgstat functions that expose backend IDs at the SQL level. Those IDs are now in fact 0-based ProcNumbers too, but the documentation still calls them "backend ids". That term still seems appropriate to describe what the numbers are, so I let it be. One user-visible effect is that pg_temp_0 is now a valid temp schema name, for backend with ProcNumber 0. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi
* Redefine backend ID to be an index into the proc arrayHeikki Linnakangas2024-03-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, backend ID was an index into the ProcState array, in the shared cache invalidation manager (sinvaladt.c). The entry in the ProcState array was reserved at backend startup by scanning the array for a free entry, and that was also when the backend got its backend ID. Things become slightly simpler if we redefine backend ID to be the index into the PGPROC array, and directly use it also as an index to the ProcState array. This uses a little more memory, as we reserve a few extra slots in the ProcState array for aux processes that don't need them, but the simplicity is worth it. Aux processes now also have a backend ID. This simplifies the reservation of BackendStatusArray and ProcSignal slots. You can now convert a backend ID into an index into the PGPROC array simply by subtracting 1. We still use 0-based "pgprocnos" in various places, for indexes into the PGPROC array, but the only difference now is that backend IDs start at 1 while pgprocnos start at 0. (The next commmit will get rid of the term "backend ID" altogether and make everything 0-based.) There is still a 'backendId' field in PGPROC, now part of 'vxid' which encapsulates the backend ID and local transaction ID together. It's needed for prepared xacts. For regular backends, the backendId is always equal to pgprocno + 1, but for prepared xact PGPROC entries, it's the ID of the original backend that processed the transaction. Reviewed-by: Andres Freund, Reid Thompson Discussion: https://www.postgresql.org/message-id/8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi
* 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