aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Centralize setup of SIGQUIT handling for postmaster child processes.Tom Lane2020-09-16
| | | | | | | | | | | | | | | | | | | | | | | | | | We decided that the policy established in commit 7634bd4f6 for the bgwriter, checkpointer, walwriter, and walreceiver processes, namely that they should accept SIGQUIT at all times, really ought to apply uniformly to all postmaster children. Therefore, get rid of the duplicative and inconsistent per-process code for establishing that signal handler and removing SIGQUIT from BlockSig. Instead, make InitPostmasterChild do it. The handler set up by InitPostmasterChild is SignalHandlerForCrashExit, which just summarily does _exit(2). In interactive backends, we almost immediately replace that with quickdie, since we would prefer to try to tell the client that we're dying. However, this patch is changing the behavior of autovacuum (both launcher and workers), as well as walsenders. Those processes formerly also used quickdie, but AFAICS that was just mindless copy-and-paste: they don't have any interactive client that's likely to benefit from being told this. The stats collector continues to be an outlier, in that it thinks SIGQUIT means normal exit. That should probably be changed for consistency, but there's another patch set where that's being dealt with, so I didn't do so here. Discussion: https://postgr.es/m/644875.1599933441@sss.pgh.pa.us
* Don't fetch partition check expression during InitResultRelInfo.Tom Lane2020-09-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since there is only one place that actually needs the partition check expression, namely ExecPartitionCheck, it's better to fetch it from the relcache there. In this way we will never fetch it at all if the query never has use for it, and we still fetch it just once when we do need it. The reason for taking an interest in this is that if the relcache doesn't already have the check expression cached, fetching it requires obtaining AccessShareLock on the partition root. That means that operations that look like they should only touch the partition itself will also take a lock on the root. In particular we observed that TRUNCATE on a partition may take a lock on the partition's root, contributing to a deadlock situation in parallel pg_restore. As written, this patch does have a small cost, which is that we are microscopically reducing efficiency for the case where a partition has an empty check expression. ExecPartitionCheck will be called, and will go through the motions of setting up and checking an empty qual, where before it would not have been called at all. We could avoid that by adding a separate boolean flag to track whether there is a partition expression to test. However, this case only arises for a default partition with no siblings, which surely is not an interesting case in practice. Hence adding complexity for it does not seem like a good trade-off. Amit Langote, per a suggestion by me Discussion: https://postgr.es/m/VI1PR03MB31670CA1BD9625C3A8C5DD05EB230@VI1PR03MB3167.eurprd03.prod.outlook.com
* Avoid unnecessary recursion to child tables in ALTER TABLE SET NOT NULL.Tom Lane2020-09-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a partitioned table's column is already marked NOT NULL, there is no need to examine its partitions, because we can rely on previous DDL to have enforced that the child columns are NOT NULL as well. (Unfortunately, the same cannot be said for traditional inheritance, so for now we have to restrict the optimization to partitioned tables.) Hence, we may skip recursing to child tables in this situation. The reason this case is worth worrying about is that when pg_dump dumps a partitioned table having a primary key, it will include the requisite NOT NULL markings in the CREATE TABLE commands, and then add the primary key as a separate step. The primary key addition generates a SET NOT NULL as a subcommand, just to be sure. So the situation where a SET NOT NULL is redundant does arise in the real world. Skipping the recursion does more than just save a few cycles: it means that a command such as "ALTER TABLE ONLY partition_parent ADD PRIMARY KEY" will take locks only on the partition parent table, not on the partitions. It turns out that parallel pg_restore is effectively assuming that that's true, and has little choice but to do so because the dependencies listed for such a TOC entry don't include the partitions. pg_restore could thus issue this ALTER while data restores on the partitions are still in progress. Taking unnecessary locks on the partitions not only hurts concurrency, but can lead to actual deadlock failures, as reported by Domagoj Smoljanovic. (A contributing factor in the deadlock is that TRUNCATE on a child partition wants a non-exclusive lock on the parent. This seems likewise unnecessary, but the fix for it is more invasive so we won't consider back-patching it. Fortunately, getting rid of one of these two poor behaviors is enough to remove the deadlock.) Although support for partitioned primary keys came in with v11, this patch is dependent on the SET NOT NULL refactoring done by commit f4a3fdfbd, so we can only patch back to v12. Patch by me; thanks to Alvaro Herrera and Amit Langote for review. Discussion: https://postgr.es/m/VI1PR03MB31670CA1BD9625C3A8C5DD05EB230@VI1PR03MB3167.eurprd03.prod.outlook.com
* Fix bogus cache-invalidation logic in logical replication worker.Tom Lane2020-09-16
| | | | | | | | | | | | | | | | | | | | | | | | | | The code recorded cache invalidation events by zeroing the "localreloid" field of affected cache entries. However, it's possible for an inval event to occur even while we have the entry open and locked. So an ill-timed inval could result in "cache lookup failed for relation 0" errors, if the worker's code tried to use the cleared field. We can fix that by creating a separate bool field to record whether the entry needs to be revalidated. (In the back branches, cram the bool into what had been padding space, to avoid an ABI break in the somewhat unlikely event that any extension is looking at this struct.) Also, rearrange the logic in logicalrep_rel_open so that it does the right thing in cases where table_open would fail. We should retry the lookup by name in that case, but we didn't. The real-world impact of this is probably small. In the first place, the error conditions are very low probability, and in the second place, the worker would just exit and get restarted. We only noticed because in a CLOBBER_CACHE_ALWAYS build, the failure can occur repeatedly, preventing the worker from making progress. Nonetheless, it's clearly a bug, and it impedes a useful type of testing; so back-patch to v10 where this code was introduced. Discussion: https://postgr.es/m/1032727.1600096803@sss.pgh.pa.us
* Avoid retrieval of CHECK constraints and DEFAULT exprs in data-only dumpMichael Paquier2020-09-16
| | | | | | | | | | | | | | Those extra queries are not necessary when doing a data-only dump. With this change, this means that the dependencies between CHECK/DEFAULT and the parent table are not tracked anymore for a data-only dump. However, these dependencies are only used for the schema generation and we have never guaranteed that a dump can be reloaded if a CHECK constraint uses a custom function whose behavior changes when loading the data, like when using cross-table references in the CHECK function. Author: Julien Rouhaud Reviewed-by: Daniel Gustafsson, Michael Paquier Discussion: https://postgr.es/m/20200712054850.GA92357@nol
* Change LogicalTapeSetBlocks() to use nBlocksWritten.Jeff Davis2020-09-15
| | | | | | | | | | | | | Previously, it was based on nBlocksAllocated to account for tapes with open write buffers that may not have made it to the BufFile yet. That was unnecessary, because callers do not need to get the number of blocks while a tape has an open write buffer; and it also conflicted with the preallocation logic added for HashAgg. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/ce5af05900fdbd0e9185747825a7423c48501964.camel@j-davis.com Backpatch-through: 13
* HashAgg: release write buffers sooner by rewinding tape.Jeff Davis2020-09-15
| | | | | | | | | | This was an oversight. The purpose of 7fdd919ae7 was to avoid keeping tape buffers around unnecessisarily, but HashAgg didn't rewind early enough. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/1fb1151c2cddf8747d14e0532da283c3f97e2685.camel@j-davis.com Backpatch-through: 13
* Fix initialization of RelationSyncEntry for streaming transactions.Amit Kapila2020-09-16
| | | | | | | | | | | | | | | | | | | | In commit 464824323e, for each RelationSyncEntry we maintained the list of xids (streamed_txns) for which we have already sent the schema. This helps us to track when to send the schema to the downstream node for replication of streaming transactions. Before this list got initialized, we were processing invalidation messages which access this list and led to an assertion failure. In passing, clean up the nearby code: * Initialize the list of xids with NIL instead of NULL which is our usual coding practice. * Remove the MemoryContext switch for creating a RelationSyncEntry in dynahash. Diagnosed-by: Amit Kapila and Tom Lane Author: Amit Kapila Reviewed-by: Tom Lane and Dilip Kumar Discussion: https://postgr.es/m/904373.1600033123@sss.pgh.pa.us
* Optimize compactify_tuples functionDavid Rowley2020-09-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This function could often be seen in profiles of vacuum and could often be a significant bottleneck during recovery. The problem was that a qsort was performed in order to sort an array of item pointers in reverse offset order so that we could use that to safely move tuples up to the end of the page without overwriting the memory of yet-to-be-moved tuples. i.e. we used to compact the page starting at the back of the page and move towards the front. The qsort that this required could be expensive for pages with a large number of tuples. In this commit, we take another approach to tuple compactification. Now, instead of sorting the remaining item pointers array we first check if the array is presorted and only memmove() the tuples that need to be moved. This presorted check can be done very cheaply in the calling functions when the array is being populated. This presorted case is very fast. When the item pointer array is not presorted we must copy tuples that need to be moved into a temp buffer before copying them back into the page again. This differs from what we used to do here as we're now copying the tuples back into the page in reverse line pointer order. Previously we left the existing order alone. Reordering the tuples results in an increased likelihood of hitting the pre-sorted case the next time around. Any newly added tuple which consumes a new line pointer will also maintain the correct sort order of tuples in the page which will also result in the presorted case being hit the next time. Only consuming an unused line pointer can cause the order of tuples to go out again, but that will be corrected next time the function is called for the page. Benchmarks have shown that the non-presorted case is at least equally as fast as the original qsort method even when the page just has a few tuples. As the number of tuples becomes larger the new method maintains its performance whereas the original qsort method became much slower when the number of tuples on the page became large. Author: David Rowley Reviewed-by: Thomas Munro Tested-by: Jakub Wartak Discussion: https://postgr.es/m/CA+hUKGKMQFVpjr106gRhwk6R-nXv0qOcTreZuQzxgpHESAL6dw@mail.gmail.com
* Fix use-after-free bug with event triggers in an extension scriptAlvaro Herrera2020-09-15
| | | | | | | | | | | | | | | | | | | | ALTER TABLE commands in an extension script are added to an event trigger command list; but starting with commit b5810de3f4 they do so in a memory context that's too short-lived, so when execution ends and time comes to use the entries, they've already been freed. (This would also be a problem with ALTER TABLE commands in a multi-command query string, but these serendipitously end in PortalContext -- which probably explains why it took so long for this to be reported.) Fix by using the memory context specifically set for that, instead. Backpatch to 13, where the aforementioned commit appeared. Reported-by: Philippe Beaudoin Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20200902193715.6e0269d4@firost
* Report resource usage at the end of recoveryDavid Rowley2020-09-16
| | | | | | | | | | Reporting this has been rather useful in some recent recovery speedup work. It also seems like something that will be useful to the average DBA too. Author: David Rowley Reviewed-by: Thomas Munro Discussion: https://postgr.es/m/CAApHDvqYVORiZxq2xPvP6_ndmmsTkvr6jSYv4UTNaFa5i1kd%3DQ%40mail.gmail.com
* Allow incremental sorts for windowing functionsDavid Rowley2020-09-15
| | | | | | | | | This expands on the work done in d2d8a229b and allows incremental sort to be considered during create_window_paths(). Author: David Rowley Reviewed-by: Daniel Gustafsson, Tomas Vondra Discussion: https://postgr.es/m/CAApHDvoOHobiA2x13NtWnWLcTXYj9ddpCkv9PnAJQBMegYf_xw%40mail.gmail.com
* Fix compiler warningDavid Rowley2020-09-15
| | | | | | | | | | | | Introduced in 0aa8f7640. MSVC warned about performing 32-bit bit shifting when it appeared like we might like a 64-bit result. We did, but it just so happened that none of the calls to this function could have caused the 32-bit shift to overflow. Here we just cast the constant to int64 to make the compiler happy. Discussion: https://postgr.es/m/CAApHDvofA_vsrpC13mq_hZyuye5B-ssKEaer04OouXYCO5-uXQ@mail.gmail.com
* Make walsenders show their replication commands in pg_stat_activity.Tom Lane2020-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | A walsender process that has executed a SQL command left the text of that command in pg_stat_activity.query indefinitely, which is quite confusing if it's in RUNNING state but not doing that query. An easy and useful fix is to treat replication commands as if they were SQL queries, and show them in pg_stat_activity according to the same rules as for regular queries. While we're at it, it seems also sensible to set debug_query_string, allowing error logging and debugging to see the replication command. While here, clean up assorted silliness in exec_replication_command: * The SQLCmd path failed to restore CurrentMemoryContext to the caller's value, and failed to delete the temp context created in this routine. It's only through great good fortune that these oversights did not result in long-term memory leaks or other problems. It seems cleaner to code SQLCmd as a separate early-exit path, so do it like that. * Remove useless duplicate call of SnapBuildClearExportedSnapshot(). * replication_scanner_finish() was never called. None of those things are significant enough to merit a backpatch, so this is for HEAD only. Discussion: https://postgr.es/m/880181.1600026471@sss.pgh.pa.us
* Fix interpolation in test name.Noah Misch2020-09-13
| | | | | | | A pre-commit review had reported the problem, but the fix reached only v10 and earlier. Back-patch to v11. Discussion: https://postgr.es/m/20200423.140546.1055476118690602079.horikyota.ntt@gmail.com
* Fix typos.Fujii Masao2020-09-14
| | | | | Author: Naoki Nakamichi Discussion: https://postgr.es/m/b6919d145af00295a8e86ce4d034b7cd@oss.nttdata.com
* Make index_set_state_flags() transactionalMichael Paquier2020-09-14
| | | | | | | | | | | | | | | | | | | | | | | | 3c84046 is the original commit that introduced index_set_state_flags(), where the presence of SnapshotNow made necessary the use of an in-place update. SnapshotNow has been removed in 813fb03, so there is no actual reasons to not make this operation transactional. Note that while making the operation more robust, using a transactional operation in this routine was not strictly necessary as there was no use case for it yet. However, some future features are going to need a transactional behavior, like support for CREATE/DROP INDEX CONCURRENTLY with partitioned tables, where indexes in a partition tree need to have all their pg_index.indis* flags updated in the same transaction to make the operation stable to the end-user by keeping partition trees consistent, even with a failure mid-flight. REINDEX CONCURRENTLY uses already transactional updates when swapping the old and new indexes, making this change more consistent with the index-swapping logic. Author: Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/20200903080440.GA8559@paquier.xyz
* Message fixes and style improvementsPeter Eisentraut2020-09-14
|
* Avoid useless allocations for information of dumpable objects in pg_dump/Michael Paquier2020-09-14
| | | | | | | | | | | | | | | | | If there are no objects of a certain type, there is no need to do an allocation for a set of DumpableObject items. The previous coding did an allocation of 1 byte instead as per the fallback of pg_malloc() in the event of an allocation size of zero. This assigns NULL instead for a set of dumpable objects. A similar rule already applied to findObjectByOid(), so this makes the code more defensive as we would just fail with a pointer dereference instead of attempting to use some incorrect data if a non-existing, positive, OID is given by a caller of this function. Author: Daniel Gustafsson Reviewed-by: Julien Rouhaud, Ranier Vilela Discussion: https://postgr.es/m/26C43E58-BDD0-4F1A-97CC-4A07B52E32C5@yesql.se
* Use the properly transformed RangeVar for expandTableLikeClause().Tom Lane2020-09-13
| | | | | | | | | | | | | | | transformCreateStmt() adjusts the transformed statement's RangeVar to specify the target schema explicitly, for the express reason of making sure that auxiliary statements derived by parse transformation operate on the right table. But the refactoring I did in commit 502898192 got this wrong and passed the untransformed RangeVar to expandTableLikeClause(). This could lead to assertion failures or weird misbehavior if the wrong table was accessed. Per report from Alexander Lakhin. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/05051f9d-b32b-cb35-6735-0e9f2ab86b5f@gmail.com
* Fix inconsistency in determining the timestamp of the db statfile.Amit Kapila2020-09-12
| | | | | | | | | | | | | | | We use the timestamp of the global statfile if we are not able to determine it for a particular database in case the entry for that database doesn't exist. However, we were using it even when the statfile is corrupt. As there is no user reported issue and it is not clear if there is any impact of this on actual application so decided not to backpatch. Reported-by: Amit Kapila Author: Amit Kapila Reviewed-by: Sawada Masahiko, Magnus Hagander and Alvaro Herrera Discussion: https://postgr.es/m/CAA4eK1J3oTJKyVq6v7K4d3jD+vtnruG9fHRib6UuWWsrwAR6Aw@mail.gmail.com
* Remove unused function declaration in logicalproto.h.Amit Kapila2020-09-12
| | | | | | | | | In the passing, fix a typo in pgoutput.c. Reported-by: Tomas Vondra Author: Tomas Vondra Reviewed-by: Dilip Kumar Discussion: https://postgr.es/m/20200909084353.pncuclpbwlr7vylh@development
* logtape.c: do not preallocate for tapes when sortingJeff Davis2020-09-11
| | | | | | | | | | | The preallocation logic is only useful for HashAgg, so disable it when sorting. Also, adjust an out-of-date comment. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wzn_o7tE2+hRVvwSFghRb75AJ5g-nqGzDUqLYMexjOAe=g@mail.gmail.com Backpatch-through: 13
* Accept SIGQUIT during error recovery in auxiliary processes.Tom Lane2020-09-11
| | | | | | | | | | | | | | | | | | | | | | | | The bgwriter, checkpointer, walwriter, and walreceiver processes claimed to allow SIGQUIT "at all times". In reality SIGQUIT would get re-blocked during error recovery, because we didn't update the actual signal mask immediately, so sigsetjmp() would save and reinstate a mask that includes SIGQUIT. This appears to be simply a coding oversight. There's never a good reason to hold off SIGQUIT in these processes, because it's going to just call _exit(2) which should be safe enough, especially since the postmaster is going to tear down shared memory afterwards. Hence, stick in PG_SETMASK() calls to install the modified BlockSig mask immediately. Also try to improve the comments around sigsetjmp blocks. Most of them were just referencing postgres.c, which is misleading because actually postgres.c manages the signals differently. No back-patch, since there's no evidence that this is causing any problems in the field. Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
* psql: Display stats target of extended statisticsAlvaro Herrera2020-09-11
| | | | | | | | | | The stats target can be set since commit d06215d03, but wasn't shown by psql. Author: Justin Pryzby <justin@telsasoft.com> Discussion: https://postgr.es/m/20200831050047.GG5450@telsasoft.com Reviewed-by: Georgios Kokolatos <gkokolatos@protonmail.com> Reviewed-by: Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
* Log a message when resorting to SIGKILL during shutdown/crash recovery.Tom Lane2020-09-11
| | | | | | | | | | Currently, no useful trace is left in the logs when the postmaster is forced to use SIGKILL to shut down children that failed to respond to SIGQUIT. Some questions were raised about how often that scenario happens in the buildfarm, so let's add a LOG-level message showing that it happened. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* Don't run atexit callbacks during signal exits from ProcessStartupPacket.Tom Lane2020-09-11
| | | | | | | | | | | | | | | | | | | Although 58c6feccf fixed the case for SIGQUIT, we were still calling proc_exit() from signal handlers for SIGTERM and timeout failures in ProcessStartupPacket. Fortunately, at the point where that code runs, we haven't yet connected to shared memory in any meaningful way, so there is nothing we need to undo in shared memory. This means it should be safe to use _exit(1) here, ie, not run any atexit handlers but also inform the postmaster that it's not a crash exit. To make sure nobody breaks the "nothing to undo" expectation, add a cross-check that no on-shmem-exit or before-shmem-exit handlers have been registered yet when we finish using these signal handlers. This change is simple enough that maybe it could be back-patched, but I won't risk that right now. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* Update copyright yearAlvaro Herrera2020-09-11
| | | | | | | Thinko in 40b3e2c201af. Reported-by: "Wang, Shenhao" <wangsh.fnst@cn.fujitsu.com> Discussion: https://postgr.es/m/ed98706b82694b57a8c0d339a10732aa@G08CNEXMBPEKD06.g08.fujitsu.local
* Print WAL logical message contents in pg_waldumpAlvaro Herrera2020-09-10
| | | | | | | | | This helps debuggability when looking at WAL streams containing logical messages. Author: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CAExHW5sWx49rKmXbg5H1Xc1t+nRv9PaYKQmgw82HPt6vWDVmDg@mail.gmail.com
* Use _exit(2) for SIGQUIT during ProcessStartupPacket, too.Tom Lane2020-09-10
| | | | | | | | | | | | | | | | | | | | | Bring the signal handling for startup-packet collection into line with the policy established in commits bedadc732 and 8e19a8264, namely don't risk running atexit callbacks when handling SIGQUIT. Ideally, we'd not do so for SIGTERM or timeout interrupts either, but that change seems a bit too risky for the back branches. For now, just improve the comments in this area to describe the risk. Also relocate where BackendInitialize re-disables these interrupts, to minimize the code span where they're active. This doesn't buy a whole lot of safety, but it can't hurt. In passing, rename startup_die() to remove confusion about whether it is for the startup process. Like the previous commits, back-patch to all supported branches. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* New contrib module, pg_surgery, with heap surgery functions.Robert Haas2020-09-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Sometimes it happens that the visibility information for a tuple becomes corrupted, either due to bugs in the database software or external factors. Provide a function heap_force_kill() that can be used to truncate such dead tuples to dead line pointers, and a function heap_force_freeze() that can be used to overwrite the visibility information in such a way that the tuple becomes all-visible. These functions are unsafe, in that you can easily use them to corrupt a database that was not previously corrupted, and you can use them to further corrupt an already-corrupted database or to destroy data. The documentation accordingly cautions against casual use. However, in some cases they permit recovery of data that would otherwise be very difficult to recover, or to allow a system to continue to function when it would otherwise be difficult to do so. Because we may want to add other functions for performing other kinds of surgery in the future, the new contrib module is called pg_surgery rather than something specific to these functions. I proposed back-patching this so that it could be more easily used by people running existing releases who are facing these kinds of problems, but that proposal did not attract enough support, so no back-patch for now. Ashutosh Sharma, reviewed and tested by Andrey M. Borodin, M. Beena Emerson, Masahiko Sawada, Rajkumar Raghuwanshi, Asim Praveen, and Mark Dilger, and somewhat revised by me. Discussion: http://postgr.es/m/CA+TgmoZW1fsU-QUNCRUQMGUygBDPVeOTLCqRdQZch=EYZnctSA@mail.gmail.com
* Remove unused parameterPeter Eisentraut2020-09-10
| | | | | | | Apparently, this was never used when introduced (3dad73e71f08abd86564d5090a58ca71740e07e0). Discussion: https://www.postgresql.org/message-id/flat/511bb100-f829-ba21-2f10-9f952ec06ead%402ndquadrant.com
* Add libpq's openssl dependencies to pkg-config filePeter Eisentraut2020-09-10
| | | | | | | | | Add libssl and libcrypto to libpq.pc's Requires.private. This allows static linking to work if those libssl or libcrypto themselves have dependencies in their *.private fields, such as -lz in some cases. Reported-by: Sandro Mani <manisandro@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/837d1dcf-2fca-ee6e-0d7e-6bce1a1bac75@gmail.com
* Add more tests for EXTRACT of date typePeter Eisentraut2020-09-10
| | | | | | | | | | | EXTRACT of date type is implemented as a wrapper around EXTRACT of timestamp, so the code is already tested there. But the externally visible behavior of EXTRACT on date is not recorded anywhere. Since there is some discussion about reimplementing or refactoring some of this, add some more explicit tests of EXTRACT on date, similar in structure to existing EXTRACT tests on other data types. Discussion: https://www.postgresql.org/message-id/flat/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
* Clean up some code and comments in partbounds.c.Etsuro Fujita2020-09-10
| | | | | | | | | | | Do some minor cleanup for commit c8434d64c: 1) remove a useless assignment (in normal builds) and 2) improve comments a little. Back-patch to v13 where the aforementioned commit went in. Author: Etsuro Fujita Reviewed-by: Alvaro Herrera Discussion: https://postgr.es/m/CAPmGK16yCd2R4=bQ4g8N2dT9TtA5ZU+qNmJ3LPc_nypbNy4_2A@mail.gmail.com
* doc: Fix some grammar and inconsistenciesMichael Paquier2020-09-10
| | | | | | | | Some comments are fixed while on it. Author: Justin Pryzby Discussion: https://postgr.es/m/20200818171702.GK17022@telsasoft.com Backpatch-through: 9.6
* Fix rd_firstRelfilenodeSubid for nailed relations, in parallel workers.Noah Misch2020-09-09
| | | | | | | | | | | Move applicable code out of RelationBuildDesc(), which nailed relations bypass. Non-assert builds experienced no known problems. Back-patch to v13, where commit c6b92041d38512a4176ed76ad06f713d2e6c01a8 introduced rd_firstRelfilenodeSubid. Kyotaro Horiguchi. Reported by Justin Pryzby. Discussion: https://postgr.es/m/20200907023737.GA7158@telsasoft.com
* Make archiver's SIGQUIT handler exit via _exit().Tom Lane2020-09-09
| | | | | | | | | | | | | | | | | | Commit 8e19a8264 changed the SIGQUIT handlers of almost all server processes not to run atexit callbacks. The archiver process was skipped, perhaps because it's not connected to shared memory; but it's just as true here that running atexit callbacks in a signal handler is unsafe. So let's make it work like the rest. In HEAD and v13, we can use the common SignalHandlerForCrashExit handler. Before that, just tweak pgarch_exit to use _exit(2) explicitly. Like the previous commit, back-patch to all supported branches. Kyotaro Horiguchi, back-patching by me Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* Expose internal function for converting int64 to numericPeter Eisentraut2020-09-09
| | | | | | | | Existing callers had to take complicated detours via DirectFunctionCall1(). This simplifies a lot of code. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
* Minor fixes in docs and error messages.Tom Lane2020-09-09
| | | | | | Alexander Lakhin Discussion: https://postgr.es/m/ce7debdd-c943-d7a7-9b41-687107b27831@gmail.com
* Add some more numeric test coveragePeter Eisentraut2020-09-09
| | | | | | max(numeric) wasn't tested at all, min(numeric) was only used by some unrelated tests. Add explicit tests with the other numeric aggregate functions.
* Check default partitions constraints while descendingAlvaro Herrera2020-09-08
| | | | | | | | | | | | | | | | | | | | | | | Partitioning tuple route code assumes that the partition chosen while descending the partition hierarchy is always the correct one. This is true except when the partition is the default partition and another partition has been added concurrently: the partition constraint changes and we don't recheck it. This can lead to tuples mistakenly being added to the default partition that should have been rejected. Fix by rechecking the default partition constraint while descending the hierarchy. An isolation test based on the reproduction steps described by Hao Wu (with tweaks for extra coverage) is included. Backpatch to 12, where this bug came in with 898e5e3290a7. Reported by: Hao Wu <hawu@vmware.com> Author: Amit Langote <amitlangote09@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqFqBmcSSap4sFnCBUEL_VfOMmEKaQ3gwUhyfa4c7J_-nA@mail.gmail.com Discussion: https://postgr.es/m/DM5PR0501MB3910E97A9EDFB4C775CF3D75A42F0@DM5PR0501MB3910.namprd05.prod.outlook.com
* Install an error check into cancel_before_shmem_exit().Tom Lane2020-09-08
| | | | | | | | | | | | | | | | | | | | | | | Historically, cancel_before_shmem_exit() just silently did nothing if the specified callback wasn't the top-of-stack. The folly of ignoring this case was exposed by the bugs fixed in 303640199 and bab150045, so let's make it throw elog(ERROR) instead. There is a decent argument to be made that PG_ENSURE_ERROR_CLEANUP should use some separate infrastructure, so it wouldn't break if something inside the guarded code decides to register a new before_shmem_exit callback. However, a survey of the surviving uses of before_shmem_exit() and PG_ENSURE_ERROR_CLEANUP doesn't show any plausible conflicts of that sort today, so for now we'll forgo the extra complexity. (It will almost certainly become necessary if anyone ever wants to wrap PG_ENSURE_ERROR_CLEANUP around arbitrary user-defined actions, though.) No backpatch, since this is developer support not a production issue. Bharath Rupireddy, per advice from Andres Freund, Robert Haas, and myself Discussion: https://postgr.es/m/CALj2ACWk7j4F2v2fxxYfrroOF=AdFNPr1WsV+AGtHAFQOqm_pw@mail.gmail.com
* Fix autovacuum cancellation.Andres Freund2020-09-08
| | | | | | | | | | | | | The problem is caused by me (Andres) having ProcSleep() look at the wrong PGPROC entry in 5788e258bb2. Unfortunately it seems hard to write a reliable test for autovacuum cancellations. Perhaps somebody will come up with a good approach, but it seems worth fixing the issue even without a test. Reported-By: Jeff Janes <jeff.janes@gmail.com> Author: Jeff Janes <jeff.janes@gmail.com> Discussion: https://postgr.es/m/CAMkU=1wH2aUy+wDRDz+5RZALdcUnEofV1t9PzXS_gBJO9vZZ0Q@mail.gmail.com
* Use plain memset() in numeric.c, not MemSet and friends.Tom Lane2020-09-08
| | | | | | | | | | | | | | | | | | | | This essentially reverts a micro-optimization I made years ago, as part of the much larger commit d72f6c750. It's doubtful that there was any hard evidence for it being helpful even then, and the case is even more dubious now that modern compilers are so much smarter about inlining memset(). The proximate reason for undoing it is to get rid of the type punning inherent in MemSet, for fear that that may cause problems now that we're applying additional optimization switches to numeric.c. At the very least this'll silence some warnings from a few old buildfarm animals. (It's probably past time for another look at whether MemSet is still worth anything at all, but I do not propose to tackle that question right now.) Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
* Use <unnamed> for name of unnamed portal's memory contextPeter Eisentraut2020-09-08
| | | | | | | Otherwise just printing an empty string makes the memory context debug output slightly confusing. Discussion: https://www.postgresql.org/message-id/flat/ccb353ef-89ff-09b3-8046-1d2514624b9c%402ndquadrant.com
* Remove isolation test reindex-partitionsMichael Paquier2020-09-08
| | | | | | | | | | | | | | | The isolation test added by a6642b3 is proving to be unstable, as once the first transaction holding a lock on the top-most partitioned table or on a partition commits, the commit order of the follow-up DROP TABLE and REINDEX could become reversed depending on the timing. The only part of the test that could be entirely reliable is the one using a SHARE lock, allowing REINDEX to commit first, but it is the least interesting of the set. Per buildfarm members rorqual and mylodon. Discussion: https://postgr.es/m/E1kFSBj-00062c-Mu@gemulon.postgresql.org
* Add support for partitioned tables and indexes in REINDEXMichael Paquier2020-09-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now, REINDEX was not able to work with partitioned tables and indexes, forcing users to reindex partitions one by one. This extends REINDEX INDEX and REINDEX TABLE so as they can accept a partitioned index and table in input, respectively, to reindex all the partitions assigned to them with physical storage (foreign tables, partitioned tables and indexes are then discarded). This shares some logic with schema and database REINDEX as each partition gets processed in its own transaction after building a list of relations to work on. This choice has the advantage to minimize the number of invalid indexes to one partition with REINDEX CONCURRENTLY in the event a cancellation or failure in-flight, as the only indexes handled at once in a single REINDEX CONCURRENTLY loop are the ones from the partition being working on. Isolation tests are added to emulate some cases I bumped into while developing this feature, particularly with the concurrent drop of a leaf partition reindexed. However, this is rather limited as LOCK would cause REINDEX to block in the first transaction building the list of partitions. Per its multi-transaction nature, this new flavor cannot run in a transaction block, similarly to REINDEX SCHEMA, SYSTEM and DATABASE. Author: Justin Pryzby, Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/db12e897-73ff-467e-94cb-4af03705435f.adger.lj@alibaba-inc.com
* Adjust cost model for HashAgg that spills to disk.Jeff Davis2020-09-07
| | | | | | | | | | | | Tomas Vondra observed that the IO behavior for HashAgg tends to be worse than for Sort. Penalize HashAgg IO costs accordingly. Also, account for the CPU effort of spilling the tuples and reading them back. Discussion: https://postgr.es/m/20200906212112.nzoy5ytrzjjodpfh@development Reviewed-by: Tomas Vondra Backpatch-through: 13
* Clarify comments in enforce_generic_type_consistency().Tom Lane2020-09-07
| | | | | | | | | | | | Some of the pre-existing comments were vague about whether they referred to all polymorphic types or only the old-style ones. Also be more consistent about using the "family 1" vs "family 2" terminology. Himanshu Upadhyaya and Tom Lane Discussion: https://postgr.es/m/CAPF61jBUg9XoMPNuLpoZ+h6UZ2VxKdNt3rQL1xw1GOBwjWzAXQ@mail.gmail.com