aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Remove obsolete information schema tablesPeter Eisentraut2019-10-25
| | | | | | | | | | | | | | | | | Remove SQL_LANGUAGES, which was eliminated in SQL:2008, and SQL_PACKAGES and SQL_SIZING_PROFILES, which were eliminated in SQL:2011. Since they were dropped by the SQL standard, the information in them was no longer updated and therefore no longer useful. This also removes the feature-package association information in sql_feature_packages.txt, but for the time begin we are keeping the information which features are in the Core package (that is, mandatory SQL features). Maybe at some point someone wants to invent a way to store that that does not involve using the "package" mechanism anymore. Discussion https://www.postgresql.org/message-id/flat/91334220-7900-071b-9327-0c6ecd012017%402ndquadrant.com
* Improve management of statement timeouts.Tom Lane2019-10-25
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit f8e5f156b added private state in postgres.c to track whether a statement timeout is running. This seems like bad design to me; timeout.c's private state should be the single source of truth about that. We already fixed one bug associated with failure to keep those states in sync (cf. be42015fc), and I've got little faith that we won't find more in future. So get rid of postgres.c's local variable by exposing a way to ask timeout.c whether a timeout is running. (Obviously, such an inquiry is subject to race conditions, but it seems fine for the purpose at hand.) To make get_timeout_active() as cheap as possible, add a flag in the per-timeout struct showing whether that timeout is active. This allows some small savings elsewhere in timeout.c, mainly elimination of unnecessary searches of the active_timeouts array. While at it, fix enable_statement_timeout to not call disable_timeout when statement_timeout is 0 and the timeout is not running. This avoids a useless deschedule-and-reschedule-timeouts cycle, which represents a significant savings (at least one kernel call) when there is any other active timeout. Right now, there usually isn't, but there are proposals around to change that. Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org
* Reset statement_timeout between queries of a multi-query string.Tom Lane2019-10-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, we started the timer (if StatementTimeout > 0) at the beginning of a simple-Query message and usually let it run until the end, so that the timeout limit applied to the entire query string, and intra-string changes of the statement_timeout GUC had no effect. But, confusingly, a COMMIT within the string would reset the state and allow a fresh timeout cycle to start with the current setting. Commit f8e5f156b changed the behavior of statement_timeout for extended query protocol, and as an apparently-unintended side effect, a change in the statement_timeout GUC during a multi-statement simple-Query message might have an effect immediately --- but only if it was going from "disabled" to "enabled". This is all pretty confusing, not to mention completely undocumented. Let's change things so that the timeout is always reset between queries of a multi-query string, whether they're transaction control commands or not. Thus the active timeout setting is applied to each query in the string, separately. This costs a few more cycles if statement_timeout is active, but it provides much more intuitive behavior, especially if one changes statement_timeout in one of the queries of the string. Also, add something to the documentation to explain all this. Per bug #16035 from Raj Mohite. Although this is a bug fix, I'm hesitant to back-patch it; conceivably somebody has worked out the old behavior and is depending on it. (But note that this change should make the behavior less restrictive in most cases, since the timeout will now be applied to shorter segments of code.) Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org
* Handle interrupts within a transaction context in REINDEX CONCURRENTLYMichael Paquier2019-10-25
| | | | | | | | | | | | | | | | | | | | | | | Phases 2 (building the new index) and 3 (validating the new index) checked for interrupts outside a transaction context, having as consequence to not release session-level locks taken on the parent relation and the old and new indexes processed. This could for example be triggered with statement_timeout and a bad timing, and would issue confusing error messages when shutting down the session still holding the locks (note that an assertion failure would be triggered first), on top of more issues with concurrent sessions trying to take a lock that would interfere with the SHARE UPDATE EXCLUSIVE locks hold here. This moves all the interruption checks inside a transaction context. Note that I have manually tested all interruptions to make sure that invalid indexes can be cleaned up properly. Partition indexes still have issues on their own with some missing dependency handling, which will be dealt with in a follow-up patch. Reported-by: Justin Pryzby Author: Michael Paquier Discussion: https://postgr.es/m/20191013025145.GC4475@telsasoft.com Backpatch-through: 12
* Fix typo in xlog.c.Fujii Masao2019-10-24
| | | | | | Author: Fujii Masao Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAHGQGwH7dtYvOZZ8c0AG5AJwH5pfiRdKaCptY1_RdHy0HYeRfQ@mail.gmail.com
* Acquire properly session-level lock on new index in REINDEX CONCURRENTLYMichael Paquier2019-10-23
| | | | | | | | | | | | | | | In the first transaction run for REINDEX CONCURRENTLY, a thinko in the existing logic caused two session locks to be taken on the old index, causing the session lock on the newly-created index to be missed. This made possible concurrent DDL commands (like ALTER INDEX) on the new index while REINDEX CONCURRENTLY was processing from the point where the first internal transaction committed. This issue has been discovered while digging into another bug. Author: Michael Paquier Discussion: https://postgr.es/m/20191021074323.GB1869@paquier.xyz Backpatch-through: 12
* Clean up properly error_context_stack in autovacuum worker on exceptionMichael Paquier2019-10-23
| | | | | | | | | | | | | | Any callback set would have no meaning in the context of an exception. As an autovacuum worker exits quickly in this context, this could be only an issue within EmitErrorReport(), where the elog hook is for example called. That's unlikely to going to be a problem, but let's be clean and consistent with other code paths handling exceptions. This is present since 2909419, which introduced autovacuum. Author: Ashwin Agrawal Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/CALfoeisM+_+dgmAdAOHAu0k-ZpEHHqSSG=GRf3pKJGm8OqWX0w@mail.gmail.com Backpatch-through: 9.4
* Fix commentPeter Eisentraut2019-10-22
| | | | | | The last argument of smgrextend() was renamed from isTemp to skipFsync in debcec7dc31a992703911a9953e299c8d730c778, but the comments at two call sites were not updated.
* Refactor jsonpath's compareDatetime()Alexander Korotkov2019-10-21
| | | | | | | | | | | | This commit refactors come ridiculous coding in compareDatetime(). Also, it provides correct cross-datatype comparison even when one of values overflows during cast. That eliminates dilemma on whether we should suppress overflow errors during cast. Reported-by: Tom Lane Discussion: https://postgr.es/m/32308.1569455803%40sss.pgh.pa.us Discussion: https://postgr.es/m/a5629d0c-8162-7559-16aa-0c8390d6ba5f%40postgrespro.ru Author: Nikita Glukhov, Alexander Korotkov
* Refactor timestamp2timestamptz_opt_error()Alexander Korotkov2019-10-21
| | | | | | | | | | | | While casting from timestamp to timestamptz we do timestamp2tm() then tm2timestamp(). This commit eliminates call to tm2timestamp(). Instead, it directly applies timezone offset to the original timestamp value. That makes upcoming datetime overflow handling in jsonpath easier. That should also save us some CPU cycles. Discussion: https://postgr.es/m/CAPpHfdvRPRh_mTGar5WmDeRZ%3DU5dOXHdxspYYD%3D76m3knNGjXA%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Tom Lane
* Update obsolete comment.Etsuro Fujita2019-10-21
| | | | | | | | | | | Commit b52b7dc25, which moved code creating PartitionBoundInfo in RelationBuildPartitionDesc() in partcache.c (relocated to partdesc.c afterwards) to partbounds.c, should have updated this, but didn't. Author: Etsuro Fujita Reviewed-by: Alvaro Herrera Backpatch-through: 12 Discussion: https://postgr.es/m/CAPmGK16Uxr%3DPatiGyaRwiQVLB7Y-GqbkK3AxRLVYzU0Czv%3DsEw%40mail.gmail.com
* Fix memory leak introduced in commit 7df159a620.Amit Kapila2019-10-21
| | | | | | | | | | | | | We memorize all internal and empty leaf pages in the 1st vacuum stage for gist indexes. They are used in the 2nd stage, to delete all the empty pages. There was a memory context page_set_context for this purpose, but we never used it. Reported-by: Amit Kapila Author: Dilip Kumar Reviewed-by: Amit Kapila Backpatch-through: 12, where it got introduced Discussion: https://postgr.es/m/CAA4eK1LGr+MN0xHZpJ2dfS8QNQ1a_aROKowZB+MPNep8FVtwAA@mail.gmail.com
* Fix most -Wundef warningsPeter Eisentraut2019-10-19
| | | | | | | | | | | | In some cases #if was used instead of #ifdef in an inconsistent style. Cleaning this up also helps when analyzing cases like 38d8dce61fff09daae0edb6bcdd42b0c7f10ebcd where this makes a difference. There are no behavior changes here, but the change in pg_bswap.h would prevent possible accidental misuse by third-party code. Discussion: https://www.postgresql.org/message-id/flat/3b615ca5-c595-3f1d-fdf7-a429e564f614%402ndquadrant.com
* Use standard compare_exchange loop style in ProcArrayGroupClearXid().Noah Misch2019-10-18
| | | | | | | | Besides style, this might improve performance in the contended case. Reviewed by Amit Kapila. Discussion: https://postgr.es/m/20191015035348.GA4166224@rfd.leadboat.com
* Remove last traces of heap_open/close in the treeMichael Paquier2019-10-19
| | | | | | | | | | | | | Since pluggable storage has been introduced, those two routines have been replaced by table_open/close, with some compatibility macros still present to allow extensions to compile correctly with v12. Some code paths using the old routines still remained, so replace them. Based on the discussion done, the consensus reached is that it is better to remove those compatibility macros so as nothing new uses the old routines, so remove also the compatibility macros. Discussion: https://postgr.es/m/20191017014706.GF5605@paquier.xyz
* Fix failure of archive recovery with recovery_min_apply_delay enabled.Fujii Masao2019-10-18
| | | | | | | | | | | | | | | | | | | | | | | | recovery_min_apply_delay parameter is intended for use with streaming replication deployments. However, the document clearly explains that the parameter will be honored in all cases if it's specified. So it should take effect even if in archive recovery. But, previously, archive recovery with recovery_min_apply_delay enabled always failed, and caused assertion failure if --enable-caasert is enabled. The cause of this problem is that; the ownership of recoveryWakeupLatch that recovery_min_apply_delay uses was taken only when standby mode is requested. So unowned latch could be used in archive recovery, and which caused the failure. This commit changes recovery code so that the ownership of recoveryWakeupLatch is taken even in archive recovery. Which prevents archive recovery with recovery_min_apply_delay from failing. Back-patch to v9.4 where recovery_min_apply_delay was added. Author: Fujii Masao Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com
* Make crash recovery ignore recovery_min_apply_delay setting.Fujii Masao2019-10-18
| | | | | | | | | | | | | | | | | | | In v11 or before, this setting could not take effect in crash recovery because it's specified in recovery.conf and crash recovery always starts without recovery.conf. But commit 2dedf4d9a8 integrated recovery.conf into postgresql.conf and which unexpectedly allowed this setting to take effect even in crash recovery. This is definitely not good behavior. To fix the issue, this commit makes crash recovery always ignore recovery_min_apply_delay setting. Back-patch to v12 where the issue was added. Author: Fujii Masao Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com Discussion: https://postgr.es/m/e445616d-023e-a268-8aa1-67b8b335340c@pgmasters.net
* Fix typoAlvaro Herrera2019-10-18
| | | | | | | Apparently while this code was being developed, ReindexRelationConcurrently operated on multiple relations. The version that was ultimately pushed doesn't, so this comment's use of plural is inaccurate.
* Update comments about progress reporting by index_dropAlvaro Herrera2019-10-18
| | | | | | | | Michaël Paquier complained that index_drop is requesting progress reporting for non-obvious reasons, so let's add a comment to explain why. Discussion: https://postgr.es/m/20191017010412.GH2602@paquier.xyz
* Fix timeout handling in logical replication workerMichael Paquier2019-10-18
| | | | | | | | | | | | | | | | The timestamp tracking the last moment a message is received in a logical replication worker was initialized in each loop checking if a message was received or not, causing wal_receiver_timeout to be ignored in basically any logical replication deployments. This also broke the ping sent to the server when reaching half of wal_receiver_timeout. This simply moves the initialization of the timestamp out of the apply loop to the beginning of LogicalRepApplyLoop(). Reported-by: Jehan-Guillaume De Rorthais Author: Julien Rouhaud Discussion: https://postgr.es/m/CAOBaU_ZHESFcWva8jLjtZdCLspMj7vqaB2k++rjHLY897ZxbYw@mail.gmail.com Backpatch-through: 10
* Fix minor bug in logical-replication walsender shutdownAlvaro Herrera2019-10-17
| | | | | | | | | | | | | | | | | Logical walsender should exit when it catches up with sending WAL during shutdown; but there was a rare corner case when it failed to because of a race condition that puts it back to wait for more WAL instead -- but since there wasn't any, it'd not shut down immediately. It would only continue the shutdown when wal_sender_timeout terminates the sleep, which causes annoying waits during shutdown procedure. Restructure the code so that we no longer forget to set WalSndCaughtUp in that case. This was an oversight in commit c6c333436. Backpatch all the way down to 9.4. Author: Craig Ringer, Álvaro Herrera Discussion: https://postgr.es/m/CAMsr+YEuz4XwZX_QmnX_-2530XhyAmnK=zCmicEnq1vLr0aZ-g@mail.gmail.com
* When restoring GUCs in parallel workers, show an error context.Thomas Munro2019-10-17
| | | | | | | | | | | Otherwise it can be hard to see where an error is coming from, when the parallel worker sets all the GUCs that it received from the leader. Bug #15726. Back-patch to 9.5, where RestoreGUCState() appeared. Reported-by: Tiago Anastacio Reviewed-by: Daniel Gustafsson, Tom Lane Discussion: https://postgr.es/m/15726-6d67e4fa14f027b3%40postgresql.org
* Fix bug that could try to freeze running multixacts.Thomas Munro2019-10-17
| | | | | | | | | | | | | Commits 801c2dc7 and 801c2dc7 made it possible for vacuum to try to freeze a multixact that is still running. That was prevented by a check, but raised an error. Repair. Back-patch all the way. Author: Nathan Bossart, Jeremy Schneider Reported-by: Jeremy Schneider Reviewed-by: Jim Nasby, Thomas Munro Discussion: https://postgr.es/m/DAFB8AFF-2F05-4E33-AD7F-FF8B0F760C17%40amazon.com
* Fix crash when reporting CREATE INDEX progressAlvaro Herrera2019-10-16
| | | | | | | | | | | | A race condition can make us try to dereference a NULL pointer to the PGPROC struct of a process that's already finished. That results in crashes during REINDEX CONCURRENTLY and CREATE INDEX CONCURRENTLY. This was introduced in ab0dfc961b6a, so backpatch to pg12. Reported by: Justin Pryzby Reviewed-by: Michaël Paquier Discussion: https://postgr.es/m/20191012004446.GT10470@telsasoft.com
* Refresh some incorrect links in pg_crc.c/hMichael Paquier2019-10-16
| | | | | Author: Vignesh C Discussion: https://postgr.es/m/CALDaNm0LPk9vTGTBPBRv0=fX=94o4r6-DuBbHNeCN2AH5bufLw@mail.gmail.com
* Use libc version as a collation version on glibc systems.Thomas Munro2019-10-16
| | | | | | | | | | | Using glibc's version string to detect potential collation definition changes is not 100% reliable, but it's better than nothing. Currently this affects only collations explicitly provided by "libc". More work will be needed to handle the default collation. Author: Thomas Munro, based on a suggestion from Christoph Berg Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/4b76c6d4-ae5e-0dc6-7d0d-b5c796a07e34%402ndquadrant.com
* Fix CLUSTER on expression indexes.Andres Freund2019-10-15
| | | | | | | | | | | | | | | | Since the introduction of different slot types, in 1a0586de3657, we create a virtual slot in tuplesort_begin_cluster(). While that looks right, it unfortunately doesn't actually work, as ExecStoreHeapTuple() is used to store tuples in the slot. Unfortunately no regression tests for CLUSTER on expression indexes existed so far. Fix the slot type, and add bare bones tests for CLUSTER on expression indexes. Reported-By: Justin Pryzby Author: Andres Freund Discussion: https://postgr.es/m/20191011210320.GS10470@telsasoft.com Backpatch: 12, like 1a0586de3657
* Update unicode.org URLsPeter Eisentraut2019-10-13
| | | | | Use https, consistent host name, remove references to ftp. Also update the URLs for CLDR, which has moved from Trac to GitHub.
* In the postmaster, rely on the signal infrastructure to block signals.Tom Lane2019-10-13
| | | | | | | | | | | | | | | | | | | | POSIX sigaction(2) can be told to block a set of signals while a signal handler executes. Make use of that instead of manually blocking and unblocking signals in the postmaster's signal handlers. This should save a few cycles, and it also prevents recursive invocation of signal handlers when many signals arrive in close succession. We have seen buildfarm failures that seem to be due to postmaster stack overflow caused by such recursion (exacerbated by a Linux PPC64 kernel bug). This doesn't change anything about the way that it works on Windows. Somebody might consider adjusting port/win32/signal.c to let it work similarly, but I'm not in a position to do that. For the moment, just apply to HEAD. Possibly we should consider back-patching this, but it'd be good to let it age awhile first. Discussion: https://postgr.es/m/14878.1570820201@sss.pgh.pa.us
* Fix dependency handling of column drop with partitioned tablesMichael Paquier2019-10-13
| | | | | | | | | | | | | | | | | | | | | | When dropping a column on a partitioned table which has one or more partitioned indexes, the operation was failing as dependencies with partitioned indexes using the column dropped were not getting removed in a way consistent with the columns involved across all the relations part of an inheritance tree. This commit refactors the code executing column drop so as all the columns from an inheritance tree to remove are gathered first, and dropped all at the end. This way, we let the dependency machinery sort out by itself the deletion of all the columns with the partitioned indexes across a partition tree. This issue has been introduced by 1d92a0c, so backpatch down to REL_12_STABLE. Author: Amit Langote, Michael Paquier Reviewed-by: Álvaro Herrera, Ashutosh Sharma Discussion: https://postgr.es/m/CA+HiwqE9kuBsZ3b5pob2-cvE8ofzPWs-og+g8bKKGnu6b4-yTQ@mail.gmail.com Backpatch-through: 12
* Fix use of term "verifier"Peter Eisentraut2019-10-12
| | | | | | | | | | | Within the context of SCRAM, "verifier" has a specific meaning in the protocol, per RFCs. The existing code used "verifier" differently, to mean whatever is or would be stored in pg_auth.rolpassword. Fix this by using the term "secret" for this, following RFC 5803. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/be397b06-6e4b-ba71-c7fb-54cae84a7e18%402ndquadrant.com
* Make crash recovery ignore restore_command and recovery_end_command settings.Fujii Masao2019-10-11
| | | | | | | | | | | | | | | | | | In v11 or before, those settings could not take effect in crash recovery because they are specified in recovery.conf and crash recovery always starts without recovery.conf. But commit 2dedf4d9a8 integrated recovery.conf into postgresql.conf and which unexpectedly allowed those settings to take effect even in crash recovery. This is definitely not good behavior. To fix the issue, this commit makes crash recovery always ignore restore_command and recovery_end_command settings. Back-patch to v12 where the issue was added. Author: Fujii Masao Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/e445616d-023e-a268-8aa1-67b8b335340c@pgmasters.net
* Fix table rewrites that include a column without a default.Andres Freund2019-10-09
| | | | | | | | | | | | | | In c2fe139c201c I made ATRewriteTable() use tuple slots. Unfortunately I did not notice that columns can be added in a rewrite that do not have a default, when another column is added/altered requiring one. Initialize columns to NULL again, and add tests. Bug: #16038 Reported-By: anonymous Author: Andres Freund Discussion: https://postgr.es/m/16038-5c974541f2bf6749@postgresql.org Backpatch: 12, where the bug was introduced in c2fe139c201c
* Revert "Use libc version as a collation version on glibc systems."Peter Eisentraut2019-10-09
| | | | | | This reverts commit 9f90b1d08d796a925808b24f77f624a0ff682c77. This needs some refinements in the pg_dump and pg_upgrade tests.
* Use libc version as a collation version on glibc systems.Peter Eisentraut2019-10-09
| | | | | | | | | Using glibc's version number to detect potential collation definition changes is not 100% reliable, but it's better than nothing. Author: Thomas Munro Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/4b76c6d4-ae5e-0dc6-7d0d-b5c796a07e34%402ndquadrant.com
* Flush logical mapping files with fd opened for read/write at checkpointMichael Paquier2019-10-09
| | | | | | | | | | | | | | | The file descriptor was opened with read-only to fsync a regular file, which would cause EBADFD errors on some platforms. This is similar to the recent fix done by a586cc4b (which was broken by me with 82a5649), except that I noticed this issue while monitoring the backend code for similar mistakes. Backpatch to 9.4, as this has been introduced since logical decoding exists as of b89e151. Author: Michael Paquier Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20191006045548.GA14532@paquier.xyz Backpatch-through: 9.4
* Remove some code for old unsupported versions of MSVCPeter Eisentraut2019-10-08
| | | | | | | | | | | | As of d9dd406fe281d22d5238d3c26a7182543c711e74, we require MSVC 2013, which means _MSC_VER >= 1800. This means that conditionals about older versions of _MSC_VER can be removed or simplified. Previous code was also in some cases handling MinGW, where _MSC_VER is not defined at all, incorrectly, such as in pg_ctl.c and win32_port.h, leading to some compiler warnings. This should now be handled better. Reviewed-by: Michael Paquier <michael@paquier.xyz>
* Update some outdated links about XLC and UNIX specificationMichael Paquier2019-10-08
| | | | | Author: Vignesh C Discussion: https://postgr.es/m/CALDaNm3Dy=dTdx8UCVw=DWbzLzmRUC1dkq45=heOZDUg3U_PtA@mail.gmail.com
* Check for too many postmaster children before spawning a bgworker.Tom Lane2019-10-07
| | | | | | | | | | | | | | | | | | | | 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
* Avoid trying to release a List's initial allocation via repalloc().Tom Lane2019-10-06
| | | | | | | | | | | | | | | | | | | | Commit 1cff1b95a 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 c477f3e44 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
* Change MemoryContextMemAllocated to return SizeTomas Vondra2019-10-05
| | | | | | | | Commit f2369bc610 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
* Fix crash caused by EPQ happening with a before update trigger present.Andres Freund2019-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 ff11e7f4b9ae 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 ff11e7f4b9ae was merged
* Use a fd opened for read/write when syncing slots during startup, take 2.Andres Freund2019-10-04
| | | | | | | | | | | | | | | | | | | | | | | Cribbing from dfbaed45975: 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 82a5649fb9db. 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 82a5649fb9db
* Rename some toasting functions based on whether they are heap-specific.Robert Haas2019-10-04
| | | | | | | | | | | | | | | | | | | | | | 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 8b94dab06617ef80a0901ab103ebd8754427ef5a 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
* Fix bitshiftright()'s zero-padding some more.Tom Lane2019-10-04
| | | | | | | | | | | | | | | Commit 5ac0d9360 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
* Use Size instead of int64 to track allocated memoryTomas Vondra2019-10-04
| | | | | | | | | | | | | | | | | | | | | Commit 5dd7fc1519 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 c477f3e449 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
* Remove AtSubStart_Notify.Robert Haas2019-10-04
| | | | | | | | | | 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
* Avoid unnecessary out-of-memory errors during encoding conversion.Tom Lane2019-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Allow repalloc() to give back space when a large chunk is downsized.Tom Lane2019-10-03
| | | | | | | | | | | | | | | | | | 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
* Selectively include window frames in expression walks/mutates.Andrew Gierth2019-10-03
| | | | | | | | | | | | | | | | | | | | 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 ec4be2ee6 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
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-05 07:38:47 +0000