aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Remove field UpdateContext->updated in nodeModifyTable.cDean Rasheed2024-02-29
| | | | | | | | | | | | | | | | | | This field has been redundant ever since it was added by commit 25e777cf8e, which split up ExecUpdate() and ExecDelete() into reusable pieces. The only place that reads it is ExecMergeMatched(), if the result from ExecUpdateAct() is TM_Ok. However, all paths through ExecUpdateAct() that return TM_Ok also set this field to true, so the return status by itself is sufficient to tell if the update happened. Removing this field is a modest simplification, and it brings the UPDATE path in ExecMergeMatched() more into line with ExecUpdate(), ensuring that ExecUpdateEpilogue() is always called if ExecUpdateAct() returns TM_Ok, reducing the chance of bugs. Dean Rasheed, reviewed by Alvaro Herrera. Discussion: https://postgr.es/m/CAEZATCWGGmigGBzLHkJm5Ccv2mMxXmwi3%2Buq0yhwDHm-tsvSLg%40mail.gmail.com
* Fix integer underflow in shared memory debuggingDaniel Gustafsson2024-02-29
| | | | | | | | | | | dsa_dump would print a large negative number instead of zero for segment bin 0. Fix by explicitly checking for underflow and add special case for bin 0. Backpatch to all supported versions. Author: Ian Ilyasov <ianilyasov@outlook.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/GV1P251MB1004E0D09D117D3CECF9256ECD502@GV1P251MB1004.EURP251.PROD.OUTLOOK.COM Backpatch-through: v12
* Fixups for commit 93db6cbda0.Amit Kapila2024-02-29
| | | | | | | | | | | | | | | Ensure to set always-secure search path for both local and remote connections during slot synchronization, so that malicious users can't redirect user code (e.g. operators). In the passing, improve the name of define, remove spurious return statement, and a minor change in one of the comments. Author: Bertrand Drouvot and Shveta Malik Reviewed-by: Amit Kapila, Peter Smith Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com Discussion: https://postgr.es/m/ZdcejBDCr+wlVGnO@ip-10-97-1-34.eu-west-3.compute.internal Discussion: https://postgr.es/m/CAJpy0uBNP=nrkNJkJSfF=jSocEh8vU2Owa8Rtpi=63fG=SvfVQ@mail.gmail.com
* Use C99-designated initializer syntax for arrays related to encodingsMichael Paquier2024-02-29
| | | | | | | | | | | | | | | | | This updates the following lookup arrays to use C99-designated initializer syntax, indexed based on the enum pg_enc: pg_enc2icu_tbl[] pg_enc2name_tbl[] pg_wchar_table[] This is more readable, and removes problems with ordering mistakes as this removes dependencies between the arrays and their lookup index in the enum pg_enc. So, adding new encodings becomes easier, even if this does not happen often. Author: Jelte Fennema-Nio Reviewed-by: Jian He, Japin Li Discussion: https://postgr.es/m/CAGECzQT3caUbcCcszNewCCmMbCuyP7XNAm60J3ybd6PN5kH2Dw@mail.gmail.com
* Fix cross-version upgrade tests after f0827b443.Tom Lane2024-02-28
| | | | | | | | Removing the get_columns_length() function from regress.so means we have to drop it when testing upgrades from versions that had it. Per buildfarm. Discussion: https://postgr.es/m/2520881.1709159002@sss.pgh.pa.us
* Fix documentation comments for test CA config filesDaniel Gustafsson2024-02-28
| | | | | | | | | | The config files which are used to generate the server and client CAs claimed that these were self-signed, when they in reality are signed by the root_ca (which however is self-signed). Reword the comments to match. Author: David Zhang <david.zhang@highgo.ca> Discussion: https://postgr.es/m/12f4c425-45fe-480f-a692-b3ed82ebcb33@highgo.ca
* Improve plpgsql's error messages for incorrect %TYPE and %ROWTYPE.Tom Lane2024-02-28
| | | | | | | | | | | If one of these constructs referenced a nonexistent object, we'd fall through to feeding the whole construct to the core parser, which would reject it with a "syntax error" message. That's pretty unhelpful and misleading. There's no good reason for plpgsql_parse_wordtype and friends not to throw a useful error for incorrect input, so make them do that instead of returning NULL. Discussion: https://postgr.es/m/1964516.1708977740@sss.pgh.pa.us
* Mop-up for AIX-ectomy: remove now-dead test code.Tom Lane2024-02-28
| | | | | | | Commit 0b16bb877 removed the test query added by commit 79b716cfb, but not the C-language support function used by that query. I don't see any plausible reason why we'd need that function again, so throw it overboard too.
* Fix mis-rounding and overflow hazards in date_bin().Tom Lane2024-02-28
| | | | | | | | | | | | | | | | | | | | In the case where the target timestamp is before the origin timestamp and their difference is already an exact multiple of the stride, the code incorrectly subtracted the stride anyway. Also detect several integer-overflow cases that previously produced bogus results. (The submitted patch tried to avoid overflow, but I'm not convinced it's right, and problematic cases are so far out of the plausibly-useful range that they don't seem worth sweating over. Let's just use overflow-detecting arithmetic and throw errors.) timestamp_bin() and timestamptz_bin() are basically identical and so had identical bugs. Fix both. Report and patch by Moaaz Assali, adjusted some by me. Back-patch to v14 where date_bin() was introduced. Discussion: https://postgr.es/m/CALkF+nvtuas-2kydG-WfofbRSJpyODAJWun==W-yO5j2R4meqA@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
* Remove AIX supportHeikki Linnakangas2024-02-28
| | | | | | | | | | | | | | | | | | | | | | There isn't a lot of user demand for AIX support, we have a bunch of hacks to work around AIX-specific compiler bugs and idiosyncrasies, and no one has stepped up to the plate to properly maintain it. Remove support for AIX to get rid of that maintenance overhead. It's still supported for stable versions. The acute issue that triggered this decision was that after commit 8af2565248, the AIX buildfarm members have been hitting this assertion: TRAP: failed Assert("(uintptr_t) buffer == TYPEALIGN(PG_IO_ALIGN_SIZE, buffer)"), File: "md.c", Line: 472, PID: 2949728 Apperently the "pg_attribute_aligned(a)" attribute doesn't work on AIX for values larger than PG_IO_ALIGN_SIZE, for a static const variable. That could be worked around, but we decided to just drop the AIX support instead. Discussion: https://www.postgresql.org/message-id/20240224172345.32@rfd.leadboat.com Reviewed-by: Andres Freund, Noah Misch, Thomas Munro
* 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
* Remove last NULL element in config_group_names[]Michael Paquier2024-02-28
| | | | | | | | | | This has not been needed since 9d77708d83ee where there was a loop to print all the possible GUC groups, relying on the last element to be NULL. Author: Japin Li Reviewed-By: Jelte Fennema-Nio Discussion: https://postgr.es/m/CAGECzQT3caUbcCcszNewCCmMbCuyP7XNAm60J3ybd6PN5kH2Dw@mail.gmail.com
* Refactor AllocSetAlloc(), separating hot and cold pathsDavid Rowley2024-02-28
| | | | | | | | | | | | | | | | | | Allocating from a free list or from a block which contains enough space already, we deem to be common code paths and want to optimize for those. Having to allocate a new block, either a normal block or a dedicated one for a large allocation, we deem to be less common, therefore we class that as "cold". Both cold paths require a malloc so are going to be slower as a result of that regardless. The main motivation here is to remove the calls to malloc() in the hot path and because of this, the compiler is now free to not bother setting up the stack frame in AllocSetAlloc(), thus making the hot path much cheaper. Author: Andres Freund Reviewed-by: David Rowley Discussion: https://postgr.es/m/20210719195950.gavgs6ujzmjfaiig@alap3.anarazel.de
* Use C99-designated initializer syntax for more arraysMichael Paquier2024-02-28
| | | | | | | | | | | This is in the same spirit as ef5e2e90859a, updating this time some arrays in parser.c, relpath.c, guc_tables.c and pg_dump_sort.c so as the order of their elements has no need to match the enum structures they are based on anymore. Author: Jelte Fennema-Nio Reviewed-by: Jian He, Japin Li Discussion: https://postgr.es/m/CAGECzQT3caUbcCcszNewCCmMbCuyP7XNAm60J3ybd6PN5kH2Dw@mail.gmail.com
* Fix comments for the dshash_parameters struct.Nathan Bossart2024-02-27
| | | | | | | | | | | | | | A recent commit added a copy_function member to the dshash_parameters struct, but it missed updating a couple of comments that refer to the function pointer members of this struct. One of those comments also refers to a tranche_name member and non- arg variants of the function pointer members, all of which were either removed during development or removed shortly after dshash table support was committed. Oversights in commits 8c0d7bafad, d7694fc148, and 42a1de3013. Discussion: https://postgr.es/m/20240227045213.GA2329190%40nathanxps13
* Rationalize and improve error messages for some jsonpath itemsAndrew Dunstan2024-02-27
| | | | | | | | | | | | | | | | | | | | This is a followup to commit 66ea94e8e6. Error mssages concerning incorrect formats for date-time types are unified and parameterized, instead of using a fully separate error message for each type. Similarly, error messages regarding numeric and string arguments to certain items are standardized, and instead of saying that the argument is out of range simply say that it is invalid. The actual invalid arguments to these itesm are now shown in the error message. Error messages relating to numeric inputs of Nan or Infinity are made more informative. Jeevan Chalke and Kyotaro Horiguchi, with some input from Tom Lane. Discussion: https://postgr.es/m/20240129.121200.235012930453045390.horikyota.ntt@gmail.com
* Remove unnecessary array object_classes[] in dependency.cMichael Paquier2024-02-27
| | | | | | | | | | | | | | | | | | object_classes[] provided unnecessary indirection between catalog OIDs and the enum ObjectClass when calling add_object_address(). This array has been originally introduced in 30ec31604d5 and was useful because not all relation OIDs were compile-time constants back then, which has not been the case for a long time now for all the elements of ObjectClass. This commit removes object_classes[], switching to the catalog OIDs when calling add_object_address(). This shaves some code while saving in maintenance because it was necessary to maintain the enum ObjectClass and the array in sync when adding new object types. Reported-by: Jeff Davis Author: Jelte Fennema-Nio Reviewed-by: Jian He, Michael Paquier Discussion: https://postgr.es/m/CAGECzQT3caUbcCcszNewCCmMbCuyP7XNAm60J3ybd6PN5kH2Dw@mail.gmail.com
* Adjust memory allocation functions to allow sibling callsDavid Rowley2024-02-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many modern compilers are able to optimize function calls to functions where the parameters of the called function match a leading subset of the calling function's parameters. If there are no instructions in the calling function after the function is called, then the compiler is free to avoid any stack frame setup and implement the function call as a "jmp" rather than a "call". This is called sibling call optimization. Here we adjust the memory allocation functions in mcxt.c to allow this optimization. This requires moving some responsibility into the memory context implementations themselves. It's now the responsibility of the MemoryContext to check for malloc failures. This is good as it both allows the sibling call optimization, but also because most small and medium allocations won't call malloc and just allocate memory to an existing block. That can't fail, so checking for NULLs in that case isn't required. Also, traditionally it's been the responsibility of palloc and the other allocation functions in mcxt.c to check for invalid allocation size requests. Here we also move the responsibility of checking that into the MemoryContext. This isn't to allow the sibling call optimization, but more because most of our allocators handle large allocations separately and we can just add the size check when doing large allocations. We no longer check this for non-large allocations at all. To make checking the allocation request sizes and ERROR handling easier, add some helper functions to mcxt.c for the allocators to use. Author: Andres Freund Reviewed-by: David Rowley Discussion: https://postgr.es/m/20210719195950.gavgs6ujzmjfaiig@alap3.anarazel.de
* Fix comment thinko in sequence.cMichael Paquier2024-02-27
| | | | | | | | One comment mentioned indexes, but the relation opened should be sequences. Reported-by: Matthias van de Meent Discussion: https://postgr.es/m/CAEze2WiMGNG9XK3NSUen-5BARhCnP=u=FXnf8pvpL2qDKeOsZg@mail.gmail.com
* Add helper functions for dshash tables with string keys.Nathan Bossart2024-02-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Presently, string keys are not well-supported for dshash tables. The dshash code always copies key_size bytes into new entries' keys, and dshash.h only provides compare and hash functions that forward to memcmp() and tag_hash(), both of which do not stop at the first NUL. This means that callers must pad string keys so that the data beyond the first NUL does not adversely affect the results of copying, comparing, and hashing the keys. To better support string keys in dshash tables, this commit does a couple things: * A new copy_function field is added to the dshash_parameters struct. This function pointer specifies how the key should be copied into new table entries. For example, we only want to copy up to the first NUL byte for string keys. A dshash_memcpy() helper function is provided and used for all existing in-tree dshash tables without string keys. * A set of helper functions for string keys are provided. These helper functions forward to strcmp(), strcpy(), and string_hash(), all of which ignore data beyond the first NUL. This commit also adjusts the DSM registry's dshash table to use the new helper functions for string keys. Reviewed-by: Andy Fan Discussion: https://postgr.es/m/20240119215941.GA1322079%40nathanxps13
* Use NULL instead of 0 for 'arg' argument in dshash_create() calls.Nathan Bossart2024-02-26
| | | | | | | | | A couple of dshash_create() callers provide 0 for the 'void *arg' argument, which might give readers the incorrect impression that this is some sort of "flags" parameter. Reviewed-by: Andy Fan Discussion: https://postgr.es/m/20240119215941.GA1322079%40nathanxps13
* slru.c: Reduce scope of variables in 'for' blocksAlvaro Herrera2024-02-26
| | | | Pretty boring.
* 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
* Fix incorrect format placeholderPeter Eisentraut2024-02-26
| | | | | Not only did the format placeholder not match the variable, the variable also didn't match the function it was getting its value from.
* Promote assertion about !ReindexIsProcessingIndex to runtime error.Tom Lane2024-02-25
| | | | | | | | | | | | | | | | | | | | | When this assertion was installed (in commit d2f60a3ab), I thought it was only for catching server logic errors that caused accesses to catalogs that were undergoing index rebuilds. However, it will also fire in case of a user-defined index expression that attempts to access its own table. We occasionally see reports of people trying to do that, and typically getting unintelligible low-level errors as a result. We can provide a more on-point message by making this a regular runtime check. While at it, adjust the similar error check in systable_beginscan_ordered to use the same message text. That one is (probably) not reachable without a coding bug, but we might as well use a translatable message if we have one. Per bug #18363 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18363-e3598a5a572d0699@postgresql.org
* Improve documentation and GUC description for transaction_timeoutAlexander Korotkov2024-02-25
| | | | Reported-by: Alexander Lakhin
* Remove flaky isolation tests for timeoutsAlexander Korotkov2024-02-25
| | | | | | | | | | 51efe38cb92f introduced bunch of tests for idle_in_transaction_session_timeout, transaction_timeout and statement_timeout. These tests were too flaky on some slow buildfarm machines, so we plan to replace them with TAP tests using injection points. This commit removes flaky tests. Discussion: https://postgr.es/m/CAAhFRxiQsRs2Eq5kCo9nXE3HTugsAAJdSQSmxncivebAxdmBjQ%40mail.gmail.com Author: Andrey Borodin
* Multiple revisions to the GROUP BY reordering testsAlexander Korotkov2024-02-24
| | | | | | Discussion: https://postgr.es/m/CAMbWs4-NKLa%2BSs%2BX%3DWR6h0x%3DT07YBJoAz70ZGHzc-2zcHUHb0A%40mail.gmail.com Author: Richard Guo Reviewed-by: Andrei Lepikhov, Alexander Korotkov
* Replace lateral references to removed rels in subqueriesAlexander Korotkov2024-02-24
| | | | | | | | | | | | | | This commit introduces a new field 'sublevels_up' in ReplaceVarnoContext, and enhances replace_varno_walker() to: 1) recurse into subselects with sublevels_up increased, and 2) perform the replacement only when varlevelsup is equal to sublevels_up. This commit also fixes some outdated comments. And besides adding relevant test cases, it makes some unification over existing SJE test cases. Discussion: https://postgr.es/m/CAMbWs4-%3DPO6Mm9gNnySbx0VHyXjgnnYYwbN9dth%3DTLQweZ-M%2Bg%40mail.gmail.com Author: Richard Guo Reviewed-by: Andrei Lepikhov, Alexander Korotkov
* Avoid dangling-pointer problem with partitionwise joins under GEQO.Tom Lane2024-02-23
| | | | | | | | | | | | | | | | build_child_join_sjinfo creates a derived SpecialJoinInfo in the short-lived GEQO context, but afterwards the semi_rhs_exprs from that may be used in a UniquePath for a child base relation. This breaks the expectation that all base-relation-level structures are in the planning-lifespan context, leading to use of a dangling pointer with probable ensuing crash later on in create_unique_plan. To fix, copy the expression trees when making a UniquePath. Per bug #18360 from Alexander Lakhin. This has been broken since partitionwise joins were added, so back-patch to all supported branches. Discussion: https://postgr.es/m/18360-a23caf3157f34e62@postgresql.org
* Fix compiler warning on typedef redeclarationHeikki Linnakangas2024-02-23
| | | | | | | | | | | | | | bulk_write.c:78:3: error: redefinition of typedef 'BulkWriteState' is a C11 feature [-Werror,-Wtypedef-redefinition] } BulkWriteState; ^ ../../../../src/include/storage/bulk_write.h:20:31: note: previous definition is here typedef struct BulkWriteState BulkWriteState; ^ 1 error generated. Per buildfarm animals 'sifaka' and 'longfin'. Discussion: https://www.postgresql.org/message-id/9e1f63c3-ef16-404c-b3cb-859a96eaba39@iki.fi
* Introduce a new smgr bulk loading facility.Heikki Linnakangas2024-02-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The new facility makes it easier to optimize bulk loading, as the logic for buffering, WAL-logging, and syncing the relation only needs to be implemented once. It's also less error-prone: We have had a number of bugs in how a relation is fsync'd - or not - at the end of a bulk loading operation. By centralizing that logic to one place, we only need to write it correctly once. The new facility is faster for small relations: Instead of of calling smgrimmedsync(), we register the fsync to happen at next checkpoint, which avoids the fsync latency. That can make a big difference if you are e.g. restoring a schema-only dump with lots of relations. It is also slightly more efficient with large relations, as the WAL logging is performed multiple pages at a time. That avoids some WAL header overhead. The sorted GiST index build did that already, this moves the buffering to the new facility. The changes to pageinspect GiST test needs an explanation: Before this patch, the sorted GiST index build set the LSN on every page to the special GistBuildLSN value, not the LSN of the WAL record, even though they were WAL-logged. There was no particular need for it, it just happened naturally when we wrote out the pages before WAL-logging them. Now we WAL-log the pages first, like in B-tree build, so the pages are stamped with the record's real LSN. When the build is not WAL-logged, we still use GistBuildLSN. To make the test output predictable, use an unlogged index. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/30e8f366-58b3-b239-c521-422122dd5150%40iki.fi
* Fix mistake in SQL features listPeter Eisentraut2024-02-23
| | | | | | | | Fix for c9f57541d97: Feature F302-02 was renamed to F305, but that commit failed to delete the old line. Reported-by: Satoru Koizumi (小泉 悟) <koizumistr@minos.ocn.ne.jp> Discussion: https://www.postgresql.org/message-id/flat/170866661469.645.14101429540172934386%40wrigleys.postgresql.org
* Fix BF failure in commit 93db6cbda0.Amit Kapila2024-02-22
| | | | | | | | | The code to match the required LOG in the test was not robust enough to match it. It was using a very specific format to search the required message which doesn't work when one uses log_error_verbosity = verbose. Author: Hou Zhijie Discussion: https://postgr.es/m/CAA4eK1KcQSk7wzC7Zfrth9OhrjW2HvxL4tKgU42qqH7p6jn+FA@mail.gmail.com
* Make GetSlotInvalidationCause() return RS_INVAL_NONE on unexpected inputMichael Paquier2024-02-22
| | | | | | | | | | | | | 943f7ae1c869 has changed GetSlotInvalidationCause() so as it would return the last element of SlotInvalidationCauses[] when an incorrect conflict reason name is given by a caller, but this should return RS_INVAL_NONE in such cases, even if such a state should never be reached in practice. Per gripe from Peter Smith. Reviewed-by: Bharath Rupireddy Discussion: https://postgr.es/m/CAHut+PtsrSWxczpGkSaSVtJo+BXrvJ3Hwp5gES14bbL-G+HL7A@mail.gmail.com
* Add a new slot sync worker to synchronize logical slots.Amit Kapila2024-02-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | By enabling slot synchronization, all the failover logical replication slots on the primary (assuming configurations are appropriate) are automatically created on the physical standbys and are synced periodically. The slot sync worker on the standby server pings the primary server at regular intervals to get the necessary failover logical slots information and create/update the slots locally. The slots that no longer require synchronization are automatically dropped by the worker. The nap time of the worker is tuned according to the activity on the primary. The slot sync worker waits for some time before the next synchronization, with the duration varying based on whether any slots were updated during the last cycle. A new parameter sync_replication_slots enables or disables this new process. On promotion, the slot sync worker is shut down by the startup process to drop any temporary slots acquired by the slot sync worker and to prevent the worker from trying to fetch the failover slots. A functionality to allow logical walsenders to wait for the physical will be done in a subsequent commit. Author: Shveta Malik, Hou Zhijie based on design inputs by Masahiko Sawada and Amit Kapila Reviewed-by: Masahiko Sawada, Bertrand Drouvot, Peter Smith, Dilip Kumar, Ajin Cherian, Nisha Moond, Kuroda Hayato, Amit Kapila Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
* pgindent fixPeter Eisentraut2024-02-22
| | | | for commit 489072ab7a
* Fix the intermittent buildfarm failures in 031_column_list.Amit Kapila2024-02-22
| | | | | | | | | | | | | | | | | | | | | | | | The reason was that the ALTER SUBSCRIPTION .. SET PUBLICATION will lead to the restarting of apply worker and after the restart, the apply worker will use the existing slot and replication origin corresponding to the subscription. Now, it is possible that before restart the origin has not been updated and the WAL start location points to a location before where PUBLICATION exists which can lead to the error "publication ... does not exist". Fix it by recreating the subscription as a newly created subscription will start processing WAL from the recent WAL location and will see the required publication. This behavior has existed from the time logical replication was introduced but is exposed by this test and we have started a discussion for a better fix for this problem. As per Buildfarm Diagnosed-by: Amit Kapila Author: Vignesh C Discussion: https://postgr.es/m/3307255.1706911634@sss.pgh.pa.us
* Remove custom Constraint node read/write implementationsPeter Eisentraut2024-02-22
| | | | | | | | | | | | | | | | | | | | | | | | | This is part of an effort to reduce the number of special cases in the automatically generated node support functions. Allegedly, only certain fields of the Constraint node are valid based on contype. But this has historically not been kept up to date in the read/write functions. The Constraint node is only used for debugging DDL statements, so there are no strong requirements for its output, and there is no enforcement for its correctness. (There was no read support before a6bc3301925.) Commits e7a552f303c and abf46ad9c7b are examples of where omissions were fixed. This patch just removes the custom read/write implementations for the Constraint node type. Now we just output all the fields, which is a bit more than before, but at least we don't have to maintain these functions anymore. Also, we lose the string representation of the contype field, but for this marginal use case that seems tolerable. This patch also changes the documentation of the Constraint struct to put less emphasis on grouping fields by constraint type but rather document for each field how it's used. Reviewed-by: Paul Jungwirth <pj@illuminatedcomputing.com> Discussion: https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca645@eisentraut.org
* Improve ERROR/LOG messages added by commits ddd5f4f54a and 7a424ece48.Amit Kapila2024-02-22
| | | | | | | | | | | | | | Additionally, in slotsync.c, replace one StringInfoData variable usage with a constant string to avoid palloc/pfree. Also, replace the inclusion of "logical.h" with "slot.h" to prevent the exposure of unnecessary implementation details. Reported-by: Kyotaro Horiguchi, Masahiko Sawada Author: Shveta Malik based on suggestions by Robert Haas and Amit Kapila Reviewed-by: Kyotaro Horiguchi, Amit Kapila Discussion: https://postgr.es/m/20240214.162652.773291409747353211.horikyota.ntt@gmail.com Discussion: https://postgr.es/m/20240219.134015.1888940527023074780.horikyota.ntt@gmail.com Discussion: https://postgr.es/m/CAD21AoCYXhDYOQDAS-rhGasC2T+tYbV=8Y18o94sB=5AxcW+yA@mail.gmail.com
* Speed up uuid_out() by not relying on a StringInfoMichael Paquier2024-02-22
| | | | | | | | | | | | | | | | | Since the size of the string representation of an uuid is fixed, there is no benefit in using a StringInfo. This commit simplifies uuid_oud() to not rely on a StringInfo, where avoiding the overhead of the string manipulation makes the function substantially faster. A COPY TO on a relation with one UUID attribute can show up to a 40% speedup when the bottleneck is the COPY computation with uuid_out() showing up at the top of the profiles (numbered measure here, Laurenz has mentioned something closer to 20% faster runtimes), for example when the data is fully in shared buffers or the OS cache. Author: Laurenz Albe Reviewed-by: Andres Freund, Michael Paquier Description: https://postgr.es/m/679d5455cbbb0af667ccb753da51a475bae1eaed.camel@cybertec.at
* Add lookup table for replication slot conflict reasonsMichael Paquier2024-02-22
| | | | | | | | | | | | | | | This commit switches the handling of the conflict cause strings for replication slots to use a table rather than being explicitly listed, using a C99-designated initializer syntax for the array elements. This makes the whole more readable while easing future maintenance with less areas to update when adding a new conflict reason. This is similar to 74a730631065, but the scale of the change is smaller as there are less conflict causes than LWLock builtin tranche names. Author: Bharath Rupireddy Reviewed-by: Jelte Fennema-Nio Discussion: https://postgr.es/m/CALj2ACUxSLA91QGFrJsWNKs58KXb1C03mbuwKmzqqmoAKLwJaw@mail.gmail.com
* Remove superfluous 'pgprocno' field from PGPROCHeikki Linnakangas2024-02-22
| | | | | | | | | It was always just the index of the PGPROC entry from the beginning of the proc array. Introduce a macro to compute it from the pointer instead. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi
* MERGE ... DO NOTHING: require SELECT privilegesAlvaro Herrera2024-02-21
| | | | | | | | | | | | | | | | | Verify that a user running MERGE with a DO NOTHING clause has privileges to read the table, even if no columns are referenced. Such privileges were already required if the ON clause or any of the WHEN conditions referenced any column at all, so there's no functional change in practice. This change fixes an assertion failure in the case where no column is referenced by the command and the WHEN clauses are all DO NOTHING. Backpatch to 15, where MERGE was introduced. Reported-by: Alena Rybakina <a.rybakina@postgrespro.ru> Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/4d65a385-7efa-4436-a825-0869f89d9d92@postgrespro.ru
* Fix typoAlvaro Herrera2024-02-21
|
* Add option force_initdb to PostgreSQL::Test::Cluster:init()Michael Paquier2024-02-21
| | | | | | | | | | | | | | | | | This option is useful to bypass the default behavior of init() which would create the data folder of a new cluster by copying it from a template previously initdb'd, if any. Copying the data folder is much cheaper than running initdb, but some tests may want to force that. For example, one scenario of pg_combinebackup updated in this commit needs a different system ID for two nodes. Previously, this could only be achieved by unsetting $ENV{'INITDB_TEMPLATE'}, which could become a problem in complex node setups by making tests less efficient. Author: Amul Sul Reviewed-by: Robert Haas, Michael Paquier Discussion: https://postgr.es/m/Zc1tX9lLonLGu6oH@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
* Replace relids in lateral subquery parse tree during SJEAlexander Korotkov2024-02-20
| | | | | | Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/56ee4520-e9d1-d519-54fe-c8bff880ce9b%40gmail.com Author: Alexander Korotkov, Andrei Lepikhov