- 07 Oct, 2019 8 commits
-
-
Bruce Momjian authored
This should help people clearly know that these days start at midnight. Reported-by: David Harper Discussion: https://postgr.es/m/156258047907.1181.11324468080514061996@wrigleys.postgresql.org Backpatch-through: 9.4
-
Bruce Momjian authored
Previously it was mentioned in the lock_timeout docs in a confusing location. Reported-by: ivaylo.zlatanov@gmail.com Discussion: https://postgr.es/m/157019615723.25307.15449102262106437404@wrigleys.postgresql.org Backpatch-through: 9.4
-
Tom Lane authored
The postmaster's code path for spawning a bgworker neglected to check whether we already have the max number of live child processes. That's a bit hard to hit, since it would necessarily be a transient condition; but if we do, AssignPostmasterChildSlot() fails causing a postmaster crash, as seen in a report from Bhargav Kamineni. To fix, invoke canAcceptConnections() in the bgworker code path, as we do in the other code paths that spawn children. Since we don't want the same pmState tests in this case, add a child-process-type parameter to canAcceptConnections() so that it can know what to do. Back-patch to 9.5. In principle the same hazard exists in 9.4, but the code is enough different that this patch wouldn't quite fix it there. Given the tiny usage of bgworkers in that branch it doesn't seem worth creating a variant patch for it. Discussion: https://postgr.es/m/18733.1570382257@sss.pgh.pa.us
-
Peter Eisentraut authored
Since 63bd0db1 we don't use tzname anymore, so we don't need to check for it. Instead, just keep the part of PGAC_STRUCT_TIMEZONE that we need, which is the check for struct tm.tm_zone. Discussion: https://www.postgresql.org/message-id/flat/5eb11a37-f3ca-5fb7-308f-4485dec25a2e%402ndquadrant.com
-
Peter Eisentraut authored
Change from HAVE_TM_ZONE to HAVE_STRUCT_TM_TM_ZONE.
-
Tom Lane authored
Temporarily change pg_ctl so that the postmaster's exit status will be printed (to the postmaster's stdout). This is to help identify the cause of intermittent "postmaster exited during a parallel transaction" failures seen on a couple of buildfarm members. This change degrades pg_ctl's functionality in a couple of minor ways, so we'll revert it once we've obtained the desired info. Discussion: https://postgr.es/m/18537.1570421268@sss.pgh.pa.us
-
Peter Eisentraut authored
HEAD as used here was CVS terminology. Now we mean master.
-
Michael Paquier authored
This includes a couple of changes around the new behavior of pg_rewind which enforces recovery to happen once on a cluster not shut down cleanly: - Some comments and documentation improvements. - Shutdown the cluster to rewind with immediate mode in all the tests, this allows to check after the forced recovery behavior which is wanted as new default. - Use -F for the forced recovery step, so as postgres does not use fsync. This was useless as a final sync is done once the tool is done. Author: Michael Paquier Reviewed-by: Alexey Kondratov Discussion: https://postgr.es/m/20191004083721.GA1829@paquier.xyz
-
- 06 Oct, 2019 2 commits
-
-
Tom Lane authored
Commit aa087ec6 was a bit over-hasty about the doc changes needed while splitting pg_statistic_ext_data off from pg_statistic_ext. It duplicated one para and inserted another in what seems to me to be the wrong section. Fix that up, and in passing do some minor copy-editing. Per report from noborusai. Discussion: https://postgr.es/m/CAAM3qnLXLUz4mOBkqa8jxigpKhKNxzSuvwpjvCRPvO5EqWjxSg@mail.gmail.com
-
Tom Lane authored
Commit 1cff1b95 included some code that supposed it could repalloc() a memory chunk to a smaller size without risk of the chunk moving. That was not a great idea, because it depended on undocumented behavior of AllocSetRealloc, which commit c477f3e4 changed thereby breaking it. (Not to mention that this code ought to work with other memory context types, which might not work the same...) So get rid of the repalloc calls, and instead just wipe the now-unused ListCell array and/or tell Valgrind it's NOACCESS, as if we'd freed it. In cases where the initial list allocation had been quite large, this could represent an annoying waste of space. In principle we could ameliorate that by allocating the initial cell array separately when it exceeds some threshold. But that would complicate new_list() which is hot code, and the returns would materialize only in narrow cases. On balance I don't think it'd be worth it. Discussion: https://postgr.es/m/17059.1570208426@sss.pgh.pa.us
-
- 05 Oct, 2019 5 commits
-
-
Tomas Vondra authored
Commit f2369bc6 switched most of the memory accounting from int64 to Size, but it forgot to change the MemoryContextMemAllocated return type. So this fixes that omission. Discussion: https://www.postgresql.org/message-id/11238.1570200198%40sss.pgh.pa.us
-
Noah Misch authored
This prints the unexpected value in more failure cases, and it removes forty-eight hand-maintained error messages. Back-patch to 9.5, which introduced these tests. Reviewed (in an earlier version) by Andres Freund. Discussion: https://postgr.es/m/20190915160021.GA24376@alvherre.pgsql
-
Tom Lane authored
This would be all right, maybe, if it didn't also match a file that definitely should not be ignored. We don't add rmgrs so often that manual maintenance of this file list is impractical, so just write out the list. (I find the equivalent wildcard use in the Makefile pretty lazy and unsafe as well, but will leave that alone until it actually causes a problem.) Per bug #16042 from Denis Stuchalin. Discussion: https://postgr.es/m/16042-c174ee692ac21cbd@postgresql.org
-
Andres Freund authored
Discussion: https://postgr.es/m/20191004222437.45qmglpto43pd3jb@alap3.anarazel.de Backpatch: 9.6-, just like c8841199 and 6e61d75f
-
Andres Freund authored
One of the upsert related tests is unstable (sometimes even hanging until isolationtester's step timeout is reached). Based on preliminary analysis that might be a problem outside of just that test, but not really related to EPQ and triggers. Disable for now, to get the buildfarm greener again. Discussion: https://postgr.es/m/20191004222437.45qmglpto43pd3jb@alap3.anarazel.de Backpatch: 9.6-, just like c8841199.
-
- 04 Oct, 2019 10 commits
-
-
Andres Freund authored
As evidenced by bug #16036 this area is woefully under-tested. Add fairly extensive tests for the combination. Backpatch back to 9.6 - before that isolationtester was not capable enough. While we don't backpatch tests all the time, future fixes to trigger.c would potentially look different enough in 12+ from the earlier branches that introducing bugs during backpatching is more likely than normal. Also, it's just a crucial and undertested area of the code. Author: Andres Freund Discussion: https://postgr.es/m/16036-28184c90d952fb7f@postgresql.org Backpatch: 9.6-, the earliest these tests work
-
Andres Freund authored
When ExecBRUpdateTriggers()'s GetTupleForTrigger() follows an EPQ chain the former needs to run the result tuple through the junkfilter again, and update the slot containing the new version of the tuple to contain that new version. The input tuple may already be in the junkfilter's output slot, which used to be OK - we don't need the previous version anymore. Unfortunately ff11e7f4 started to use ExecCopySlot() to update newslot, and ExecCopySlot() doesn't support copying a slot into itself, leading to a slot in a corrupt state, which then can cause crashes or other symptoms. Fix this by skipping the ExecCopySlot() when copying into itself. While we could have easily made ExecCopySlot() handle that case, it seems better to add an assert forbidding doing so instead. As the goal of copying might be to make the contents of one slot independent from another, it seems failure prone to handle doing so silently. A follow-up commit will add tests for the obviously under-covered combination of EPQ and triggers. Done as a separate commit as it might make sense to backpatch them further than this bug. Also remove confusion with confusing variable names for slots in ExecBRDeleteTriggers() and ExecBRUpdateTriggers(). Bug: #16036 Reported-By: Антон Власов Author: Andres Freund Discussion: https://postgr.es/m/16036-28184c90d952fb7f@postgresql.org Backpatch: 12-, where ff11e7f4 was merged
-
Andres Freund authored
Cribbing from dfbaed45: Some operating systems, including the reporter's windows, return EBADFD or similar when fsync() is invoked on a O_RDONLY file descriptor. Unfortunately RestoreSlotFromDisk() does exactly that; which causes failures after restarts in at least some scenarios. If you hit the bug the error message will be something like ERROR: could not fsync file "pg_replslot/$name/state": Bad file descriptor Simply use O_RDWR instead of O_RDONLY when opening the relevant file descriptor to fix the bug. Unfortunately this fix was undone in 82a5649f. Re-apply, and add a comment. Bug: 16039 Reported-By: Hans Buschmann Author: Andres Freund Discussion: https://postgr.es/m/16039-196fc97cc05e141c@postgresql.org Backpatch: 12-, as 82a5649f
-
Andrew Dunstan authored
First, make sure that the .exe name is quoted when trying to get the version number. Also, don't quote the lib name for using in the project files if it's already been quoted. This second change applies to all libraries, not just OpenSSL. This has clearly been broken forever, so backpatch to all live branches.
-
Robert Haas authored
The old names for the attribute-detoasting functions names included the word "heap," which seems outdated now that the heap is only one of potentially many table access methods. On the other hand, toast_insert_or_update and toast_delete are heap-specific, so rename them by adding "heap_" as a prefix. Not all of the work of making the TOAST system fully accessible to AMs other than the heap is done yet, but there seems to be little harm in getting this renaming out of the way now. Commit 8b94dab0 already divided up the functions among various files partially according to whether it was intended that they should be heap-specific or AM-agnostic, so this is just clarifying the division contemplated by that commit. Patch by me, reviewed and tested by Prabhat Sabu, Thomas Munro, Andres Freund, and Álvaro Herrera. Discussion: http://postgr.es/m/CA+TgmoZv-=2iWM4jcw5ZhJeL18HF96+W1yJeYrnGMYdkFFnEpQ@mail.gmail.com
-
Tom Lane authored
Commit 5ac0d936 failed to entirely fix bitshiftright's habit of leaving one-bits in the pad space that should be all zeroes, because in a moment of sheer brain fade I'd concluded that only the code path used for not-a-multiple-of-8 shift distances needed to be fixed. Of course, a multiple-of-8 shift distance can also cause the problem, so we need to forcibly zero the extra bits in both cases. Per bug #16037 from Alexander Lakhin. As before, back-patch to all supported branches. Discussion: https://postgr.es/m/16037-1d1ebca564db54f4@postgresql.org
-
Tomas Vondra authored
Commit 5dd7fc15 added block-level memory accounting, but used int64 variable to track the amount of allocated memory. That is incorrect, because we have Size for exactly these purposes, but it was mostly harmless until c477f3e4 which changed how we handle with repalloc() when downsizing the chunk. Previously we've ignored these cases and just kept using the original chunk, but now we need to update the accounting, and the code was doing this: context->mem_allocated += blksize - oldblksize; Both blksize and oldblksize are Size (so unsigned) which means the subtraction underflows, producing a very high positive value. On 64-bit platforms (where Size has the same size as mem_alllocated) this happens to work because the result wraps to the right value, but on (some) 32-bit platforms this fails. This fixes two things - it changes mem_allocated (and related variables) to Size, and it splits the update to two separate steps, to prevent any underflows. Discussion: https://www.postgresql.org/message-id/15151.1570163761%40sss.pgh.pa.us
-
Robert Haas authored
Allocate notify-related state lazily instead. This makes trivial subtransactions noticeably faster. Patch by me, reviewed and tested by Dilip Kumar, Kyotaro Horiguchi, and Jeevan Ladhe. Discussion: https://postgr.es/m/CA+TgmobE1J22S1eC-6N-je9LgrcwZypkwp+zH6JXo9mc=4Nk3A@mail.gmail.com
-
Michael Paquier authored
This fixes two issues with recent features added in pg_rewind: - --dry-run should do nothing on the target directory, but 927474ce forgot to consider that for --write-recovery-conf. - --no-ensure-shutdown was not actually working. There is no test coverage for this option yet, but a subsequent patch will add that. Author: Alexey Kondratov Discussion: https://postgr.es/m/7ca88204-3e0b-2f4c-c8af-acadc4b266e5@postgrespro.ru
-
Michael Paquier authored
Even if --dry-run mode was specified, the control file was getting updated, preventing follow-up runs of pg_rewind to work properly on the target data folder. The origin of the problem came from the refactoring done by ce6afc68. Author: Alexey Kondratov Discussion: https://postgr.es/m/7ca88204-3e0b-2f4c-c8af-acadc4b266e5@postgrespro.ru Backpatch-through: 12
-
- 03 Oct, 2019 4 commits
-
-
Tom Lane authored
Encoding conversion uses the very simplistic rule that the output can't be more than 4X longer than the input, and palloc's a buffer of that size. This results in failure to convert any string longer than 1/4 GB, which is becoming an annoying limitation. As a band-aid to improve matters, allow the allocated output buffer size to exceed 1GB. We still insist that the final result fit into MaxAllocSize (1GB), though. Perhaps it'd be safe to relax that restriction, but it'd require close analysis of all callers, which is daunting (not least because external modules might call these functions). For the moment, this should allow a 2X to 4X improvement in the longest string we can convert, which is a useful gain in return for quite a simple patch. Also, once we have successfully converted a long string, repalloc the output down to the actual string length, returning the excess to the malloc pool. This seems worth doing since we can usually expect to give back several MB if we take this path at all. This still leaves much to be desired, most notably that the assumption that MAX_CONVERSION_GROWTH == 4 is very fragile, and yet we have no guard code verifying that the output buffer isn't overrun. Fixing that would require significant changes in the encoding conversion APIs, so it'll have to wait for some other day. The present patch seems safely back-patchable, so patch all supported branches. Alvaro Herrera and Tom Lane Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us
-
Tom Lane authored
Up to now, if you resized a large (>8K) palloc chunk down to a smaller size, aset.c made no attempt to return any space to the malloc pool. That's unpleasant if a really large allocation is resized to a significantly smaller size. I think no such cases existed when this code was designed, and I'm not sure whether they're common even yet, but an upcoming fix to encoding conversion will certainly create such cases. Therefore, fix AllocSetRealloc so that it gives realloc() a chance to do something with the block. This doesn't noticeably increase complexity, we mostly just have to change the order in which the cases are considered. Back-patch to all supported branches. Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us
-
Andrew Gierth authored
query_tree_walker and query_tree_mutator were skipping the windowClause of the query, without regard for the fact that the startOffset and endOffset in a WindowClause node are expression trees that need to be processed. This was an oversight in commit ec4be2ee from 2010 which added the expression fields; the main symptom is that function parameters in window frame clauses don't work in inlined functions. Fix (as conservatively as possible since this needs to not break existing out-of-tree callers) and add tests. Backpatch all the way, since this has been broken since 9.0. Per report from Alastair McKinley; fix by me with kibitzing and review from Tom Lane. Discussion: https://postgr.es/m/DB6PR0202MB2904E7FDDA9D81504D1E8C68E3800@DB6PR0202MB2904.eurprd02.prod.outlook.com
-
Amit Kapila authored
These new options allow users to partition the pgbench_accounts table by specifying the number of partitions and partitioning method. The values allowed for partitioning method are range and hash. This feature allows users to measure the overhead of partitioning if any. Author: Fabien COELHO Reviewed-by: Amit Kapila, Amit Langote, Dilip Kumar, Asif Rehman, and Alvaro Herrera Discussion: https://postgr.es/m/alpine.DEB.2.21.1907230826190.7008@lancre
-
- 02 Oct, 2019 2 commits
-
-
Michael Paquier authored
cbc55da5 has reworked the order of some actions at the end of archive recovery. Unfortunately this overlooked the fact that the startup process needs to remove RECOVERYXLOG (for temporary WAL segment newly recovered from archives) and RECOVERYHISTORY (for temporary history file) at this step, leaving the files around even after recovery ended. Backpatch to 9.5, like the previous commit. Author: Sawada Masahiko Reviewed-by: Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/CAD21AoBO_eDQub6zojFnWtnmutRBWvYf7=cW4Hsqj+U_R26w3Q@mail.gmail.com Backpatch-through: 9.5
-
Michael Paquier authored
The location of the session end hook has been chosen so as it is possible to allow modules to do their own transactions, however any trying to any any subsystem which went through before_shmem_exit() would cause issues, limiting the pluggability of the hook. Per discussion with Tom Lane and Andres Freund. Discussion: https://postgr.es/m/18722.1569906636@sss.pgh.pa.us
-
- 01 Oct, 2019 7 commits
-
-
Tomas Vondra authored
Commit 11a078cf triggered failures on big-endian machines, and the only plausible place for an issue seems to be that TOAST_COMPRESS_SIZE calls VARSIZE instead of VARSIZE_ANY. So try fixing that blindly. Discussion: https://www.postgresql.org/message-id/20191001131803.j6uin7nho7t6vxzy%40development
-
Tomas Vondra authored
This fixes two compiler warnings about unused variables in non-assert builds, introduced by 5dd7fc15.
-
Tomas Vondra authored
Commit 4d0e994e added support for partial TOAST decompression, so the decompression is interrupted after producing the requested prefix. For prefix and slices near the beginning of the entry, this may saves a lot of decompression work. That however only deals with decompression - the whole compressed entry was still fetched and re-assembled, even though the compression used only a small fraction of it. This commit improves that by computing how much compressed data may be needed to decompress the requested prefix, and then fetches only the necessary part. We always need to fetch a bit more compressed data than the requested (uncompressed) prefix, because the prefix may not be compressible at all and pglz itself adds a bit of overhead. That means this optimization is most effective when the requested prefix is much smaller than the whole compressed entry. Author: Binguo Bao Reviewed-by: Andrey Borodin, Tomas Vondra, Paul Ramsey Discussion: https://www.postgresql.org/message-id/flat/CAL-OGkthU9Gs7TZchf5OWaL-Gsi=hXqufTxKv9qpNG73d5na_g@mail.gmail.com
-
Michael Paquier authored
Several buildfarm machines have been complaining about the new module test_session_hooks to be unstable, like crake and thorntail. The issue was that the module was trying to log some start and end session activity for parallel workers, which makes little sense as they don't support DML, so just prevent this pattern to happen in the module. This could be reproduced by enforcing force_parallel_mode=regress, which is the value used by some of the buildfarm members. Discussion: https://postgr.es/m/20191001045246.GF2781@paquier.xyz
-
Michael Paquier authored
These hooks can be used in loadable modules. A simple test module is included. The first attempt was done with cd8ce3a2 but we lacked handling for NO_INSTALLCHECK in the MSVC scripts (problem solved afterwards by 431f1599) so the buildfarm got angry. This also fixes a couple of issues noticed upon review compared to the first attempt, so the code has slightly changed, resulting in a more simple test module. Author: Fabrízio de Royes Mello, Yugo Nagata Reviewed-by: Andrew Dunstan, Michael Paquier, Aleksandr Parfenov Discussion: https://postgr.es/m/20170720204733.40f2b7eb.nagata@sraoss.co.jp Discussion: https://postgr.es/m/20190823042602.GB5275@paquier.xyz
-
Michael Paquier authored
When using a client compiled without channel binding support (linking to OpenSSL 1.0.1 or older) to connect to a server which supports channel binding (linking to OpenSSL 1.0.2 or newer), libpq would generate a confusing error message with channel_binding=require for an SSL connection, where the server sends back SCRAM-SHA-256-PLUS: "channel binding is required, but server did not offer an authentication method that supports channel binding." This is confusing because the server did send a SASL mechanism able to support channel binding, but libpq was not able to detect that properly. The situation can be summarized as followed for the case described in the previous paragraph for the SASL mechanisms used with the various modes of channel_binding: 1) Client supports channel binding. 1-1) channel_binding = disable => OK, with SCRAM-SHA-256. 1-2) channel_binding = prefer => OK, with SCRAM-SHA-256-PLUS. 1-3) channel_binding = require => OK, with SCRAM-SHA-256-PLUS. 2) Client does not support channel binding. 2-1) channel_binding = disable => OK, with SCRAM-SHA-256. 2-2) channel_binding = prefer => OK, with SCRAM-SHA-256. 2-3) channel_binding = require => failure with new error message, instead of the confusing one. This commit updates case 2-3 to generate a better error message. Note that the SSL TAP tests are not impacted as it is not possible to test with mixed versions of OpenSSL for the backend and libpq. Reported-by: Tom Lane Author: Michael Paquier Reviewed-by: Jeff Davis, Tom Lane Discussion: https://postgr.es/m/24857.1569775891@sss.pgh.pa.us
-
Tomas Vondra authored
Adds accounting of memory allocated in a memory context. Compared to various ad hoc solutions, the main advantage is that the accounting is transparent and does not require direct control over allocations (this matters for use cases where the allocations happen in user code, like for example aggregate states allocated in a transition functions). To reduce overhead, the accounting happens at the block level (not for individual chunks) and only the context immediately owning the block is updated. When inquiring about amount of memory allocated in a context, we have to recursively walk all children contexts. This "lazy" accounting works well for cases with relatively small number of contexts in the relevant subtree and/or with infrequent inquiries. Author: Jeff Davis Reivewed-by: Tomas Vondra, Melanie Plageman, Soumyadeep Chakraborty Discussion: https://www.postgresql.org/message-id/flat/027a129b8525601c6a680d27ce3a7172dab61aab.camel@j-davis.com
-
- 30 Sep, 2019 2 commits
-
-
Andres Freund authored
That avoids unnecessary work during both interpreted execution, and JIT compiled expression evaluation. Both benefit from fewer expression steps needing be processed, and for interpreted execution there now is a fastpath dedicated to just fetching a value from a virtual slot. That's e.g. beneficial for hashjoins over nodes that perform projections, as the hashed columns are currently fetched individually. Author: Soumyadeep Chakraborty, Andres Freund Discussion: https://postgr.es/m/CAE-ML+9OKSN71+mHtfMD-L24oDp8dGTfaVjDU6U+j+FNAW5kRQ@mail.gmail.com
-
Andres Freund authored
This is mainly in preparation for adding further fastpath evaluation routines. Also reorder ExecJust*Var functions to be consistent with the order in which they're used. Author: Andres Freund Discussion: https://postgr.es/m/CAE-ML+9OKSN71+mHtfMD-L24oDp8dGTfaVjDU6U+j+FNAW5kRQ@mail.gmail.com
-