- 19 Dec, 2019 3 commits
-
-
Robert Haas authored
Commit d986d4e8 renamed a variable but neglected to update the corresponding comment. Amit Langote
-
Robert Haas authored
The previous coding imagined that it could call before_shmem_exit() when a non-exclusive backup began and then remove the previously-added handler by calling cancel_before_shmem_exit() when that backup ended. However, this only works provided that nothing else in the system has registered a before_shmem_exit() hook in the interim, because cancel_before_shmem_exit() is documented to remove a callback only if it is the latest callback registered. It also only works if nothing can ERROR out between the time that sessionBackupState is reset and the time that cancel_before_shmem_exit(), which doesn't seem to be strictly true. To fix, leave the handler installed for the lifetime of the session, arrange to install it just once, and teach it to quietly do nothing if there isn't a non-exclusive backup in process. This is a bug, but for now I'm not going to back-patch, because the consequences are minor. It's possible to cause a spurious warning to be generated, but that doesn't really matter. It's also possible to trigger an assertion failure, but production builds shouldn't have assertions enabled. Patch by me, reviewed by Kyotaro Horiguchi, Michael Paquier (who preferred a different approach, but got outvoted), Fujii Masao, and Tom Lane, and with comments by various others. Discussion: http://postgr.es/m/CA+TgmobMjnyBfNhGTKQEDbqXYE3_rXWpc4CM63fhyerNCes3mA@mail.gmail.com
-
Peter Eisentraut authored
It's basically a variant of Cygwin, so use that template. Discussion: https://www.postgresql.org/message-id/flat/6b467edc-4018-521f-ab18-171f098557ca%402ndquadrant.com
-
- 18 Dec, 2019 11 commits
-
-
Robert Haas authored
Commit 7dbfea3c thought it could get away with removing this, but Thomas Munro reports, on behalf of the buildfarm, that it's still needed at least on Windows to avoid compiler warnings.
-
Robert Haas authored
The new function, heap_fetch_toast_slice, is shared between toast_fetch_datum_slice and toast_fetch_datum, and does all the work of scanning the TOAST table, fetching chunks, and storing them into the space allocated for the result varlena. As an incidental side effect, this allows toast_fetch_datum_slice to perform the scan with only a single scankey if all chunks are being fetched, which might have some tiny performance benefit. Discussion: http://postgr.es/m/CA+TgmobBzxwFojJ0zV0Own3dr09y43hp+OzU2VW+nos4PMXWEg@mail.gmail.com
-
Tom Lane authored
Older gcc versions are not happy with having multiple declarations for the same typedef name (not struct name). I'm a bit dubious as to how well-thought-out that patch was at all, but for the moment just fix it enough so I can get some work done today. Discussion: https://postgr.es/m/20191218101338.GB325369@paquier.xyz
-
Peter Eisentraut authored
The main use right now is getting properly spaced diff views on GitHub, but perhaps this will also help developers with editors that we currently don't have setup recipes for. The settings mirror mostly what's currently in .dir-locals.el. Discussion: https://www.postgresql.org/message-id/flat/273cb788-bbb2-ff34-ad6f-5192b44e5049%402ndquadrant.com
-
Michael Paquier authored
Oversight in commit e1551f96. Reported-by: Erik Rijkers Discussion: https://postgr.es/m/b7ad911d3eaa29af9fcdb9ccb26c363c@xs4all.nl
-
Michael Paquier authored
Tuple conversion support in tupconvert.c is able to convert rowtypes between two relations, inner and outer, which are logically equivalent but have a different ordering or even dropped columns (used mainly for inheritance tree and partitions). This makes use of attribute mappings, which are simple arrays made of AttrNumber elements with a length matching the number of attributes of the outer relation. The length of the attribute mapping has been treated as completely independent of the mapping itself until now, making it easy to pass down an incorrect mapping length. This commit refactors the code related to attribute mappings and moves it into an independent facility called attmap.c, extracted from tupconvert.c. This merges the attribute mapping with its length, avoiding to try to guess what is the length of a mapping to use as this is computed once, when the map is built. This will avoid mistakes like what has been fixed in dc816e58, which has used an incorrect mapping length by matching it with the number of attributes of an inner relation (a child partition) instead of an outer relation (a partitioned table). Author: Michael Paquier Reviewed-by: Amit Langote Discussion: https://postgr.es/m/20191121042556.GD153437@paquier.xyz
-
Amit Kapila authored
This patch allows building the local relmap cache for a subscribed relation after processing pending invalidation messages and potential relcache updates. Without this, the attributes in the local cache don't tally with the updated relcache entry leading to invalid memory access. Reported-by Jehan-Guillaume de Rorthais Author: Jehan-Guillaume de Rorthais and Vignesh C Reviewed-by: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/20191025175929.7e90dbf5@firost
-
Michael Paquier authored
That's more consistent with the style we have been using with for example EXPLAIN, VACUUM or ANALYZE (this one had only one option in v11). Based on a suggestion from Pavel Stehule. Author: Josef Šimánek Discussion: https://postgr.es/m/CAFj8pRCrUS+eMFvssVPGZN-VDEMP3XN+1Dop0=CmeBq2D+dqOg@mail.gmail.com Discussion: https://postgr.es/m/CAFp7QwpeMPEtAR5AYpsG623ooMWX03wMjq5cpZn=X+6OCkfwJw@mail.gmail.com
-
Michael Paquier authored
These have been missed in 01368e5d, and count for plpython and the backend's tsearch code. Author: Mahendra Singh Discussion: https://postgr.es/m/CAKYtNAo4mxRRyDB0YqE6QLh17XD7pPQotpGm3GnHS+gQKz4zQQ@mail.gmail.com
-
Bruce Momjian authored
Reported-by: Ranier Vilela Discussion: https://postgr.es/m/MN2PR18MB2927E73FADCA8967B2302469E3490@MN2PR18MB2927.namprd18.prod.outlook.com Author: Ranier Vilela Backpatch-through: master
-
Michael Paquier authored
This changes the routines in charge of recycling WAL segments past the last redo LSN to not use anymore "RedoRecPtr" as a local variable, which is also available in the context of the session as a static declaration, replacing it with "lastredoptr". This confusion has been introduced by d9fadbf1, so backpatch down to v11 like the other commit. Thanks to Tom Lane, Robert Haas, Alvaro Herrera, Mark Dilger and Kyotaro Horiguchi for the input provided. Author: Ranier Vilela Discussion: https://postgr.es/m/MN2PR18MB2927F7B5F690065E1194B258E35D0@MN2PR18MB2927.namprd18.prod.outlook.com Backpatch-through: 11
-
- 17 Dec, 2019 14 commits
-
-
Tom Lane authored
If CheckAttributeType() threw an error about the datatype of an index expression column, it would report an empty column name, which is pretty unhelpful and certainly not the intended behavior. I (tgl) evidently broke this in commit cfc5008a, by not noticing that the column's attname was used above where I'd placed the assignment of it. In HEAD and v12, this is trivially fixable by moving up the assignment of attname. Before v12 the code is a bit more messy; to avoid doing substantial refactoring, I took the lazy way out and just put in two copies of the assignment code. Report and patch by Amit Langote. Back-patch to all supported branches. Discussion: https://postgr.es/m/CA+HiwqFA+BGyBFimjiYXXMa2Hc3fcL0+OJOyzUNjhU4NCa_XXw@mail.gmail.com
-
Robert Haas authored
Commit d5406dea used a slightly novel, and wrong, approach to compute the length of the last toast chunk. It worked fine unless the last chunk happened to have the largest possible size.
-
Robert Haas authored
Rework some of the checks for bad TOAST chunks to be a bit simpler and easier to understand. These checks verify that (1) we get all and only the chunk numbers we expect to see and (2) each chunk has the expected size. However, the existing code was a bit hard to understand, at least for me; try to make it clearer. As part of that, have toast_fetch_datum_slice check the relationship between endchunk and totalchunks only with an Assert() rather than checking every chunk number against both values. There's no need to check that relationship in production builds because it's not a function of whether on-disk corruption is present; it's just a question of whether the code does the right math. Also, have toast_fetch_datum_slice() use ereport(ERROR) rather than elog(ERROR). Commit fd6ec93b made the two functions inconsistent with each other. Rename assorted variables for better clarity and consistency, and move assorted variables from function scope to the function's main loop. Remove a few variables that are used only once entirely. Patch by me, reviewed by Peter Eisentraut. Discussion: http://postgr.es/m/CA+TgmobBzxwFojJ0zV0Own3dr09y43hp+OzU2VW+nos4PMXWEg@mail.gmail.com
-
Robert Haas authored
Commit 48995040 removed the largest barrier to use of simplehash in frontend code, but there's one more problem: it uses elog(ERROR, ...) or elog(LOG, ...) in a couple of places. Work around that by changing those to pg_log_error() and pg_log_info() when FRONTEND is defined. Patch by me, reviewed by Andres Freund. Discussion: http://postgr.es/m/CA+Tgmob8oyh02NrZW=xCScB+5GyJ-jVowE3+TWTUmPF=FsGWTA@mail.gmail.com
-
Robert Haas authored
If the SH_RAW_ALLOCATOR is defined, it will be used to allocate bytes for the hash table, and no dependencies on MemoryContext will exist. This means, in particular, that the SH_CREATE function will not take a MemoryContext argument. Patch by me, reviewed by Andres Freund. Discussion: http://postgr.es/m/CA+Tgmob8oyh02NrZW=xCScB+5GyJ-jVowE3+TWTUmPF=FsGWTA@mail.gmail.com
-
Robert Haas authored
Commit 5910d6c7 got this wrong. Report and patch by Andrew Gierth. Discussion: http://postgr.es/m/8736diaj98.fsf@news-spur.riddles.org.uk
-
Robert Haas authored
Where possible, share signal handler code and main loop interrupt checking. This saves quite a bit of code and should simplify maintenance, too. This commit intends not to change the way anything works, even though that might allow more code to be unified. It does unify a bunch of individual variables into a ShutdownRequestPending flag that has is now used by a bunch of different process types, though. Patch by me, reviewed by Andres Freund and Daniel Gustafsson. Discussion: http://postgr.es/m/CA+TgmoZwDk=BguVDVa+qdA6SBKef=PKbaKDQALTC_9qoz1mJqg@mail.gmail.com
-
Robert Haas authored
There seems to be no reason for every background process to have its own flag indicating that a config-file reload is needed. Instead, let's just use ConfigFilePending for that purpose everywhere. Patch by me, reviewed by Andres Freund and Daniel Gustafsson. Discussion: http://postgr.es/m/CA+TgmoZwDk=BguVDVa+qdA6SBKef=PKbaKDQALTC_9qoz1mJqg@mail.gmail.com
-
Robert Haas authored
Some auxiliary processes, as well as the autovacuum launcher, have interrupt handling code directly in their main loops. Try to abstract things a little better by moving it into separate functions. This doesn't make any functional difference, and leaves in place relatively large differences among processes in how interrupts are handled, but hopefully it at least makes it easier to see the commonalities and differences across process types. Patch by me, reviewed by Andres Freund and Daniel Gustafsson. Discussion: http://postgr.es/m/CA+TgmoZwDk=BguVDVa+qdA6SBKef=PKbaKDQALTC_9qoz1mJqg@mail.gmail.com
-
Amit Kapila authored
This Assert thought that an overflowed transaction can never get registered for the group update. But that is not true, because even when the number of children for a transaction got reduced, the overflow flag is not changed. And, for group update, we only care about the current number of children for a transaction that is being committed. Based on comments by Andres Freund, remove a redundant Assert in TransactionIdSetPageStatus as we already had a static Assert for the same condition a few lines earlier. Reported-by: Vignesh C Author: Dilip Kumar Reviewed-by: Amit Kapila Backpatch-through: 11 Discussion: https://postgr.es/m/CAFiTN-s5=uJw-Z6JC9gcqtBSjXsrHnU63PXBrA=pnBjqnkm5UA@mail.gmail.com
-
Peter Geoghegan authored
Rename two function-style macros, removing the word "inner". This makes things more consistent.
-
Michael Paquier authored
The refactoring done in a4fd3aa7 for query cancellation has messed up with the logic in psql by mixing CancelRequested and cancel_pressed, breaking for example \watch. The former would be switched to true if a cancellation request has been attempted and that it actually succeeded, and the latter tracks if a cancellation attempt has been done. This commit brings back the code of psql to a state consistent to what it was before a4fd3aa7, without giving up on the refactoring pieces introduced. It should be actually possible to merge more both flags as their concepts are close enough, however note that psql's --single-step mode relies on cancel_pressed to be always set, so this requires more careful analysis left for later. While on it, fix the declarations of CancelRequested (in cancel.c) and cancel_pressed (in psql) to be volatile sig_atomic_t. Previously, both were declared as booleans, which should be fine on modern platforms, but the C standard recommends the use of sig_atomic_t for variables used in signal handlers. Note that since its introduction in a1792320, CancelRequested declaration was not volatile. Reported-by: Jeff Janes Author: Michael Paquier Discussion: https://postgr.es/m/CAMkU=1zpoUDGKqWKuMWkj7t-bOCaJDx0r=5te_-d0B2HVLABXg@mail.gmail.com
-
Tom Lane authored
force_parallel_mode = regress is supposed to force use of a Gather node without having any impact on EXPLAIN output. But it failed to accomplish that if both ANALYZE and VERBOSE are given, because that enables per-worker output data that you wouldn't see if the Gather hadn't been inserted. Improve the logic so that we suppress the per-worker data too. This allows putting the new test case added by commit 5935917c back into the originally intended form (cf. 776a2c88, 22864f6e). We can also get rid of a kluge in subselect.sql, which previously had to clean up after force_parallel_mode's failure to do what it said on the tin. Discussion: https://postgr.es/m/18445.1576177309@sss.pgh.pa.us
-
Peter Geoghegan authored
get_actual_variable_range() hasn't used a dirty snapshot since commit 3ca930fc, which invented a new snapshot type specifically to meet selfuncs.c's requirements (HeapTupleSatisfiesNonVacuumable() type snapshots were added). Discussion: https://postgr.es/m/CAH2-Wzn2pSqEOcBDAA40CnO82oEy-EOpE2bNh_XL_cfFoA86jw@mail.gmail.com
-
- 16 Dec, 2019 6 commits
-
-
Tom Lane authored
Attempting to open a file fails with ERROR_ACCESS_DENIED if the file is flagged for deletion but not yet actually gone (another in a long list of reasons why Windows is broken, if you ask me). This seems likely to explain a lot of irreproducible failures we see in the buildfarm. This state generally persists for only a millisecond or so, so just wait a bit and retry. If it's a real permissions problem, we'll eventually give up and report it as such. If it's the pending deletion case, we'll see file-not-found and report that after the deletion completes, and the caller will treat that in an appropriate way. In passing, rejigger the existing retry logic for some other error cases so that we don't uselessly wait an extra time when we're not going to retry anymore. Alexander Lakhin (with cosmetic tweaks by me). Back-patch to all supported branches, since this seems like a pretty safe change and the problem is definitely real. Discussion: https://postgr.es/m/16161-7a985d2f1bbe8f71@postgresql.org
-
Alvaro Herrera authored
recoveryDelayUntilTime was introduced by commit 36da3cfb as a global because its method of operation was devilishly intrincate. Commit c945af80 removed all that complexity and could have turned it into a local variable, but didn't. Do so now. Discussion: https://postgr.es/m/20191213200751.GA10731@alvherre.pgsql Reviewed-by: Michaël Paquier, Daniel Gustafsson
-
Heikki Linnakangas authored
Commit a7ee7c85 fixed a bug in GiST page split during index creation, where we failed to re-find the position of a downlink after the page containing it was split. However, that fix was incomplete; the other call to gistinserttuples() in the same function needs to also clear 'downlinkoffnum'. Fixes bug #16134 reported by Alexander Lakhin, for real this time. The previous fix was enough to fix the crash with the reproducer script for bug #16162, but the original script for #16134 was still crashing. Backpatch to v12, like the previous incomplete fix. Discussion: https://www.postgresql.org/message-id/d869f537-abe4-d2ea-0510-38cd053f5152%40gmail.com
-
Peter Eisentraut authored
Commit f14413b6 broke the build of Perl-using modules on Windows. Perl might have its own definitions of uid_t and gid_t, so we hide ours, but then we can't use ours in our header files such as port.h which don't see the Perl definition. Hide our definition of getpeereid() on Windows in Perl-using modules, using PLPERL_HAVE_UID_GID define. That means we can't portably use getpeeruid() is such modules right now, but there is no need anyway.
-
Peter Eisentraut authored
The getpeereid() uses have so far been protected by HAVE_UNIX_SOCKETS, so they didn't ever care about Windows support. But in anticipation of Unix-domain socket support on Windows, that needs to be handled differently. Windows doesn't support getpeereid() at this time, so we use the existing not-supported code path. We let configure do its usual thing of picking up the replacement from libpgport, instead of the custom overrides that it was doing before. But then Windows doesn't have struct passwd, so this patch sprinkles some additional #ifdef WIN32 around to make it work. This is similar to existing code that deals with this issue. Reviewed-by: Andrew Dunstan <andrew.dunstan@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/5974caea-1267-7708-40f2-6009a9d653b0@2ndquadrant.com
-
Etsuro Fujita authored
Also, add a comment explaining a test case. Back-patch to 11 where the regression test was added. Discussion: https://postgr.es/m/CAPmGK15adZPh2B%2BmGUjSOMH%2BH39ogDRWfCfm4G6jncZCAs9V_Q%40mail.gmail.com
-
- 15 Dec, 2019 1 commit
-
-
Michael Paquier authored
This has been introduced by c16dc1ac since progress reporting for VACUUM has been added. As this issue just causes some extra work and is harmless, no backpatch is done. Author: Justin Pryzby Discussion: https://postgr.es/m/20191213030831.GT2082@telsasoft.com
-
- 14 Dec, 2019 5 commits
-
-
Tom Lane authored
It appears that a concurrent autovacuum/autoanalyze run can cause changes in the plans expected by this test. To prevent that, change the tables it uses to be temp tables --- there's no need for them to be permanent, and this should save a few cycles too. Discussion: https://postgr.es/m/3244.1576160824@sss.pgh.pa.us
-
Tom Lane authored
The RTE_RESULT simplification logic added by commit 4be058fe had a flaw: it would collapse out a RTE_RESULT that is due to compute a PlaceHolderVar, and reassign the PHV to the parent join level, even if another input relation of the join contained a lateral reference to the PHV. That can't work because the PHV would be computed too late. In practice it led to failures of internal sanity checks later in planning (either assertion failures or errors such as "failed to construct the join relation"). To fix, add code to check for the presence of such PHVs in relevant portions of the query tree. Notably, this required refactoring range_table_walker so that a caller could ask to walk individual RTEs not the whole list. (It might be a good idea to refactor range_table_mutator in the same way, if only to keep those functions looking similar; but I didn't do so here as it wasn't necessary for the bug fix.) This exercise also taught me that find_dependent_phvs(), as it stood, could only safely be used on the entire Query, not on subtrees. Adjust its API to reflect that; which in passing allows it to have a fast path for the common case of no PHVs anywhere. Per report from Will Leinweber. Back-patch to v12 where the bug was introduced. Discussion: https://postgr.es/m/CALLb-4xJMd4GZt2YCecMC95H-PafuWNKcmps4HLRx2NHNBfB4g@mail.gmail.com
-
Michael Paquier authored
When loading DH parameters used for the generation of ephemeral DH keys in the backend, the code has never bothered releasing the memory used for the DH information loaded from a file or from libpq's default. This commit makes sure that the information is properly free()'d. Note that as SSL parameters can be reloaded, this can cause an accumulation of memory leaked. As the leak is minor, no backpatch is done. Reported-by: Dmitry Uspenskiy Discussion: https://postgr.es/m/16160-18367e56e9a28264@postgresql.org
-
Thomas Munro authored
The previous commit failed to consider that FileGetRawDesc() might not return a valid fd, as discovered on the build farm. Switch to using the File interface only. Back-patch to 12, like the previous commit.
-
Thomas Munro authored
_mdfd_getseg() opens all segments up to the requested one. That causes problems for mdsyncfiletag(), if mdunlinkfork() has already unlinked other segment files. Open the file we want directly by name instead, if it's not already open. The consequence of this bug was a rare panic in the checkpointer, made more likely if you saturated the sync request queue so that the SYNC_FORGET_REQUEST messages for a given relation were more likely to be absorbed in separate cycles by the checkpointer. Back-patch to 12. Defect in commit 3eb77eba. Author: Thomas Munro Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20191119115759.GI30362%40telsasoft.com
-