aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix handling of WAL segments ready to be archived during crash recoveryMichael Paquier2020-04-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 78ea8b5 has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Benoît Lobréau Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5
* Remove ACLDEBUG #define and associated code.Tom Lane2020-04-23
| | | | | | | | | | In the footsteps of aaf069aa3, remove ACLDEBUG, which was the only other remaining undocumented symbol in pg_config_manual.h. The fact that nobody had bothered to document it in seventeen years is a good clue to its usefulness. In practice, none of the tracing logic it enabled would be of any value without additional effort. Discussion: https://postgr.es/m/6631.1587565046@sss.pgh.pa.us
* Remove useless (and broken) logging logic in memory context functions.Tom Lane2020-04-23
| | | | | | | | | | | Nobody really uses this stuff, especially not since we created valgrind-based infrastructure that does the same thing better. It is thus unsurprising that the generation.c and slab.c versions were actually broken. Rather than fix 'em, let's just remove 'em. Alexander Lakhin Discussion: https://postgr.es/m/8936216c-3492-3f6e-634b-d638fddc5f91@gmail.com
* Also rename 'struct manifest_info'.Robert Haas2020-04-23
| | | | | The previous commit renamed the struct's typedef, but not the struct name itself.
* Rename exposed identifiers to say "backup manifest".Robert Haas2020-04-23
| | | | | | | | | | Function names declared "extern" now use BackupManifest in the name rather than just Manifest, and data types use backup_manifest rather than just manifest. Per note from Michael Paquier. Discussion: http://postgr.es/m/20200418125713.GG350229@paquier.xyz
* Fix transient memory leak for SRFs in FROM.Andres Freund2020-04-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | In a9c35cf85ca I changed ExecMakeTableFunctionResult() to dynamically allocate the FunctionCallInfo used to call the SRF. Unfortunately I did not account for the fact that the surrounding memory context has query lifetime, leading to a leak till the end of the query. In most cases the leak is fairly inconsequential, but if the FunctionScan is done many times in the query, the leak can add up. This happens e.g. if the function scan is on the inner side of a nested loop, due to a lateral join. EXPLAIN SELECT sum(f) FROM generate_series(1, 100000000) g(i), generate_series(i, i+1) f; quickly shows the leak. Instead of explicitly freeing the FunctionCallInfo it seems better to make sure all the per-set temporary state in ExecMakeTableFunctionResult() is cleaned up wholesale. Currently that's probably just the FunctionCallInfo allocation, but since there's some initialization work, and since there's already an appropriate context, this seems like a more robust approach. Bug: #16112 Reported-By: Ben Cornett Author: Andres Freund Reviewed-By: Tom Lane Discussion: https://postgr.es/m/16112-4448bbf55a404189%40postgresql.org Backpatch: 12, a9c35cf85ca
* Fix option related issues in pg_verifybackup.Fujii Masao2020-04-23
| | | | | | | | | | | | This commit does: - get rid of the garbage code for unused --print-parse-wal option. - add help message for --quiet option into usage(). - fix typo of option name in help message. Author: Fujii Masao Reviewed-by: Robert Haas Discussion: https://postgr.es/m/ff4710f7-2331-4f6b-012e-d76da3275e91@oss.nttdata.com
* nbtree: Rename BT_RESERVED_OFFSET_MASK.Peter Geoghegan2020-04-22
| | | | | | | | | | The mask was added by commit 8224de4f, which introduced INCLUDE nbtree indexes. The status bits really were reserved initially. We now use 2 out of 4 of the bits for additional tuple metadata, though. Rename the mask to BT_STATUS_OFFSET_MASK. Also consolidate related nbtree.h code comments about the format of pivot tuples and posting list tuples.
* Fix cost_incremental_sort for expressions with varno 0Tomas Vondra2020-04-23
| | | | | | | | | | | | | | When estimating the number of pre-sorted groups in cost_incremental_sort we must not pass Vars with varno 0 to estimate_num_groups, which would cause failues in find_base_rel. This may happen when sorting output of set operations, thanks to generate_append_tlist. Unlike recurse_set_operations we can't easily access the original target list, so if we find any Vars with varno 0, we fall back to the default estimate DEFAULT_NUM_DISTINCT. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20200411214639.GK2228%40telsasoft.com
* Remove bogus Assert in foreign key cloning codeDavid Rowley2020-04-22
| | | | | | | | | | | | This Assert was trying to ensure that the number of columns in the foreign key being cloned was the same number of attributes in the parentRel.  Of course, it's perfectly valid to have columns in the table which are not part of the foreign key constraint. It appears that this Assert was misunderstanding that. Reported-by: Rajkumar Raghuwanshi Reviewed-by: amul sul Discussion: https://postgr.es/m/CAKcux6=z1dtiWw5BOpqDx-U6KTiq+zD0Y2m810zUtWL+giVXWA@mail.gmail.com
* Remove HEAPDEBUGALLPeter Eisentraut2020-04-22
| | | | | | | | | This has been broken since PostgreSQL 12 and was probably never really used. PostgreSQL 12 added an analogous HEAPAMSLOTDEBUGALL, which still works right now, but it's also not very useful, so remove that as well. Discussion: https://www.postgresql.org/message-id/flat/645c0646-4218-d4c3-409a-a7003a0c108d%402ndquadrant.com
* Fix single-record reads to use restore_command if available in pg_rewindMichael Paquier2020-04-22
| | | | | | | | | | readOneRecord() is used now when looking for a checkpoint record to check if the target server is an ancestor of the source across multiple timelines, and using a restore_command if available improves the stability of the operation. This part was missed in a7e8ece. Reported-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/20200421.150830.1410714948345179794.horikyota.ntt@gmail.com
* psql \d: Display table where trigger is defined, if inheritedAlvaro Herrera2020-04-21
| | | | | | | | | | | | | | It's important to know that a trigger is cloned from a parent table, because of the behavior that the trigger is dropped on detach. Make psql's \d display it. We'd like to backpatch, but lack of the pg_trigger.tgparentid column makes it more difficult. Punt for now. If somebody wants to volunteer an implementation that reads pg_depend on older versions, that can probably be backpatched. Authors: Justin Pryzby, Amit Langote, Álvaro Herrera Discussion: https://postgr.es/m/20200419002206.GM26953@telsasoft.com
* Fix memory leak in libpq when using sslmode=verify-fullMichael Paquier2020-04-22
| | | | | | | | | | | | | Checking if Subject Alternative Names (SANs) from a certificate match with the hostname connected to leaked memory after each lookup done. This is broken since acd08d7 that added support for SANs in SSL certificates, so backpatch down to 9.5. Author: Roman Peshkurov Reviewed-by: Hamid Akhtar, Michael Paquier, David Steele Discussion: https://postgr.es/m/CALLDf-pZ-E3mjxd5=bnHsDu9zHEOnpgPgdnO84E2RuwMCjjyPw@mail.gmail.com Backpatch-through: 9.5
* Fix possible crash during FATAL exit from reindexing.Tom Lane2020-04-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | index.c supposed that it could just use a PG_TRY block to clean up the state associated with an active REINDEX operation. However, that code doesn't run if we do a FATAL exit --- for example, due to a SIGTERM shutdown signal --- while the REINDEX is happening. And that state does get consulted during catalog accesses, which makes it problematic if we do any catalog accesses during shutdown --- for example, to clean up any temp tables created in the session. If this combination of circumstances occurred, we could find ourselves trying to access already-freed memory. In debug builds that'd fairly reliably cause an assertion failure. In production we might often get away with it, but with some bad luck it could cause a core dump. Another possible bad outcome is an erroneous conclusion that an index-to-be-accessed is being reindexed; but it looks like that would be unlikely to have any consequences worse than failing to drop temp tables right away. (They'd still get dropped by the next session that uses that temp schema.) To fix, get rid of the use of PG_TRY here, and instead hook into the transaction abort mechanisms to clean up reindex state. Per bug #16378 from Alexander Lakhin. This has been wrong for a very long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/16378-7a70ca41b3ec2009@postgresql.org
* Fix minor violations of FunctionCallInvoke usage protocol.Tom Lane2020-04-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Working on commit 1c455078b led me to check through FunctionCallInvoke call sites to see if every one was being honest about (a) making sure that fcinfo.isnull is initially false, and (b) checking its state after the call. Sure enough, I found some violations. The main one is that finalize_partialaggregate re-used serialfn_fcinfo without resetting isnull, even though it clearly intends to cater for serialfns that return NULL. There would only be an issue with a non-strict serialfn, since it's unlikely that a serialfn would return NULL for non-null input. We have no non-strict serialfns in core, and there may be none in the wild either, which would account for the lack of complaints. Still, it's clearly wrong, so back-patch that fix to 9.6 where finalize_partialaggregate was introduced. Also, arrayfuncs.c and rowtypes.c contained various callers that were not bothering to check for result nulls. While what's being called is a comparison or hash function that probably *shouldn't* return null, that's a lousy excuse for not having any check at all. There are existing places that just Assert(!fcinfo->isnull) in comparable situations, so I added that to the places that were calling btree comparison or hash support functions. In the places calling boolean-returning equality functions, it's quite cheap to have them treat isnull as FALSE, so make those places do that. Also remove some "locfcinfo->isnull = false" assignments that are unnecessary given the assumption that no previous call returned null. These changes seem like mostly neatnik-ism or debugging support, so I didn't back-patch.
* Fix detaching partitions with cloned row triggersAlvaro Herrera2020-04-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a partition is detached, any triggers that had been cloned from its parent were not properly disentangled from its parent triggers. This resulted in triggers that could not be dropped because they depended on the trigger in the trigger in the no-longer-parent table: ALTER TABLE t DETACH PARTITION t1; DROP TRIGGER trig ON t1; ERROR: cannot drop trigger trig on table t1 because trigger trig on table t requires it HINT: You can drop trigger trig on table t instead. Moreover the table can no longer be re-attached to its parent, because the trigger name is already taken: ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2); ERROR: trigger "trig" for relation "t1" already exists The former is a bug introduced in commit 86f575948c77. (The latter is not necessarily a bug, but it makes the bug more uncomfortable.) To avoid the complexity that would be needed to tell whether the trigger has a local definition that has to be merged with the one coming from the parent table, establish the behavior that the trigger is removed when the table is detached. Backpatch to pg11. Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/20200408152412.GZ2228@telsasoft.com
* Consider outliers in split interval calculation.Peter Geoghegan2020-04-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 0d861bbb, which introduced deduplication to nbtree, added some logic to take large posting list tuples into account when choosing a split point. We subtract firstright posting list overhead from the projected new high key size when calculating leftfree/rightfree values for an affected candidate split point. Posting list tuples aren't special to nbtsplitloc.c, but taking them into account like this makes a huge difference in practice. Posting list tuples are frequently tuple size outliers. However, commit 0d861bbb missed a closely related issue: split interval itself is calculated based on the assumption that tuples on the page being split are roughly equisized. That assumption was acceptable back when commit fab25024 taught the logic for choosing a split point about suffix truncation, but it's pretty questionable now that very large tuple sizes are common. This oversight led to unbalanced page splits in low cardinality multi-column indexes when deduplication was used: page splits that don't give sufficient weight to how unbalanced the split is when the interval happens to include some large posting list tuples (and when most other tuples on the page are not so large). Nail this down by calculating an initial split interval in a way that's attuned to the actual cost that we want to keep under control (not a fuzzy proxy for the cost): apply a leftfree + rightfree evenness test to each candidate split point that actually gets included in the split interval (for the default strategy). This replaces logic that used a percentage of all legal split points for the page as the basis of the initial split interval. Discussion: https://postgr.es/m/CAH2-WznJt5aT2uUB2Bs+JBLdwe0XTX67+xeLFcaNvCKxO=QBVQ@mail.gmail.com
* Allow matchingsel() to be used with operators that might return NULL.Tom Lane2020-04-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Although selfuncs.c will never call a target operator with null inputs, some functions might return null anyway. The existing coding will fail if that happens (since FunctionCall2Coll will punt), which seems undesirable given that matchingsel() has such a broad range of potential applicability --- in fact, we already have a problem because we apply it to jsonb_path_exists_opr, which can return null. Hence, rejigger the underlying functions mcv_selectivity and histogram_selectivity to cope, treating a null result as false. While we are at it, we can move the InitFunctionCallInfoData overhead out of the inner loops, which isn't a huge number of cycles but might save something considering we are likely calling functions as cheap as int4eq(). Plus, the number of loop cycles to be expected is much more than it was when this code was written, since typical settings of default_statistics_target are higher. In view of that consideration, let's apply the same change to var_eq_const, eqjoinsel_inner, and eqjoinsel_semi. We do not expect equality functions to ever return null for non-null inputs (and certainly that code has been that way a long time without complaints), but the cycle savings seem attractive, especially in the eqjoinsel loops where there's potentially an O(N^2) savings. Similar code exists in ineq_histogram_selectivity and get_variable_range, but I forebore from changing those for now. The performance argument for changing ineq_histogram_selectivity is really weak anyway, since that will only iterate log2(N) times. Nikita Glukhov and Tom Lane Discussion: https://postgr.es/m/9d3b0959-95d6-c37e-2c0b-287bcfe5c705@postgrespro.ru
* Clean up cpluspluscheck violation.Tom Lane2020-04-21
| | | | | | | "operator" is a reserved word in C++, so per project conventions, don't use it as an identifier in header files. My oversight in commit a80818605.
* Fix duplicate typedef from commit 0d8c9c121.Tom Lane2020-04-21
| | | | | | | | | | | | Older gcc versions don't like duplicate typedefs, so get rid of that in favor of doing it like we do it elsewhere, ie just use a "struct" declaration when trying to avoid importing a whole header file. Also, there seems no reason to include stringinfo.h here at all, so get rid of that addition too. Discussion: https://postgr.es/m/27239.1587415696@sss.pgh.pa.us
* Move the server's backup manifest code to a separate file.Robert Haas2020-04-20
| | | | | | | | basebackup.c is already a pretty big and complicated file, so it makes more sense to keep the backup manifest support routines in a separate file, for clarity and ease of maintenance. Discussion: http://postgr.es/m/CA+TgmoavRak5OdP76P8eJExDYhPEKWjMb0sxW7dF01dWFgE=uA@mail.gmail.com
* Add tab-completion for ALTER INDEX .. [NO] DEPENDS ONAlvaro Herrera2020-04-20
| | | | | | | | | | | ... as added in the prior commit. (We'd like to have tab-completion for the other object types too, but they don't have sub-command completion yet.) Author: Ibrar Ahmed <ibrar.ahmad@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CALtqXTcogrFEVP9uou5vFtnGsn+vHZUu9+9a0inarfYVOHScYQ@mail.gmail.com
* Add ALTER .. NO DEPENDS ONAlvaro Herrera2020-04-20
| | | | | | | | | | | | | | Commit f2fcad27d59c (9.6 era) added the ability to mark objects as dependent an extension, but forgot to add a way for such dependencies to be removed. This commit fixes that oversight. Strictly speaking this should be backpatched to 9.6, but due to lack of demand we're not doing so at this time. Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql Reviewed-by: ahsan hadi <ahsan.hadi@gmail.com> Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
* Allow pg_read_all_stats to access all stats views againMagnus Hagander2020-04-20
| | | | | | | | | | | | The views pg_stat_progress_* had not gotten the memo that pg_read_all_stats is supposed to be able to read all statistics. Also make a pass over all text-returning pg_stat_xyz functions that could return "insufficient privilege" and make sure they also respect pg_read_all_status. Reported-by: Andrey M. Borodin Reviewed-by: Andrey M. Borodin, Kyotaro Horiguchi Discussion: https://postgr.es/m/13145F2F-8458-4977-9D2D-7B2E862E5722@yandex-team.ru
* Fix missing pfree() in logtape.c, missed by 24d85952.Jeff Davis2020-04-19
|
* Fix update-unicode targetPeter Eisentraut2020-04-19
| | | | | | | | The normalization-check target needs to be run last, after moving the newly generated files into place. Also, we need an additional dependency so that unicode_norm.o is rebuilt first. Otherwise, norm_test will still test the old files but against the new expected results, which will probably fail.
* Fix race conditions in synchronous standby management.Tom Lane2020-04-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have repeatedly seen the buildfarm reach the Assert(false) in SyncRepGetSyncStandbysPriority. This apparently is due to failing to consider the possibility that the sync_standby_priority values in shared memory might be inconsistent; but they will be whenever only some of the walsenders have updated their values after a change in the synchronous_standby_names setting. That function is vastly too complex for what it does, anyway, so rewriting it seems better than trying to apply a band-aid fix. Furthermore, the API of SyncRepGetSyncStandbys is broken by design: it returns a list of WalSnd array indexes, but there is nothing guaranteeing that the contents of the WalSnd array remain stable. Thus, if some walsender exits and then a new walsender process takes over that WalSnd array slot, a caller might make use of WAL position data that it should not, potentially leading to incorrect decisions about whether to release transactions that are waiting for synchronous commit. To fix, replace SyncRepGetSyncStandbys with a new function SyncRepGetCandidateStandbys that copies all the required data from shared memory while holding the relevant mutexes. If the associated walsender process then exits, this data is still safe to make release decisions with, since we know that that much WAL *was* sent to a valid standby server. This incidentally means that we no longer need to treat sync_standby_priority as protected by the SyncRepLock rather than the per-walsender mutex. SyncRepGetSyncStandbys is no longer used by the core code, so remove it entirely in HEAD. However, it seems possible that external code is relying on that function, so do not remove it from the back branches. Instead, just remove the known-incorrect Assert. When the bug occurs, the function will return a too-short list, which callers should treat as meaning there are not enough sync standbys, which seems like a reasonably safe fallback until the inconsistent state is resolved. Moreover it's bug-compatible with what has been happening in non-assert builds. We cannot do anything about the walsender-replacement race condition without an API/ABI break. The bogus assertion exists back to 9.6, but 9.6 is sufficiently different from the later branches that the patch doesn't apply at all. I chose to just remove the bogus assertion in 9.6, feeling that the probability of a bad outcome from the walsender-replacement race condition is too low to justify rewriting the whole patch for 9.6. Discussion: https://postgr.es/m/21519.1585272409@sss.pgh.pa.us
* Fix possible crash with GENERATED ALWAYS columnsDavid Rowley2020-04-18
| | | | | | | | | | | | | | | | | | In some corner cases, this could also lead to corrupted values being included in the tuple. Users who are concerned that they are affected by this should first upgrade and then perform a base backup of their database and restore onto an off-line server. They should then query each table with generated columns to ensure there are no rows where the generated expression does not match a newly calculated version of the GENERATED ALWAYS expression. If no crashes occur and no rows are returned then you're not affected. Fixes bug #16369. Reported-by: Cameron Ezell Discussion: https://postgr.es/m/16369-5845a6f1bef59884@postgresql.org Backpatch-through: 12 (where GENERATED ALWAYS columns were added.)
* Revert "Only provide new libpq sslpasskey hook for openssl-enabled builds"Andrew Dunstan2020-04-17
| | | | | | | This reverts commit 9e24109f1a4e4d8d1d372b004d6a0dd06e673fe7. This caused build errors when building without openssl, and it's simplest just to revert it.
* Only provide openssl_tls_init_hook if building with opensslAndrew Dunstan2020-04-17
| | | | | | | This should have been protected by #ifdef USE_OPENSSL in commit 896fcdb230. Per the real complaint this time from Daniel Gustafsson.
* Use a slightly more liberal regex to detect Visual Studio versionAndrew Dunstan2020-04-17
| | | | | | | | | | | Apparently in some language versions of Visual Studio nmake outputs some material after the version number and before the end of the line. This has been seen in Chinese versions. Therefore, we no longer demand that the version string comes at the end of a line. Per complaint from Cuiping Lin. Backpatch to all live branches.
* Only provide new libpq sslpasskey hook for openssl-enabled buildsAndrew Dunstan2020-04-17
| | | | | | | In commit 4dc6355210 I neglected to put #ifdef USE_OPENSSL around the declarations of the new items. This is remedied here. Per complaint from Daniel Gustafsson.
* Fix possible future cache reference leak in ALTER EXTENSION ADD/DROP.Tom Lane2020-04-17
| | | | | | | | | | | | | | | | | recordExtObjInitPriv and removeExtObjInitPriv were sloppy about calling ReleaseSysCache. The cases cannot occur given current usage in ALTER EXTENSION ADD/DROP, since we wouldn't get here for these relkinds; but it seems wise to clean up better. In passing, extend test logic in test_pg_dump to exercise the dropped-column code paths here. Since the case is unreachable at present, there seems no great need to back-patch; hence fix HEAD only. Kyotaro Horiguchi, with test case and comment adjustments by me Discussion: https://postgr.es/m/20200417.151831.1153577605111650154.horikyota.ntt@gmail.com
* Fix minor memory leak in pg_basebackup and pg_receivewalMichael Paquier2020-04-17
| | | | | | | | | | | | | | | The result of the query used to retrieve the WAL segment size from the backend was not getting freed in two code paths. Both pg_basebackup and pg_receivewal exit immediately if a failure happened on this query, so this was not an actual problem, but it could be an issue if this code gets used for other tools in different ways, be they future tools in this code tree or external, existing, ones. Oversight in commit fc49e24, so backpatch down to 11. Author: Jie Zhang Discussion: https://postgr.es/m/970ad9508461469b9450b64027842331@G08CNEXMBPEKD06.g08.fujitsu.local Backpatch-through: 11
* Remove unneeded constraint dependency trackingDavid Rowley2020-04-17
| | | | | | | | | | | | | | | | | | | | | | | | | | It was previously thought that remove_useless_groupby_columns() needed to keep track of which constraints the generated plan depended upon, however, this is unnecessary. The confusion likely arose regarding this because of check_functional_grouping(), which does need to track the dependency to ensure VIEWs with columns which are functionally dependant on the GROUP BY remain so. For remove_useless_groupby_columns(), cached plans will just become invalidated when the primary key's underlying index is removed through the normal relcache invalidation code. Here we just remove the unneeded code which records the dependency and updates the comments. The previous comments claimed that we could not use UNIQUE constraints for the same optimization due to lack of a pg_constraint record for NOT NULL constraints (which are required because NULLs can be duplicated in a unique index). Since we don't actually need a pg_constraint record to handle the invalidation, it looks like we could add code to do this in the future. But not today. We're not really fixing any bug in the code here, this fix is just to set the record straight on UNIQUE constraints. This code was added back in 9.6, but due to lack of any bug, we'll not be backpatching this. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAApHDvrdYa=VhOoMe4ZZjZ-G4ALnD-xuAeUNCRTL+PYMVN8OnQ@mail.gmail.com
* Fix the usage of parallel and full options of vacuum command.Amit Kapila2020-04-16
| | | | | | | | | | | | | | | Earlier we were inconsistent in allowing the usage of parallel and full options. Change it such that we disallow them only when they are combined in a way that we don't support. In passing, improve the comments in some of the existing tests of parallel vacuum. Reported-by: Tushar Ahuja Author: Justin Pryzby, Amit Kapila Reviewed-by: Sawada Masahiko, Michael Paquier, Mahendra Singh Thalor and Amit Kapila Discussion: https://postgr.es/m/58c8d171-e665-6fa3-a9d3-d9423b694dae%40enterprisedb.com
* Disable silently generation of manifests with servers <= 12 in pg_basebackupMichael Paquier2020-04-16
| | | | | | | | | | | | | | | | | | | | | | | Since 0d8c9c1, pg_basebackup would generate an error if connected to a backend version older than 12 where backup manifests are not supported. Avoiding this error is possible by using the --no-manifest option. This error handling could be confusing for some users, where patching a backup script that interacts with multiple backend versions would cause the addition of --no-manifest to potentially not generate a backup manifest even for Postgres 13 and newer versions. As we want to encourage the use of backup manifests as much as possible, this commit silently disables manifests where not supported, instead of generating an error. While on it, rework a bit the code to make it more consistent with the surroundings when generating the BASE_BACKUP command. Per discussion with Andres Freund, Stephen Frost, Robert Haas, Álvaro Herrera, Kyotaro Horiguchi, Tom Lane, David Steele, and me. Author: Michael Paquier Discussion: https://postgr.es/m/20200410080910.GZ1606@paquier.xyz
* Slightly simplify nbtree split point choice loop.Peter Geoghegan2020-04-15
| | | | | Spotted during post-commit review of the nbtree deduplication commit (commit 0d861bbb).
* Fix minor memory leak in pg_dumpMichael Paquier2020-04-15
| | | | | | | | | | | | A query used to read default ACL information from the catalogs did not free a set of PQExpBuffer. Oversight in commit e2090d9, so backpatch down to 9.6. Author: Jie Zhang Reviewed-by: Sawada Masahiko Discussion: https://postgr.es/m/05bcbc5857f948efa0b451b85a48ae10@G08CNEXMBPEKD06.g08.fujitsu.local Backpatch-through: 9.6
* Code review for backup manifest.Fujii Masao2020-04-15
| | | | | | | | | | | | | | | This commit prevents pg_basebackup from receiving backup_manifest file when --no-manifest is specified. Previously, when pg_basebackup was writing a tarfile to stdout, it tried to receive backup_manifest file even when --no-manifest was specified, and reported an error. Also remove unused -m option from pg_basebackup. Also fix typo in BASE_BACKUP command documentation. Author: Fujii Masao Reviewed-by: Michael Paquier, Robert Haas Discussion: https://postgr.es/m/01e3ed3a-8729-5aaa-ca84-e60e3ca59db8@oss.nttdata.com
* Remove obsolete "hole in center of page" comment.Peter Geoghegan2020-04-14
| | | | | | | | | A comment from the Berkeley days incorrectly claimed that the page management code cares about the contents of the hole in the center of the page (at least in the case of the left half of an nbtree page split). Commit 8fa30f906be added an addendum that stated that the original comment was "probably obsolete". It's definitely obsolete, though, so remove the original comment plus the addendum.
* Account for collation when coercing the output of a SQL function.Tom Lane2020-04-14
| | | | | | | Commit 913bbd88d overlooked that the result of coerce_to_target_type might need collation fixups. Per report from Andreas Joseph Krogh. Discussion: https://postgr.es/m/VisenaEmail.72.37d08ec2b8cb8fb5.17179940cd3@tc7-visena
* Stop requiring an explicit return from perl subroutinesAndrew Dunstan2020-04-14
| | | | | | | The consensus of the project appears to be that this provides little benefit and is simply an annoyance. Discussion: https://postgr.es/m/27481.1586618092@sss.pgh.pa.us
* Set Perl search path more idiomaticallyAndrew Dunstan2020-04-14
| | | | | | | Back in commits 1df92eeafe, f884a96819, and 592123efbb I used some hackish code to set the script search path, unaware despite decades of perl that there was a completely standard way to do this. This patch changes those cases to use the standard perl FindBin package.
* Rearrange _bt_insertonpg() "update metapage" code.Peter Geoghegan2020-04-14
| | | | | | Nest the "update metapage as part of insert into root-like page" branch inside the broader "insert into internal page" branch. This improves readability.
* Fix collection of typos and grammar mistakes in the tree, volume 2Michael Paquier2020-04-14
| | | | | | | | This fixes some comments and documentation new as of Postgres 13, and is a follow-up of the work done in dd0f37e. Author: Justin Pryzby Discussion: https://postgr.es/m/20200408165653.GF2228@telsasoft.com
* Add defensive "split_only_page" nbtree assertion.Peter Geoghegan2020-04-13
| | | | | | | | | | | | | | Clearly it's not okay for nbtree to split a page that is the only page on its level, and then find that it has to split the parent one level up in turn. There is simply no code to handle the split_only_page case in the _bt_insertonpg() "newitem won't fit" branch (only the "newitem fits" branch handles split_only_page). Add a defensive assertion that will fail if a split_only_page call to _bt_insertonpg() somehow ends up splitting the target/parent page. I (pgeoghegan) believe that we don't need split_only_page handling for the "newitem won't fit" branch because anybody calling _bt_insertonpg() like this would have to hold a lock on the same one and only child page.
* Comments and doc fixes for commit 40d964ec99.Amit Kapila2020-04-14
| | | | | | | Reported-by: Justin Pryzby Author: Justin Pryzby, with few changes by me Reviewed-by: Amit Kapila and Sawada Masahiko Discussion: https://postgr.es/m/20200322021801.GB2563@telsasoft.com
* Make _bt_insertonpg() more like _bt_split().Peter Geoghegan2020-04-13
| | | | | | | | | | It seems like a good idea for nbtree's retail insert code to be absolutely consistent with nbtree's page split code for anything that naturally requires equivalent handling. Anything that concerns inserting newitem (which is handled as part of the page split atomic action when a page split is required) should work in exactly the same way. With that in mind, make _bt_insertonpg() handle 'cbuf' in a way that matches _bt_split().