aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Add support for streaming to built-in logical replication.Amit Kapila2020-09-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | To add support for streaming of in-progress transactions into the built-in logical replication, we need to do three things: * Extend the logical replication protocol, so identify in-progress transactions, and allow adding additional bits of information (e.g. XID of subtransactions). * Modify the output plugin (pgoutput) to implement the new stream API callbacks, by leveraging the extended replication protocol. * Modify the replication apply worker, to properly handle streamed in-progress transaction by spilling the data to disk and then replaying them on commit. We however must explicitly disable streaming replication during replication slot creation, even if the plugin supports it. We don't need to replicate the changes accumulated during this phase, and moreover we don't have a replication connection open so we don't have where to send the data anyway. Author: Tomas Vondra, Dilip Kumar and Amit Kapila Reviewed-by: Amit Kapila, Kuntal Ghosh and Ajin Cherian Tested-by: Neha Sharma, Mahendra Singh Thalor and Ajin Cherian Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com
* Add string_to_table() function.Tom Lane2020-09-02
| | | | | | | | | | | | | | | This splits a string at occurrences of a delimiter. It is exactly like string_to_array() except for producing a set of values instead of an array of values. Thus, the relationship of these two functions is the same as between regexp_split_to_table() and regexp_split_to_array(). Although the same results could be had from unnest(string_to_array()), this is somewhat faster than that, and anyway it seems reasonable to have it for symmetry with the regexp functions. Pavel Stehule, reviewed by Peter Smith Discussion: https://postgr.es/m/CAFj8pRD8HOpjq2TqeTBhSo_QkzjLOhXzGCpKJ4nCs7Y9SQkuPw@mail.gmail.com
* Remove unused parameterPeter Eisentraut2020-09-02
| | | | | | unused since 39bd3fd1db6f3aa3764d4a1bebcd71c4e9c00281 Discussion: https://www.postgresql.org/message-id/flat/511bb100-f829-ba21-2f10-9f952ec06ead%402ndquadrant.com
* Add access method names to \d[i|m|t]+ in psqlMichael Paquier2020-09-02
| | | | | | | | | | | | | | | Listing a full set of relations with those psql meta-commands, without a matching pattern, has never showed the access method associated with each relation. This commit adds the access method of tables, indexes and matviews, masking it for relation kinds where it does not apply. Note that when HIDE_TABLEAM is enabled, the information does not show up. This is available when connecting to a backend version of at least 12, where table AMs have been introduced. Author: Georgios Kokolatos Reviewed-by: Vignesh C, Michael Paquier, Justin Pryzby Discussion: https://postgr.es/m/svaS1VTOEscES9CLKVTeKItjJP1EEJuBhTsA0ESOdlnbXeQSgycYwVlliL5zt8Jwcfo4ATYDXtEqsExxjkSkkhCSTCL8fnRgaCAJdr0unUg=@protonmail.com
* Fix thinko with definition of REINDEXOPT_MISSING_OKMichael Paquier2020-09-02
| | | | | | | | This had no direct consequences, but let's be consistent and it would be confusing when adding new flags. Oversight in 1d65416. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20200902024148.GB20149@telsasoft.com
* Avoid unnecessary acquisition of SyncRepLock in transaction commit time.Fujii Masao2020-09-02
| | | | | | | | | | | | | | | | In SyncRepWaitForLSN() routine called in transaction commit time, SyncRepLock is necessary to atomically both check the shared sync_standbys_defined flag and operate the sync replication wait-queue. On the other hand, when the flag is false, the lock is not necessary because the wait-queue is not touched. But due to the changes by commit 48c9f49265, previously the lock was taken whatever the flag was. This could cause unnecessary performance overhead in every transaction commit time. Therefore this commit avoids that unnecessary aquisition of SyncRepLock. Author: Fujii Masao Reviewed-by: Asim Praveen, Masahiko Sawada, Discussion: https://postgr.es/m/20200406050332.nsscfqjzk2d57zyx@alap3.anarazel.de
* Fix typo in commentAlvaro Herrera2020-09-01
| | | | | | Introduced by 8b08f7d4820f; backpatch to 11. Discussion: https://postgr.es/m/20200812214918.GA30353@alvherre.pgsql
* Improve handling of dropped relations for REINDEX DATABASE/SCHEMA/SYSTEMMichael Paquier2020-09-02
| | | | | | | | | | | | | | | | | | | When multiple relations are reindexed, a scan of pg_class is done first to build the list of relations to work on. However the REINDEX logic has never checked if a relation listed still exists when beginning the work on it, causing for example sudden cache lookup failures. This commit adds safeguards against dropped relations for REINDEX, similarly to VACUUM or CLUSTER where we try to open the relation, ignoring it if it is missing. A new option is added to the REINDEX routines to control if a missed relation is OK to ignore or not. An isolation test, based on REINDEX SCHEMA, is added for the concurrent and non-concurrent cases. Author: Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/20200813043805.GE11663@paquier.xyz
* Improve test coverage of ginvacuum.c.Tom Lane2020-09-01
| | | | | | | | | | Add a test case that exercises vacuum's deletion of empty GIN posting pages. Since this is a temp table, it should now work reliably to delete a bunch of rows and immediately VACUUM. Before the preceding commit, this would not have had the desired effect, at least not in parallel regression tests. Discussion: https://postgr.es/m/3490536.1598629609@sss.pgh.pa.us
* Set cutoff xmin more aggressively when vacuuming a temporary table.Tom Lane2020-09-01
| | | | | | | | | | | | | | | | | | Since other sessions aren't allowed to look into a temporary table of our own session, we do not need to worry about the global xmin horizon when setting the vacuum XID cutoff. Indeed, if we're not inside a transaction block, we may set oldestXmin to be the next XID, because there cannot be any in-doubt tuples in a temp table, nor any tuples that are dead but still visible to some snapshot of our transaction. (VACUUM, of course, is never inside a transaction block; but we need to test that because CLUSTER shares the same code.) This approach allows us to always clean out a temp table completely during VACUUM, independently of concurrent activity. Aside from being useful in its own right, that simplifies building reproducible test cases. Discussion: https://postgr.es/m/3490536.1598629609@sss.pgh.pa.us
* Raise error on concurrent drop of partitioned indexAlvaro Herrera2020-09-01
| | | | | | | | | | | | | | | | | | We were already raising an error for DROP INDEX CONCURRENTLY on a partitioned table, albeit a different and confusing one: ERROR: DROP INDEX CONCURRENTLY must be first action in transaction Change that to throw a more comprehensible error: ERROR: cannot drop partitioned index \"%s\" concurrently Michael Paquier authored the test case for indexes on temporary partitioned tables. Backpatch to 11, where indexes on partitioned tables were added. Reported-by: Jan Mussler <jan.mussler@zalando.de> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/16594-d2956ca909585067@postgresql.org
* Teach libpq to handle arbitrary-length lines in .pgpass files.Tom Lane2020-09-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically there's been a hard-wired assumption here that no line of a .pgpass file could be as long as NAMEDATALEN*5 bytes. That's a bit shaky to start off with, because (a) there's no reason to suppose that host names fit in NAMEDATALEN, and (b) this figure fails to allow for backslash escape characters. However, it fails completely if someone wants to use a very long password, and we're now hearing reports of people wanting to use "security tokens" that can run up to several hundred bytes. Another angle is that the file is specified to allow comment lines, but there's no reason to assume that long comment lines aren't possible. Rather than guessing at what might be a more suitable limit, let's replace the fixed-size buffer with an expansible PQExpBuffer. That adds one malloc/free cycle to the typical use-case, but that's surely pretty cheap relative to the I/O this code has to do. Also, add TAP test cases to exercise this code, because there was no test coverage before. This reverts most of commit 2eb3bc588, as there's no longer a need for a warning message about overlength .pgpass lines. (I kept the explicit check for comment lines, though.) In HEAD and v13, this also fixes an oversight in 74a308cf5: there's not much point in explicit_bzero'ing the line buffer if we only do so in two of the three exit paths. Back-patch to all supported branches, except that the test case only goes back to v10 where src/test/authentication/ was added. Discussion: https://postgr.es/m/4187382.1598909041@sss.pgh.pa.us
* Fix the SharedFileSetUnregister API.Amit Kapila2020-09-01
| | | | | | | | | | | | Commit 808e13b282 introduced a few APIs to extend the existing Buffile interface. In SharedFileSetDeleteOnProcExit, it tries to delete the list element while traversing the list with 'foreach' construct which makes the behavior of list traversal unpredictable. Author: Amit Kapila Reviewed-by: Dilip Kumar Tested-by: Dilip Kumar and Neha Sharma Discussion: https://postgr.es/m/CAA4eK1JhLatVcQ2OvwA_3s0ih6Hx9+kZbq107cXVsSWWukH7vA@mail.gmail.com
* C comment: remove mention of use of t_hoff WAL structure memberBruce Momjian2020-08-31
| | | | | | | | Reported-by: Antonin Houska Discussion: https://postgr.es/m/21643.1595353537@antos Backpatch-through: 9.5
* Mark factorial operator, and postfix operators in general, as deprecated.Tom Lane2020-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | Per discussion, we're planning to remove parser support for postfix operators in order to simplify the grammar. So it behooves us to put out a deprecation notice at least one release before that. There is only one built-in postfix operator, ! for factorial. Label it deprecated in the docs and in pg_description, and adjust some examples that formerly relied on it. (The sister prefix operator !! is also deprecated. We don't really have to remove that one, but since we're suggesting that people use factorial() instead, it seems better to remove both operators.) Also state in the CREATE OPERATOR ref page that postfix operators in general are going away. Although this changes the initial contents of pg_description, I did not force a catversion bump; it doesn't seem essential. In v13, also back-patch 4c5cf5431, so that there's someplace for the <link>s to point to. Mark Dilger and John Naylor, with some adjustments by me Discussion: https://postgr.es/m/BE2DF53D-251A-4E26-972F-930E523580E9@enterprisedb.com
* Redefine pg_class.reltuples to be -1 before the first VACUUM or ANALYZE.Tom Lane2020-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, we've considered the state with relpages and reltuples both zero as indicating that we do not know the table's tuple density. This is problematic because it's impossible to distinguish "never yet vacuumed" from "vacuumed and seen to be empty". In particular, a user cannot use VACUUM or ANALYZE to override the planner's normal heuristic that an empty table should not be believed to be empty because it is probably about to get populated. That heuristic is a good safety measure, so I don't care to abandon it, but there should be a way to override it if the table is indeed intended to stay empty. Hence, represent the initial state of ignorance by setting reltuples to -1 (relpages is still set to zero), and apply the minimum-ten-pages heuristic only when reltuples is still -1. If the table is empty, VACUUM or ANALYZE (but not CREATE INDEX) will override that to reltuples = relpages = 0, and then we'll plan on that basis. This requires a bunch of fiddly little changes, but we can get rid of some ugly kluges that were formerly needed to maintain the old definition. One notable point is that FDWs' GetForeignRelSize methods will see baserel->tuples = -1 when no ANALYZE has been done on the foreign table. That seems like a net improvement, since those methods were formerly also in the dark about what baserel->tuples = 0 really meant. Still, it is an API change. I bumped catversion because code predating this change would get confused by seeing reltuples = -1. Discussion: https://postgr.es/m/F02298E0-6EF4-49A1-BCB6-C484794D9ACC@thebuild.com
* Reset indisreplident for an invalid index in DROP INDEX CONCURRENTLYMichael Paquier2020-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | A failure when dropping concurrently an index used in a replica identity could leave in pg_index an index marked as !indisvalid and indisreplident. Reindexing this index would switch back indisvalid to true, and if the replica identity of the parent relation was switched to use a different index, it would be possible to finish with more than one index marked as indisreplident. If that were to happen, this could mess up with the relation cache as an incorrect index could be used for the replica identity. Indexes marked as invalid are discarded as candidates for the replica identity, as of RelationGetIndexList(), so similarly to what is done with indisclustered, resetting indisreplident when the index is marked as invalid keeps things consistent. REINDEX CONCURRENTLY's swapping already resets the flag for the old index, while the new index inherits the value of the old index to-be-dropped, so only DROP INDEX was an issue. Even if this is a bug, the sequence able to reproduce a problem requires a failure while running DROP INDEX CONCURRENTLY, something unlikely going to happen in the field, so no backpatch is done. Author: Michael Paquier Reviewed-by: Dmitry Dolgov Discussion: https://postgr.es/m/20200827025721.GN2017@paquier.xyz
* Fix code for re-finding scan position in a multicolumn GIN index.Tom Lane2020-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | collectMatchBitmap() needs to re-find the index tuple it was previously looking at, after transiently dropping lock on the index page it's on. The tuple should still exist and be at its prior position or somewhere to the right of that, since ginvacuum never removes tuples but concurrent insertions could add one. However, there was a thinko in that logic, to the effect of expecting any inserted tuples to have the same index "attnum" as what we'd been scanning. Since there's no physical separation of tuples with different attnums, it's not terribly hard to devise scenarios where this fails, leading to transient "lost saved point in index" errors. (While I've duplicated this with manual testing, it seems impossible to make a reproducible test case with our available testing technology.) Fix by just continuing the scan when the attnum doesn't match. While here, improve the error message used if we do fail, so that it matches the wording used in btree for a similar case. collectMatchBitmap()'s posting-tree code path was previously not exercised at all by our regression tests. While I can't make a regression test that exhibits the bug, I can at least improve the code coverage here, so do that. The test case I made for this is an extension of one added by 4b754d6c1, so it only works in HEAD and v13; didn't seem worth trying hard to back-patch it. Per bug #16595 from Jesse Kinkead. This has been broken since multicolumn capability was added to GIN (commit 27cb66fdf), so back-patch to all supported branches. Discussion: https://postgr.es/m/16595-633118be8eef9ce2@postgresql.org
* Fix comment in procarray.cMichael Paquier2020-08-27
| | | | | | | | The description of GlobalVisDataRels was missing, GlobalVisCatalogRels being mentioned instead. Author: Jim Nasby Discussion: https://postgr.es/m/8e06c883-2858-1fd4-07c5-560c28b08dcd@amazon.com
* Suppress compiler warning in non-cassert builds.Tom Lane2020-08-26
| | | | | | Oversight in 808e13b28, reported by Bruce Momjian. Discussion: https://postgr.es/m/20200826160251.GB21909@momjian.us
* Add additional information in the vacuum error context.Amit Kapila2020-08-26
| | | | | | | | | | The additional information added will be an offset number for heap operations. This information will help us in finding the exact tuple due to which the error has occurred. Author: Mahendra Singh Thalor and Amit Kapila Reviewed-by: Sawada Masahiko, Justin Pryzby and Amit Kapila Discussion: https://postgr.es/m/CAKYtNApK488TDF4bMbw+1QH8HJf9cxdNDXquhU50TK5iv_FtCQ@mail.gmail.com
* Extend the BufFile interface.Amit Kapila2020-08-26
| | | | | | | | | | | | | | | | | | | | | | Allow BufFile to support temporary files that can be used by the single backend when the corresponding files need to be survived across the transaction and need to be opened and closed multiple times. Such files need to be created as a member of a SharedFileSet. Additionally, this commit implements the interface for BufFileTruncate to allow files to be truncated up to a particular offset and extends the BufFileSeek API to support the SEEK_END case. This also adds an option to provide a mode while opening the shared BufFiles instead of always opening in read-only mode. These enhancements in BufFile interface are required for the upcoming patch to allow the replication apply worker, to handle streamed in-progress transactions. Author: Dilip Kumar, Amit Kapila Reviewed-by: Amit Kapila Tested-by: Neha Sharma Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com
* Add regression test for pg_backend_memory_contexts.Fujii Masao2020-08-26
| | | | | | Author: Atsushi Torikoshi Reviewed-by: Michael Paquier, Fujii Masao Discussion: https://postgr.es/m/20200819135545.GC19121@paquier.xyz
* Move codes for pg_backend_memory_contexts from mmgr/mcxt.c to adt/mcxtfuncs.c.Fujii Masao2020-08-26
| | | | | | | | | | | Previously the codes for pg_backend_memory_contexts were in src/backend/utils/mmgr/mcxt.c. This commit moves them to src/backend/utils/adt/mcxtfuncs.c so that mcxt.c basically includes only the low-level interface for memory contexts. Author: Atsushi Torikoshi Reviewed-by: Michael Paquier, Fujii Masao Discussion: https://postgr.es/m/20200819135545.GC19121@paquier.xyz
* Prevent non-superusers from reading pg_backend_memory_contexts, by default.Fujii Masao2020-08-26
| | | | | | | | | | | | | pg_backend_memory_contexts view contains some internal information of memory contexts. Since exposing them to any users by default may cause security issue, this commit allows only superusers to read this view, by default, like we do for pg_shmem_allocations view. Bump catalog version. Author: Atsushi Torikoshi Reviewed-by: Michael Paquier, Fujii Masao Discussion: https://postgr.es/m/1414992.1597849297@sss.pgh.pa.us
* Fixup some misusages of bms_num_members()David Rowley2020-08-26
| | | | | | | | | | | It's a bit inefficient to test if a Bitmapset is empty by counting all the members and seeing if that number is zero. It's much better just to use bms_is_empty(). Likewise for checking if there are at least two members, just use bms_membership(), which does not need to do anything more after finding two members. Discussion: https://postgr.es/m/CAApHDvpvwm_QjbDOb5xga%2BKmX9XkN9xQavNGm3SvDbVnCYOerQ%40mail.gmail.com Reviewed-by: Tomas Vondra
* Improve the vacuum error context phase information.Amit Kapila2020-08-24
| | | | | | | | | | | | | | | We were displaying the wrong phase information for 'info' message in the index clean up phase because we were switching to the previous phase a bit early. We were also not displaying context information for heap phase unless the block number is valid which is fine for error cases but for messages at 'info' or lower error level it appears to be inconsistent with index phase information. Reported-by: Sawada Masahiko Author: Sawada Masahiko Reviewed-by: Amit Kapila Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/CA+fd4k4HcbhPnCs7paRTw1K-AHin8y4xKomB9Ru0ATw0UeTy2w@mail.gmail.com
* Avoid pushing quals down into sub-queries that have grouping sets.Tom Lane2020-08-22
| | | | | | | | | | | | | | | | | | | | | | | | | | The trouble with doing this is that an apparently-constant subquery output column isn't really constant if it is a grouping column that appears in only some of the grouping sets. A qual using such a column would be subject to incorrect const-folding after push-down, as seen in bug #16585 from Paul Sivash. To fix, just disable qual pushdown altogether if the sub-query has nonempty groupingSets. While we could imagine far less restrictive solutions, there is not much point in working harder right now, because subquery_planner() won't move HAVING clauses to WHERE within such a subquery. If the qual stays in HAVING it's not going to be a lot more useful than if we'd kept it at the outer level. Having said that, this restriction could be removed if we used a parsetree representation that distinguished such outputs from actual constants, which is something I hope to do in future. Hence, make the patch a minimal addition rather than integrating it more tightly (e.g. by renumbering the existing items in subquery_is_pushdown_safe's comment). Back-patch to 9.5 where grouping sets were introduced. Discussion: https://postgr.es/m/16585-9d8c340d23ade8c1@postgresql.org
* Fix ALTER TABLE's scheduling rules for AT_AddConstraint subcommands.Tom Lane2020-08-22
| | | | | | | | | | | | | | | | | | | | | | Commit 1281a5c90 rearranged the logic in this area rather drastically, and it broke the case of adding a foreign key constraint in the same ALTER that adds the pkey or unique constraint it depends on. While self-referential fkeys are surely a pretty niche case, this used to work so we shouldn't break it. To fix, reorganize the scheduling rules in ATParseTransformCmd so that a transformed AT_AddConstraint subcommand will be delayed into a later pass in all cases, not only when it's been spit out as a side-effect of parsing some other command type. Also tweak the logic so that we won't run ATParseTransformCmd twice while doing this. It seems to work even without that, but it's surely wasting cycles to do so. Per bug #16589 from Jeremy Evans. Back-patch to v13 where the new code was introduced. Discussion: https://postgr.es/m/16589-31c8d981ca503896@postgresql.org
* Fix handling of CREATE TABLE LIKE with inheritance.Tom Lane2020-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a CREATE TABLE command uses both LIKE and traditional inheritance, Vars in CHECK constraints and expression indexes that are absorbed from a LIKE parent table tended to get mis-numbered, resulting in wrong answers and/or bizarre error messages (though probably not any actual crashes, thanks to validation occurring in the executor). In v12 and up, the same could happen to Vars in GENERATED expressions, even in cases with no LIKE clause but multiple traditional-inheritance parents. The cause of the problem for LIKE is that parse_utilcmd.c supposed it could renumber such Vars correctly during transformCreateStmt(), which it cannot since we have not yet accounted for columns added via inheritance. Fix that by postponing processing of LIKE INCLUDING CONSTRAINTS, DEFAULTS, GENERATED, INDEXES till after we've performed DefineRelation(). The error with GENERATED and multiple inheritance is a simple oversight in MergeAttributes(); it knows it has to renumber Vars in inherited CHECK constraints, but forgot to apply the same processing to inherited GENERATED expressions (a/k/a defaults). Per bug #16272 from Tom Gottfried. The non-GENERATED variants of the issue are ancient, presumably dating right back to the addition of CREATE TABLE LIKE; hence back-patch to all supported branches. Discussion: https://postgr.es/m/16272-6e32da020e9a9381@postgresql.org
* Fix explain regression test failure.Fujii Masao2020-08-22
| | | | | | | | | | | | | | | | Commit 9d701e624f caused the regression test for EXPLAIN to fail on the buildfarm member prion. This happened because of instability of test output, i.e., in text format, whether "Planning:" line is output varies depending on the system state. This commit updated the regression test so that it ignores that "Planning:" line to produce more stable test output and get rid of the test failure. Back-patch to v13. Author: Fujii Masao Discussion: https://postgr.es/m/1803897.1598021621@sss.pgh.pa.us
* Rework EXPLAIN for planner's buffer usage.Fujii Masao2020-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit ce77abe63c allowed EXPLAIN (BUFFERS) to report the information on buffer usage during planning phase. However three issues were reported regarding this feature. (1) Previously, EXPLAIN option BUFFERS required ANALYZE. So the query had to be actually executed by specifying ANALYZE even when we want to see only the planner's buffer usage. This was inconvenient especially when the query was write one like DELETE. (2) EXPLAIN included the planner's buffer usage in summary information. So SUMMARY option had to be enabled to report that. Also this format was confusing. (3) The output structure for planning information was not consistent between TEXT format and the others. For example, "Planning" tag was output in JSON format, but not in TEXT format. For (1), this commit allows us to perform EXPLAIN (BUFFERS) without ANALYZE to report the planner's buffer usage. For (2), this commit changed EXPLAIN output so that the planner's buffer usage is reported before summary information. For (3), this commit made the output structure for planning information more consistent between the formats. Back-patch to v13 where the planner's buffer usage was allowed to be reported in EXPLAIN. Reported-by: Pierre Giraud, David Rowley Author: Fujii Masao Reviewed-by: David Rowley, Julien Rouhaud, Pierre Giraud Discussion: https://postgr.es/m/07b226e6-fa49-687f-b110-b7c37572f69e@dalibo.com
* Fix typos in comments.Fujii Masao2020-08-21
| | | | | | Author: Masahiko Sawada Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CA+fd4k4m9hFSrRLB3etPWO5_v5=MujVZWRtz63q+55hM0Dz25Q@mail.gmail.com
* Fix a few typos in JIT comments and READMEDavid Rowley2020-08-21
| | | | | | | Reviewed-by: Abhijit Menon-Sen Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CAApHDvobgmCs6CohqhKTUf7D8vffoZXQTCBTERo9gbOeZmvLTw%40mail.gmail.com Backpatch-through: 11, where JIT was added
* Revert "Make vacuum a bit more verbose to debug BF failure."Andres Freund2020-08-20
| | | | | | | | | | | This reverts commit 49967da65aec970fcda123acc681f1df5d70bfc6. Enough time has passed that we can be confident that 07f32fcd23a resolved the issue. Therefore we can remove the temporary debugging aids. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/E1k7tGP-0005V0-5k@gemulon.postgresql.org
* Acquire ProcArrayLock exclusively in ProcArrayClearTransaction.Andres Freund2020-08-19
| | | | | | | | | | | | | | | This corrects an oversight by me in 20729324078, which made ProcArrayClearTransaction() increment xactCompletionCount. That requires an exclusive lock, obviously. There's other approaches that avoid the exclusive acquisition, but given that a 2PC commit is fairly heavyweight, it doesn't seem worth doing so. I've not been able to measure a performance difference, unsurprisingly. I did add a comment documenting that we could do so, should it ever become a bottleneck. Reported-By: Tom Lane <tgl@sss.pgh.pa.us> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/1355915.1597794204@sss.pgh.pa.us
* Suppress unnecessary RelabelType nodes in yet more cases.Tom Lane2020-08-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit a477bfc1d fixed eval_const_expressions() to ensure that it didn't generate unnecessary RelabelType nodes, but I failed to notice that some other places in the planner had the same issue. Really noplace in the planner should be using plain makeRelabelType(), for fear of generating expressions that should be equal() to semantically equivalent trees, but aren't. An example is that because canonicalize_ec_expression() failed to be careful about this, we could end up with an equivalence class containing both a plain Const, and a Const-with-RelabelType representing exactly the same value. So far as I can tell this led to no visible misbehavior, but we did waste a bunch of cycles generating and evaluating "Const = Const-with-RelabelType" to prove such entries are redundant. Hence, move the support function added by a477bfc1d to where it can be more generally useful, and use it in the places where planner code previously used makeRelabelType. Back-patch to v12, like the previous patch. While I have no concrete evidence of any real misbehavior here, it's certainly possible that I overlooked a case where equivalent expressions that aren't equal() could cause a user-visible problem. In any case carrying extra RelabelType nodes through planning to execution isn't very desirable. Discussion: https://postgr.es/m/1311836.1597781384@sss.pgh.pa.us
* Add pg_backend_memory_contexts system view.Fujii Masao2020-08-19
| | | | | | | | | | | | | | | | | | | This view displays the usages of all the memory contexts of the server process attached to the current session. This information is useful to investigate the cause of backend-local memory bloat. This information can be also collected by calling MemoryContextStats(TopMemoryContext) via a debugger. But this technique cannot be uesd in some environments because no debugger is available there. And it outputs lots of text messages and it's not easy to analyze them. So, pg_backend_memory_contexts view allows us to access to backend-local memory contexts information more easily. Bump catalog version. Author: Atsushi Torikoshi, Fujii Masao Reviewed-by: Tatsuhito Kasahara, Andres Freund, Daniel Gustafsson, Robert Haas, Michael Paquier Discussion: https://postgr.es/m/72a656e0f71d0860161e0b3f67e4d771@oss.nttdata.com
* Fix race condition in snapshot caching when 2PC is used.Andres Freund2020-08-18
| | | | | | | | | | | | | | | | | | | | When preparing a transaction xactCompletionCount needs to be incremented, even though the transaction has not committed yet. Otherwise the snapshot used within the transaction otherwise can get reused outside of the prepared transaction. As GetSnapshotData() does not include the current xid when building a snapshot, reuse would not be correct. Somewhat surprisingly the regression tests only rarely show incorrect results without the fix. The reason for that is that often the snapshot's xmax will be >= the backend xid, yielding a snapshot that is correct, despite the bug. I'm working on a reliable test for the bug, but it seems worth seeing whether this fixes all the BF failures while I do. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/E1k7tGP-0005V0-5k@gemulon.postgresql.org
* Avoid non-constant format string argument to fprintf().Heikki Linnakangas2020-08-18
| | | | | | | | | As Tom Lane pointed out, it could defeat the compiler's printf() format string verification. Backpatch to v12, like that patch that introduced it. Discussion: https://www.postgresql.org/message-id/1069283.1597672779%40sss.pgh.pa.us
* snapshot scalability: cache snapshots using a xact completion counter.Andres Freund2020-08-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previous commits made it faster/more scalable to compute snapshots. But not building a snapshot is still faster. Now that GetSnapshotData() does not maintain RecentGlobal* anymore, that is actually not too hard: This commit introduces xactCompletionCount, which tracks the number of top-level transactions with xids (i.e. which may have modified the database) that completed in some form since the start of the server. We can avoid rebuilding the snapshot's contents whenever the current xactCompletionCount is the same as it was when the snapshot was originally built. Currently this check happens while holding ProcArrayLock. While it's likely possible to perform the check without acquiring ProcArrayLock, it seems better to do that separately / later, some careful analysis is required. Even with the lock this is a significant win on its own. On a smaller two socket machine this gains another ~1.03x, on a larger machine the effect is roughly double (earlier patch version tested though). If we were able to safely avoid the lock there'd be another significant gain on top of that. Author: Andres Freund <andres@anarazel.de> Reviewed-By: Robert Haas <robertmhaas@gmail.com> Reviewed-By: Thomas Munro <thomas.munro@gmail.com> Reviewed-By: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/20200301083601.ews6hz5dduc3w2se@alap3.anarazel.de
* Fix use-after-release issue in PL/SampleMichael Paquier2020-08-18
| | | | | Introduced in adbe62d0. Per buildfarm member prion, when using RELCACHE_FORCE_RELEASE.
* Add PL/Sample to src/test/modules/Michael Paquier2020-08-18
| | | | | | | | | | | | | | | | | PL/Sample is an example template of procedural-language handler. This can be used as a base to implement a custom PL, or as a facility to test APIs dedicated to PLs. Much more could be done in this module, like adding a simple validator, but this is left as future work. The documentation included originally some C code to understand the basics of PL handler implementation, but it was outdated, and not really helpful either if trying to implement a new procedural language, particularly when it came to the integration of a PL installation with CREATE EXTENSION. Author: Mark Wong Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/20200612172648.GA3327@2ndQuadrant.com
* Disable autovacuum for BRIN test tableAlvaro Herrera2020-08-17
| | | | | | | | This should improve stability in the tests. Per buildfarm member hyrax (CLOBBER_CACHE_ALWAYS) via Tom Lane. Discussion: https://postgr.es/m/871534.1597503261@sss.pgh.pa.us
* Mark commit and abort WAL records with XLR_SPECIAL_REL_UPDATE.Heikki Linnakangas2020-08-17
| | | | | | | | | | | | | | | | If a commit or abort record includes "dropped relfilenodes", then replaying the record will remove data files. That is surely a "special rel update", but the records were not marked as such. Fix that, teach pg_rewind to expect and ignore them, and add a test case to cover it. It's always been like this, but no backporting for fear of breaking existing applications. If an application parsed the WAL but was not handling commit/abort records, it would stop working. That might be a good thing if it really needed to handle the dropped rels, but it will be caught when the application is updated to work with PostgreSQL v14 anyway. Discussion: https://www.postgresql.org/message-id/07b33e2c-46a6-86a1-5f9e-a7da73fddb95%40iki.fi Reviewed-by: Amit Kapila, Michael Paquier
* Make xact.h usable in frontend.Heikki Linnakangas2020-08-17
| | | | | | | | xact.h included utils/datetime.h, which cannot be used in the frontend (it includes fmgr.h, which needs Datum). But xact.h only needs the definition of TimestampTz from it, which is available directly in datatypes/timestamp.h. Change xact.h to include that instead of utils/datetime.h, so that it can be used in client programs.
* Fix printing last progress report line in client programs.Heikki Linnakangas2020-08-17
| | | | | | | | | | | | | | | | | A number of client programs have a "--progress" option that when printing to a TTY, updates the current line by printing a '\r' and overwriting it. After the last line, '\n' needs to be printed to move the cursor to the next line. pg_basebackup and pgbench got this right, but pg_rewind and pg_checksums were slightly wrong. pg_rewind printed the newline to stdout instead of stderr, and pg_checksums printed the newline even when not printing to a TTY. Fix them, and also add a 'finished' argument to pg_basebackup's progress_report() function, to keep it consistent with the other programs. Backpatch to v12. pg_rewind's newline was broken with the logging changes in commit cc8d415117 in v12, and pg_checksums was introduced in v12. Discussion: https://www.postgresql.org/message-id/82b539e5-ae33-34b0-1aee-22b3379fd3eb@iki.fi
* Fix use of wrong index in ComputeXidHorizons().Andres Freund2020-08-16
| | | | | | | | | | | | | | | | This bug, recently introduced in 941697c3c1a, at least lead to vacuum failing because it found tuples inserted by a running transaction, but below the freeze limit. The freeze limit in turn is directly affected by the aforementioned bug. Thanks to Tom Lane figuring how to make the bug reproducible. We should add a few more assertions to make sure this type of bug isn't as hard to notice, but it's not yet clear how to best do so. Co-Diagnosed-By: Tom Lane <tgl@sss.pgh.pa.us> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/1013484.1597609043@sss.pgh.pa.us
* Make vacuum a bit more verbose to debug BF failure.Andres Freund2020-08-16
| | | | | | | This is temporary. While possibly some more error checking / debugging in this path would be a good thing, it'll not look exactly like this. Discussion: https://postgr.es/m/20200816181604.l54m6kss5ntd6xow@alap3.anarazel.de
* Correct several behavior descriptions in comments.Noah Misch2020-08-15
| | | | | | | | | Reuse cautionary language from src/test/ssl/README in src/test/kerberos/README. SLRUs have had access to six-character segments names since commit 73c986adde5d73a5e2555da9b5c8facedb146dcd, and recovery stopped calling HeapTupleHeaderAdvanceLatestRemovedXid() in commit 558a9165e081d1936573e5a7d576f5febd7fb55a. The other corrections are more self-evident.