aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Avoid failure if autovacuum tries to access a just-dropped temp namespace.Tom Lane2020-02-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Such an access became possible when commit 246a6c8f7 added more aggressive cleanup of orphaned temp relations by autovacuum. Since autovacuum's snapshot might be slightly stale, it could attempt to access an already-dropped temp namespace, resulting in an assertion failure or null-pointer dereference. (In practice, since we don't drop temp namespaces automatically but merely recycle them, this situation could only arise if a superuser does a manual drop of a temp namespace. Still, that should be allowed.) The core of the bug, IMO, is that isTempNamespaceInUse and its callers failed to think hard about whether to treat "temp namespace isn't there" differently from "temp namespace isn't in use". In hopes of forestalling future mistakes of the same ilk, replace that function with a new one checkTempNamespaceStatus, which makes the same tests but returns a three-way enum rather than just a bool. isTempNamespaceInUse is gone entirely in HEAD; but just in case some external code is relying on it, keep it in the back branches, as a bug-compatible wrapper around the new function. Per report originally from Prabhat Kumar Sahu, investigated by Mahendra Singh and Michael Paquier; the final form of the patch is my fault. This replaces the failed fix attempt in a052f6cbb. Backpatch as far as v11, as 246a6c8f7 was. Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com
* Remove TAP test for createdb --lc-ctypeMichael Paquier2020-02-27
| | | | | | | | | | | OpenBSD falls back to "C" when using an incorrect input with setlocale() and LC_CTYPE, causing this test, introduced by 008cf04, to fail. This removes the culprit test to avoid the portability issue. Per report from Robert Haas, via buildfarm member curculio. Discussion: https://postgr.es/m/CA+TgmoZ6ddh3mHD9gU8DvNYoFmuJaYYn1+4AvZNp25vTdRwCAQ@mail.gmail.com Backpatch-through: 11
* Skip foreign tablespaces when running pg_checksums/pg_verify_checksumsMichael Paquier2020-02-27
| | | | | | | | | | | | | | | | | | | Attempting to use pg_checksums (pg_verify_checksums in 11) on a data folder which includes tablespace paths used across multiple major versions would cause pg_checksums to scan all directories present in pg_tblspc, and not only marked with TABLESPACE_VERSION_DIRECTORY. This could lead to failures when for example running sanity checks on an upgraded instance with --check. Even worse, it was possible to rewrite on-disk pages with --enable for a cluster potentially online. This commit makes pg_checksums skip any directories not named TABLESPACE_VERSION_DIRECTORY, similarly to what is done for base backups. Reported-by: Michael Banck Author: Michael Banck, Bernd Helmle Discussion: https://postgr.es/m/62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de backpatch-through: 11
* createdb: Fix quoting of --encoding, --lc-ctype and --lc-collateMichael Paquier2020-02-27
| | | | | | | | | | | | The original coding failed to properly quote those arguments, leading to failures when using quotes in the values used. As the quoting can be encoding-sensitive, the connection to the backend needs to be taken before applying the correct quoting. Author: Michael Paquier Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/20200214041004.GB1998@paquier.xyz Backpatch-through: 9.5
* Suppress unnecessary RelabelType nodes in more cases.Tom Lane2020-02-26
| | | | | | | | | | | | | | | | | | | | | | | | | | eval_const_expressions sometimes produced RelabelType nodes that were useless because they just relabeled an expression to the same exposed type it already had. This is worth avoiding because it can cause two equivalent expressions to not be equal(), preventing recognition of useful optimizations. In the test case added here, an unpatched planner fails to notice that the "sqli = constant" clause renders a sort step unnecessary, because one code path produces an extra RelabelType and another doesn't. Fix by ensuring that eval_const_expressions_mutator's T_RelabelType case will not add in an unnecessary RelabelType. Also save some code by sharing a subroutine with the effectively-equivalent cases for CollateExpr and CoerceToDomain. (CollateExpr had no bug, and I think that the case couldn't arise with CoerceToDomain, but it seems prudent to do the same check for all three cases.) Back-patch to v12. In principle this has been wrong all along, but I haven't seen a case where it causes visible misbehavior before v12, so refrain from changing stable branches unnecessarily. Per investigation of a report from Eric Gillum. Discussion: https://postgr.es/m/CAMmjdmvAZsUEskHYj=KT9sTukVVCiCSoe_PBKOXsncFeAUDPCQ@mail.gmail.com
* Add prefix checks in exclude lists for pg_rewind, pg_checksums and base backupsMichael Paquier2020-02-24
| | | | | | | | | | | | | | | | | | An instance of PostgreSQL crashing with a bad timing could leave behind temporary pg_internal.init files, potentially causing failures when verifying checksums. As the same exclusion lists are used between pg_rewind, pg_checksums and basebackup.c, all those tools are extended with prefix checks to keep everything in sync, with dedicated checks added for pg_internal.init. Backpatch down to 11, where pg_checksums (pg_verify_checksums in 11) and checksum verification for base backups have been introduced. Reported-by: Michael Banck Author: Michael Paquier Reviewed-by: Kyotaro Horiguchi, David Steele Discussion: https://postgr.es/m/62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de Backpatch-through: 11
* Simplify FK-to-partitioned regression test queryAlvaro Herrera2020-02-20
| | | | | | | | | | Avoid a join between relations having the FK to detect FK violation. The planner might optimize this considering the PK must exist on the referenced side at some point, effectively masking a bug this test tries to detect. Tom Lane and Jehan-Guillaume de Rorthais Discussion: https://postgr.es/m/467.1581270529@sss.pgh.pa.us
* Remove extra word from comment.Etsuro Fujita2020-02-20
|
* Fix typoPeter Eisentraut2020-02-19
| | | | Reported-by: Daniel Verite <daniel@manitou-mail.org>
* Fix confusion about event trigger vs. plain function in plpgsql.Tom Lane2020-02-19
| | | | | | | | | | | | | | | | | | | | | | | | | The function hash table keys made by compute_function_hashkey() failed to distinguish event-trigger call context from regular call context. This meant that once we'd successfully made a hash entry for an event trigger (either by validation, or by normal use as an event trigger), an attempt to call the trigger function as a plain function would find this hash entry and thereby bypass the you-can't-do-that check in do_compile(). Thus we'd attempt to execute the function, leading to strange errors or even crashes, depending on function contents and server version. To fix, add an isEventTrigger field to PLpgSQL_func_hashkey, paralleling the longstanding infrastructure for regular triggers. This fits into what had been pad space, so there's no risk of an ABI break, even assuming that any third-party code is looking at these hash keys. (I considered replacing isTrigger with a PLpgSQL_trigtype enum field, but felt that that carried some API/ABI risk. Maybe we should change it in HEAD though.) Per bug #16266 from Alexander Lakhin. This has been broken since event triggers were invented, so back-patch to all supported branches. Discussion: https://postgr.es/m/16266-fcd7f838e97ba5d4@postgresql.org
* Fix mesurement of elapsed time during truncating heap in VACUUM.Fujii Masao2020-02-19
| | | | | | | | | | | | | | | | | | | | | | | | | | VACUUM may truncate heap in several batches. The activity report is logged for each batch, and contains the number of pages in the table before and after the truncation, and also the elapsed time during the truncation. Previously the elapsed time reported in each batch was the total elapsed time since starting the truncation until finishing each batch. For example, if the truncation was processed dividing into three batches, the second batch reported the accumulated time elapsed during both first and second batches. This is strange and confusing because the number of pages in the table reported together is not total. Instead, each batch should report the time elapsed during only that batch. The cause of this issue was that the resource usage snapshot was initialized only at the beginning of the truncation and was never reset later. This commit fixes the issue by changing VACUUM so that the resource usage snapshot is reset at each batch. Back-patch to all supported branches. Reported-by: Tatsuhito Kasahara Author: Tatsuhito Kasahara Reviewed-by: Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/CAP0=ZVJsf=NvQuy+QXQZ7B=ZVLoDV_JzsVC1FRsF1G18i3zMGg@mail.gmail.com
* Stop demanding that top xact must be seen before subxact in decoding.Amit Kapila2020-02-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | Manifested as ERROR: subtransaction logged without previous top-level txn record this check forbids legit behaviours like - First xl_xact_assignment record is beyond reading, i.e. earlier restart_lsn. - After restart_lsn there is some change of a subxact. - After that, there is second xl_xact_assignment (for another subxact) revealing the relationship between top and first subxact. Such a transaction won't be streamed anyway because we hadn't seen it in full. Saying for sure whether xact of some record encountered after the snapshot was deserialized can be streamed or not requires to know whether it wrote something before deserialization point --if yes, it hasn't been seen in full and can't be decoded. Snapshot doesn't have such info, so there is no easy way to relax the check. Reported-by: Hsu, John Diagnosed-by: Arseny Sher Author: Arseny Sher, Amit Kapila Reviewed-by: Amit Kapila, Dilip Kumar Backpatch-through: 9.5 Discussion: https://postgr.es/m/AB5978B2-1772-4FEE-A245-74C91704ECB0@amazon.com
* Teach pg_dump to dump comments on RLS policy objects.Tom Lane2020-02-17
| | | | | | | | | | This was unaccountably omitted in the original RLS patch. The SQL syntax is basically the same as for comments on triggers, so crib code from dumpTrigger(). Per report from Marc Munro. Back-patch to all supported branches. Discussion: https://postgr.es/m/1581889298.18009.15.camel@bloodnok.com
* Fill in extraUpdatedCols in logical replicationPeter Eisentraut2020-02-17
| | | | | | | | | | | | | | The extraUpdatedCols field of the target RTE records which generated columns are affected by an update. This is used in a variety of places, including per-column triggers and foreign data wrappers. When an update was initiated by a logical replication subscription, this field was not filled in, so such an update would not affect generated columns in a way that is consistent with normal updates. To fix, factor out some code from analyze.c to fill in extraUpdatedCols in the logical replication worker as well. Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/b05e781a-fa16-6b52-6738-761181204567@2ndquadrant.com
* Add description about GSSOpenServer wait event into document.Fujii Masao2020-02-17
| | | | | | | | | | | | | This commit also updates wait event enum into alphabetical order. Previously the enum entry for GSSOpenServer was added out-of-order. Back-patch to v12 where commit b0b39f72b9 introduced GSSOpenServer wait event. In v12, the commit doesn't include the update of wait event enum, not to break ABI. Author: Fujii Masao Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/949931aa-4ed4-d867-a7b5-de9c02b2292b@oss.nttdata.com
* Try again to work around Windows' ERROR_SHARING_VIOLATION in pg_ctl.Tom Lane2020-02-16
| | | | | | | | | | | | | | | | | | | | | | | | | Commit 0da33c762 introduced an unfortunate regression in pg_ctl on Windows: if the log file specified with -l doesn't exist yet, and pg_ctl is running with Administrator privileges, then the log file might get created with permissions that prevent the postmaster from writing on it. (It seems that whether this happens depends on whether the log file is inside the user's home directory or not, and perhaps on other phase-of-the-moon conditions, which may explain why we failed to notice it sooner.) To fix, just don't create the log file if it doesn't exist yet. The case where we need to wait obviously only occurs with a pre-existing log file. In passing, switch from using fopen() to plain open(), saving a few cycles. Per bug #16259 from Jonathan Katz and Heath Lord. Back-patch to v12, as the faulty commit was. Alexander Lakhin Discussion: https://postgr.es/m/16259-c5ebed32a262a8b1@postgresql.org
* Avoid a performance regression in float overflow/underflow detection.Tom Lane2020-02-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 6bf0bc842 replaced float.c's CHECKFLOATVAL() macro with static inline subroutines, but that wasn't too well thought out. In the original coding, the unlikely condition (isinf(result) or result == 0) was checked first, and the inf_is_valid or zero_is_valid condition only afterwards. The inline-subroutine coding caused that to be swapped around, which is pretty horrid for performance because (a) in common cases the is_valid condition is twice as expensive to evaluate (e.g., requiring two isinf() calls not one) and (b) in common cases the is_valid condition is false, requiring us to perform the unlikely-condition check anyway. Net result is that one isinf() call becomes two or three, resulting in visible performance loss as reported by Keisuke Kuroda. The original fix proposal was to revert the replacement of the macro, but on second thought, that macro was just a bad idea from the beginning: if anything it's a net negative for readability of the code. So instead, let's just open-code all the overflow/underflow tests, being careful to test the unlikely condition first (and mark it unlikely() to help the compiler get the point). Also, rather than having N copies of the actual ereport() calls, collapse those into out-of-line error subroutines to save some code space. This does mean that the error file/line numbers won't be very helpful for figuring out where the issue really is --- but we'd already burned that bridge by putting the ereports into static inlines. In HEAD, check_float[48]_val() are gone altogether. In v12, leave them present in float.h but unused in the core code, just in case some extension is depending on them. Emre Hasegeli, with some kibitzing from me and Andres Freund Discussion: https://postgr.es/m/CANDwggLe1Gc1OrRqvPfGE=kM9K0FSfia0hbeFCEmwabhLz95AA@mail.gmail.com
* Document the pg_upgrade -j/--jobs option as taking an argumentPeter Eisentraut2020-02-11
|
* Stamp 12.2.REL_12_2Tom Lane2020-02-10
|
* createuser: fix parsing of --connection-limit argumentAlvaro Herrera2020-02-10
| | | | | | | The original coding failed to quote the argument properly. Reported-by: Daniel Gustafsson Discussion: 1B8AE66C-85AB-4728-9BB4-612E8E61C219@yesql.se
* Fix priv checks for ALTER <object> DEPENDS ON EXTENSIONAlvaro Herrera2020-02-10
| | | | | | | | | | | | | | Marking an object as dependant on an extension did not have any privilege check whatsoever; this allowed any user to mark objects as droppable by anyone able to DROP EXTENSION, which could be used to cause system-wide havoc. Disallow by checking that the calling user owns the mentioned object. (No constraints are placed on the extension.) Security: CVE-2020-1720 Reported-by: Tom Lane Discussion: 31605.1566429043@sss.pgh.pa.us
* Translation updatesPeter Eisentraut2020-02-10
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: bcdfb83b81a7aa3c3948c0a5221f9c68d7010ac5
* Revert "pg_upgrade: Fix quoting of some arguments in pg_ctl command"Michael Paquier2020-02-10
| | | | | | | This reverts commit d1c0b61. The patch has some downsides that require more attention, as discussed with Noah Misch. Backpatch-through: 9.5
* pg_upgrade: Fix quoting of some arguments in pg_ctl commandMichael Paquier2020-02-10
| | | | | | | | | | | | | | | | | | | | The previous coding forgot to apply shell quoting to the socket directory and the data folder, leading to failures when running pg_upgrade. This refactors the code generating the pg_ctl command starting clusters to use a more correct shell quoting. Failures are easier to trigger in 12 and newer versions by using a value of --socketdir that includes quotes, but it is also possible to cause failures with quotes included in the default socket directory used by pg_upgrade or the data folders of the clusters involved in the upgrade. As 9.4 is going to be EOL'd with the next minor release, nobody is likely going to upgrade to it now so this branch is not included in the set of branches fixed. Author: Michael Paquier Reviewed-by: Álvaro Herrera, Noah Misch Backpatch-through: 9.5
* Store the deletion horizon XID for a deleted GIN page on the right page.Tom Lane2020-02-09
| | | | | | | | | | | | | | | | | Commit b10714080 moved the GinPageSetDeleteXid() call to a spot where the "page" variable was pointing to the wrong page, causing the XID to be inserted on a page that's not being deleted, thus allowing later GinPageIsRecyclable tests to recycle the deleted page too soon. It might be a good idea to stop using the single "page" variable for multiple purposes in this function. But for the moment I just moved the GinPageSetDeleteXid() call down beside the GinPageSetDeleted() call, which seems like a more logical place for it anyway. Back-patch to v11, as the faulty patch was. (Fortunately, the bug hasn't made it into any release yet.) Discussion: https://postgr.es/m/21620.1581098806@sss.pgh.pa.us
* Revert recent change of ScanOption values and renumber SO_TYPE_TIDSCAN.Fujii Masao2020-02-08
| | | | | | | | | | | | | | | Commit 598b466e80 in v12 renumbered ScanOption enum values to add the option for Tid scan. But the change of the codes assigned to the existing values caused an ABI break and may break some extensions depending on them. This should be avoided in minor version. This commit reverts the renumbering of ScanOption enum values made by commit 598b466e80 in v12 and put the ScanOption for Tid scan with new value. This is applied only to v12. Per complaint from Tom Lane. Discussion: https://postgr.es/m/5261.1581103527@sss.pgh.pa.us
* Fix failure to create FKs correctly in partitionsAlvaro Herrera2020-02-07
| | | | | | | | | | | | | | | | | | | On a multi-level partioned table, when adding a partition not directly connected to the root table, foreign key constraints referencing the root were not cloned to the new partition, leading to the FK being possibly inadvertently violated later on. This was caused by fuzzy thinking in CloneFkReferenced (commit f56f8f8da6af): it was skipping constraints marked as having parents on the theory that cloning those would create duplicates; but that's only correct for the top level of the partitioning hierarchy. For levels below that one, such constraints must still be considered and only skipped if later on we see that we'd create duplicates. Apparently, I (Álvaro) wrote the comments right but the code implemented something slightly different. Author: Jehan-Guillaume de Rorthais Discussion: https://postgr.es/m/20200206004948.238352db@firost
* Fix TRUNCATE .. CASCADE on partitionsAlvaro Herrera2020-02-07
| | | | | | | | | | | | | | | | When running TRUNCATE CASCADE on a child of a partitioned table referenced by another partitioned table, the truncate was not applied to partitions of the referencing table; this could leave rows violating the constraint in the referencing partitioned table. Repair by walking the pg_constraint chain all the way up to the topmost referencing table. Note: any partitioned tables containing FKs that reference other partitioned tables should be checked for possible violating rows, if TRUNCATE has occurred in partitions of the referenced table. Reported-by: Christophe Courtois Author: Jehan-Guillaume de Rorthais Discussion: https://postgr.es/m/20200204183906.115f693e@firost
* Fix bug in Tid scan.Fujii Masao2020-02-07
| | | | | | | | | | | | | | | | | | | | | | | Commit 147e3722f7 changed Tid scan so that it calls table_beginscan() and uses the scan option for seq scan. This change caused two issues. (1) The change caused Tid scan to take a predicate lock on the entire relation in serializable transaction even when relation-level lock is not necessary. This could lead to an unexpected serialization error. (2) The change caused Tid scan to increment the number of seq_scan in pg_stat_*_tables views even though it's not seq scan. This could confuse the users. This commit adds the scan option for Tid scan and makes Tid scan use it, to avoid those issues. Back-patch to v12, where the bug was introduced. Author: Tatsuhito Kasahara Reviewed-by: Kyotaro Horiguchi, Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/CAP0=ZVKy+gTbFmB6X_UW0pP3WaeJ-fkUWHoD-pExS=at3CY76g@mail.gmail.com
* Revert "Add GUC checks for ssl_min_protocol_version and ↵Michael Paquier2020-02-07
| | | | | | | | | | | | | ssl_max_protocol_version" This reverts commit 41aadee, as the GUC checks could run on older values with the new values used, and result in incorrect errors if both parameters are changed at the same time. Per complaint from Tom Lane. Discussion: https://postgr.es/m/27574.1581015893@sss.pgh.pa.us Backpatch-through: 12
* Fix typo.Amit Kapila2020-02-06
| | | | | | | Reported-by: Amit Langote Author: Amit Langote Backpatch-through: 9.6, where it was introduced Discussion: https://postgr.es/m/CA+HiwqFNADeukaaGRmTqANbed9Fd81gLi08AWe_F86_942Gspw@mail.gmail.com
* Fix bug in LWLock statistics mechanism.Fujii Masao2020-02-06
| | | | | | | | | | | | | | | | | | | | | | | | | | Previously PostgreSQL built with -DLWLOCK_STATS could report more than one LWLock statistics entries for the same backend process and the same LWLock. This is strange and only one statistics should be output in that case, instead. The cause of this issue is that the key variable used for LWLock stats hash table was not fully initialized. The key consists of two fields and they were initialized. But the following 4 bytes allocated in the key variable for the alignment was not initialized. So even if the same key was specified, hash_search(HASH_ENTER) could not find the existing entry for that key and created new one. This commit fixes this issue by initializing the key variable with zero. As the side effect of this commit, the volume of LWLock statistics output would be reduced very much. Back-patch to v10, where commit 3761fe3c20 introduced the issue. Author: Fujii Masao Reviewed-by: Julien Rouhaud, Kyotaro Horiguchi Discussion: https://postgr.es/m/26359edb-798a-568f-d93a-6aafac49752d@oss.nttdata.com
* Force tuple conversion when the source has missing attributes.Andrew Gierth2020-02-05
| | | | | | | | | | | | | | | | | | | | | Tuple conversion incorrectly concluded that no conversion was needed as long as all the attributes lined up. But if the source tuple has a missing attribute (from addition of a column with default), then the destination tupdesc might not reflect the same default. The typical symptom was that the affected columns would be unexpectedly NULL. Repair by always forcing conversion if the source has missing attributes, which will be filled in by the deform operation. (In theory we could optimize for when the destination has the same default, but that seemed overkill.) Backpatch to 11 where missing attributes were added. Per bug #16242. Vik Fearing (discovery, code, testing) and me (analysis, testcase). Discussion: https://postgr.es/m/16242-d1c9fca28445966b@postgresql.org
* When a TAP file has non-zero exit status, retain temporary directories.Noah Misch2020-02-05
| | | | | | | | | | | | PostgresNode already retained base directories in such cases. Stop using $SIG{__DIE__}, which is redundant with the exit status check, in lieu of proliferating it to TestLib. Back-patch to 9.6, where commit 88802e068017bee8cea7a5502a712794e761c7b5 introduced retention on failure. Reviewed by Daniel Gustafsson. Discussion: https://postgr.es/m/20200202170155.GA3264196@rfd.leadboat.com
* Handle lack of DSM slots in parallel btree build, take 2.Thomas Munro2020-02-05
| | | | | | | | | | Commit 74618e77 added a new check intended to fix a bug, but put it in the wrong place so that parallel btree build was always disabled. Do the check after we've actually tried to create a DSM segment. Back-patch to 11, like the earlier commit. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WzmDABkJzrNnvf%2BOULK-_A_j9gkYg_Dz-H62jzNv4eKQTw%40mail.gmail.com
* Fix handling of "Subplans Removed" field in EXPLAIN output.Tom Lane2020-02-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 499be013d added this field in a rather poorly-thought-through manner, with the result being that rather than being a field of the Append or MergeAppend plan node as intended (and as it seems to be, in text format), it was actually an element of the "Plans" subgroup. At least in JSON format, that's flat out invalid syntax, because "Plans" is an array not an object. While it's not hard to move the generation of the field so that it appears where it's supposed to, this does result in a visible change in field order in text format, in cases where a Append or MergeAppend plan node has any InitPlans attached. That's slightly annoying to do in stable branches; but the alternative of continuing to emit broken non-text formats seems worse. Also, since the set of fields emitted is not supposed to be data-dependent in non-text formats, make sure that "Subplans Removed" appears in Append and MergeAppend nodes even when it's zero, in those formats. (The previous coding made it look like it could appear in some other node types such as BitmapAnd, but we don't actually support runtime pruning there, so don't emit it in those cases.) Per bug #16171 from Mahadevan Ramachandran. Fix by Daniel Gustafsson and Tom Lane, reviewed by Hamid Akhtar. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/16171-b72259ab75505fa2@postgresql.org
* Add missing break out seqscan loop in logical replicationAlvaro Herrera2020-02-03
| | | | | | | | | | | | | When replica identity is FULL (an admittedly unusual case), the loop that searches for tuples in execReplication.c didn't stop scanning the table when once a matching tuple was found. Add the missing 'break'. Note slight behavior change: we now return the first matching tuple rather than the last one. They are supposed to be indistinguishable anyway, so this shouldn't matter. Author: Konstantin Knizhnik Discussion: https://postgr.es/m/379743f6-ae91-b866-f7a2-5624e6d2b0a4@postgrespro.ru
* Revert commit de0177788b.Fujii Masao2020-02-03
| | | | | | | | | | | | This commit reverts the fix "Make inherited TRUNCATE perform access permission checks on parent table only" only in the back branches. It's not hard to imagine that there are some applications expecting the old behavior and the fix breaks their security. To avoid this compatibility problem, we decided to apply the fix only in HEAD and revert it in all supported back branches. Discussion: https://postgr.es/m/21015.1580400165@sss.pgh.pa.us
* Fix memory leak on DSM slot exhaustion.Thomas Munro2020-02-01
| | | | | | | | | | | | | | If we attempt to create a DSM segment when no slots are available, we should return the memory to the operating system. Previously we did that if the DSM_CREATE_NULL_IF_MAXSEGMENTS flag was passed in, but we didn't do it if an error was raised. Repair. Back-patch to 9.4, where DSM segments arrived. Author: Thomas Munro Reviewed-by: Robert Haas Reported-by: Julian Backes Discussion: https://postgr.es/m/CA%2BhUKGKAAoEw-R4om0d2YM4eqT1eGEi6%3DQot-3ceDR-SLiWVDw%40mail.gmail.com
* Fix CheckAttributeType's handling of collations for ranges.Tom Lane2020-01-31
| | | | | | | | | | | | | | | | | | | | Commit fc7695891 changed CheckAttributeType to recurse into ranges, but made it pass down the wrong collation (always InvalidOid, since ranges as such have no collation). This would result in guaranteed failure when considering a range type whose subtype is collatable. Embarrassingly, we lack any regression tests that would expose such a problem (but fortunately, somebody noticed before we shipped this bug in any release). Fix it to pass down the range's subtype collation property instead, and add some regression test cases to exercise collatable-subtype ranges a bit more. Back-patch to all supported branches, as the previous patch was. Report and patch by Julien Rouhaud, test cases tweaked by me Discussion: https://postgr.es/m/CAOBaU_aBWqNweiGUFX0guzBKkcfJ8mnnyyGC_KBQmO12Mj5f_A@mail.gmail.com
* Fix parallel pg_dump/pg_restore for failure to create worker processes.Tom Lane2020-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we failed to fork a worker process, or create a communication pipe for one, WaitForTerminatingWorkers would suffer an assertion failure if assert-enabled, otherwise crash or go into an infinite loop. This was a consequence of not accounting for the startup condition where we've not yet forked all the workers. The original bug was that ParallelBackupStart would set workerStatus to WRKR_IDLE before it had successfully forked a worker. I made things worse in commit b7b8cc0cf by not understanding the undocumented fact that the WRKR_TERMINATED state was also meant to represent the case where a worker hadn't been started yet: I changed enum T_WorkerStatus so that *all* the worker slots were initially in WRKR_IDLE state. But this wasn't any more broken in practice, since even one slot in the wrong state would keep WaitForTerminatingWorkers from terminating. In v10 and later, introduce an explicit T_WorkerStatus value for worker-not-started, in hopes of preventing future oversights of the same ilk. Before that, just document that WRKR_TERMINATED is supposed to cover that case (partly because it wasn't actively broken, and partly because the enum is exposed outside parallel.c in those branches, so there's microscopically more risk involved in changing it). In all branches, introduce a WORKER_IS_RUNNING status test macro to hide which T_WorkerStatus values mean that, and be more careful not to access ParallelSlot fields till we're sure they're valid. Per report from Vignesh C, though this is my patch not his. Back-patch to all supported branches. Discussion: https://postgr.es/m/CALDaNm1Luv-E3sarR+-unz-BjchquHHyfP+YC+2FS2pt_J+wxg@mail.gmail.com
* Fix typo in recently-added TAP test for replication slotsMichael Paquier2020-01-31
| | | | Oversight in commit b0afdca.
* Handle lack of DSM slots in parallel btree build.Thomas Munro2020-01-31
| | | | | | | | | | | | | If no DSM slots are available, a ParallelContext can still be created, but its seg pointer is NULL. Teach parallel btree build to cope with that by falling back to a regular non-parallel build, to avoid crashing with a segmentation fault. Back-patch to 11, where parallel CREATE INDEX landed. Reported-by: Nicola Contu Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/CA%2BhUKGJgJEBnkuODBVomyK3MWFvDBbMVj%3Dgdt6DnRPU-5sQ6UQ%40mail.gmail.com
* Make inherited TRUNCATE perform access permission checks on parent table only.Fujii Masao2020-01-31
| | | | | | | | | | | | | | Previously, TRUNCATE command through a parent table checked the permissions on not only the parent table but also the children tables inherited from it. This was a bug and inherited queries should perform access permission checks on the parent table only. This commit fixes that bug. Back-patch to all supported branches. Author: Amit Langote Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAHGQGwFHdSvifhJE+-GSNqUHSfbiKxaeQQ7HGcYz6SC2n_oDcg@mail.gmail.com
* Fix slot data persistency when advancing physical replication slotsMichael Paquier2020-01-30
| | | | | | | | | | | | | | | | | | | Advancing a physical replication slot with pg_replication_slot_advance() did not mark the slot as dirty if any advancing was done, preventing the follow-up checkpoint to flush the slot data to disk. This caused the advancing to be lost even on clean restarts. This does not happen for logical slots as any advancing marked the slot as dirty. Per discussion, the original feature has been implemented so as in the event of a crash the slot may move backwards to a past LSN. This property is kept and more documentation is added about that. This commit adds some new TAP tests to check the persistency of physical and logical slots after advancing across clean restarts. Author: Alexey Kondratov, Michael Paquier Reviewed-by: Andres Freund, Kyotaro Horiguchi, Craig Ringer Discussion: https://postgr.es/m/059cc53a-8b14-653a-a24d-5f867503b0ee@postgrespro.ru Backpatch-through: 11
* Fix dispsize for libpq connection parameters channel_binding and gssencmodeMichael Paquier2020-01-29
| | | | | | | | | | | | | channel_binding's longest allowed value is not "7", it is actually "8". gssencmode also got that wrong. A similar mistake has been fixed as of f4051e3. Backpatch down to v12, where gssencmode has been introduced. Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/20200128053633.GD1552@paquier.xyz Backpatch-through: 12
* Fix dangling pointer in EvalPlanQual machinery.Tom Lane2020-01-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | EvalPlanQualStart() supposed that it could re-use the relsubs_rowmark and relsubs_done arrays from a prior instantiation. But since they are allocated in the es_query_cxt of the recheckestate, that's just wrong; EvalPlanQualEnd() will blow away that storage. Therefore we were using storage that could have been reallocated to something else, causing all sorts of havoc. I think this was modeled on the old code's handling of es_epqTupleSlot, but since the code was anyway clearing the arrays at re-use, there's clearly no expectation of importing any outside state. So it's just a dubious savings of a couple of pallocs, which is negligible compared to setting up a new planstate tree. Therefore, just allocate the arrays always. (I moved the allocations slightly for readability.) In principle this bug could cause a problem whenever EPQ rechecks are needed in more than one target table of a ModifyTable plan node. In practice it seems not quite so easy to trigger as that; I couldn't readily duplicate a crash with a partitioned target table, for instance. That's probably down to incidental choices about when to free or reallocate stuff. The added isolation test case does seem to reliably show an assertion failure, though. Per report from Oleksii Kliukin. Back-patch to v12 where the bug was introduced (evidently by commit 3fb307bc4). Discussion: https://postgr.es/m/EEF05F66-2871-4786-992B-5F45C92FEE2E@hintbits.com
* Avoid unnecessary shm writes in Parallel Hash Join.Thomas Munro2020-01-27
| | | | | | | | | | | | | | Currently, Parallel Hash Join cannot be used for full/right joins, so there is no point in setting the match flag. It turns out that the cache coherence traffic generated by those writes slows down large systems running many-core joins, so let's stop doing that. In future, if we need to use match bits in parallel joins, we might want to consider setting them only if not already set. Back-patch to 11, where Parallel Hash Join arrived. Reported-by: Deng, Gang Discussion: https://postgr.es/m/0F44E799048C4849BAE4B91012DB910462E9897A%40SHSMSX103.ccr.corp.intel.com
* Fix EXPLAIN (SETTINGS) to follow policy about when to print empty fields.Tom Lane2020-01-26
| | | | | | | | | | | | | | | | | | | | | | In non-TEXT output formats, the "Settings" field should appear when requested, even if it would be empty. Also, get rid of the premature optimization of counting all the GUC_EXPLAIN variables at startup. Since there was no provision for adjusting that count later, all it'd take would be some extension marking a parameter as GUC_EXPLAIN to risk an assertion failure or memory stomp. We could make get_explain_guc_options() count those variables on-the-fly, or dynamically resize its array ... but TBH I do not think that making a transient array of pointers a bit smaller is worth any extra complication, especially when you consider all the other transient space EXPLAIN eats. So just allocate that array at the max possible size. In HEAD, also add some regression test coverage for this feature. Because of the memory-stomp hazard, back-patch to v12 where this feature was added. Discussion: https://postgr.es/m/19416.1580069629@sss.pgh.pa.us
* Fix an oversight in commit 4c70098ff.Tom Lane2020-01-23
| | | | | | | | | | | | | | | | | | | | I had supposed that the from_char_seq_search() call sites were all passing the constant arrays you'd expect them to pass ... but on looking closer, the one for DY format was passing the days[] array not days_short[]. This accidentally worked because the day abbreviations in English are all the same as the first three letters of the full day names. However, once we took out the "maximum comparison length" logic, it stopped working. As penance for that oversight, add regression test cases covering this, as well as every other switch case in DCH_from_char() that was not reached according to the code coverage report. Also, fold the DCH_RM and DCH_rm cases into one --- now that seq_search is case independent, there's no need to pass different comparison arrays for those cases. Back-patch, as the previous commit was.
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-04 16:33:26 +0000