aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Skip unnecessary stat() calls in walkdir().Thomas Munro2020-09-07
| | | | | | | | | | | Some kernels can tell us the type of a "dirent", so we can avoid a call to stat() or lstat() in many cases. Define a new function get_dirent_type() to contain that logic, for use by the backend and frontend versions of walkdir(), and perhaps other callers in future. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BFzxupGGN4GpUdbzZN%2Btn6FQPHo8w0Q%2BAPH5Wz8RG%2Bww%40mail.gmail.com
* Add additional tests to test streaming of in-progress transactions.Amit Kapila2020-09-07
| | | | | | | | | | This covers the functionality tests for streaming in-progress subtransactions, streaming transactions containing rollback to savepoints, and streaming transactions having DDLs. Author: Tomas Vondra, Amit Kapila and Dilip Kumar Reviewed-by: Dilip Kumar Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com
* Apply auto-vectorization to the inner loop of numeric multiplication.Tom Lane2020-09-06
| | | | | | | | | | | | | | | | | | | | | | | Compile numeric.c with -ftree-vectorize where available, and adjust the innermost loop of mul_var() so that it is amenable to being auto-vectorized. (Mainly, that involves making it process the arrays left-to-right not right-to-left.) Applying -ftree-vectorize actually makes numeric.o smaller, at least with my compiler (gcc 8.3.1 on x86_64), and it's a little faster too. Independently of that, fixing the inner loop to be vectorizable also makes things a bit faster. But doing both is a huge win for multiplications with lots of digits. For me, the numeric regression test is the same speed to within measurement noise, but numeric_big is a full 45% faster. We also looked into applying -funroll-loops, but that makes numeric.o bloat quite a bit, and the additional speed improvement is very marginal. Amit Khandekar, reviewed and edited a little by me Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
* Split Makefile symbol CFLAGS_VECTOR into two symbols.Tom Lane2020-09-06
| | | | | | | | | | | | | Replace CFLAGS_VECTOR with CFLAGS_UNROLL_LOOPS and CFLAGS_VECTORIZE, allowing us to distinguish whether we want to apply -funroll-loops, -ftree-vectorize, or both to a particular source file. Up to now the only consumer of the symbol has been checksum.c which wants both, so that there was no need to distinguish; but that's about to change. Amit Khandekar, reviewed and edited a little by me Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
* Remove arbitrary line length limits in pg_regress (plain and ECPG).Tom Lane2020-09-06
| | | | | | | | | | | | | | | | | | | | | | Refactor replace_string() to use a StringInfo for the modifiable string argument. This allows the string to be of indefinite size initially and/or grow substantially during replacement. The previous logic in convert_sourcefiles_in() had a hard-wired limit of 1024 bytes on any line in input/*.sql or output/*.out files. While we've not had reports of trouble yet, it'd surely have bit us someday. This also fixes replace_string() so it won't get into an infinite loop if the string-to-be-replaced is a substring of the replacement. That's unlikely to happen in current usage, but the function surely shouldn't depend on it. Also fix ecpg_filter() to use a StringInfo and thereby remove its hard limit of 300 bytes on the length of an ecpg source line. Asim Rama Praveen and Georgios Kokolatos, reviewed by Alvaro Herrera and myself Discussion: https://postgr.es/m/y9Dlk2QhiZ39DhaB1QE9mgZ95HcOQKZCNtGwN7XCRKMdBRBnX_0woaRUtTjloEp4PKA6ERmcUcfq3lPGfKPOJ5xX2TV-5WoRYyySeNHRzdw=@protonmail.com
* Refactor pg_get_line() to expose an alternative StringInfo-based API.Tom Lane2020-09-06
| | | | | | | | | | | | | | | | | Letting the caller provide a StringInfo to read into is helpful when the caller needs to merge lines or otherwise modify the data after it's been read. Notably, now the code added by commit 8f8154a50 can use pg_get_line_append() instead of having its own copy of that logic. A follow-on commit will also make use of this. Also, since StringInfo buffers are a minimum of 1KB long, blindly using pg_get_line() in a loop can eat a lot more memory than one would expect. I discovered for instance that commit e0f05cd5b caused initdb to consume circa 10MB to read postgres.bki, even though that's under 1MB worth of data. A less memory-hungry alternative is to re-use the same StringInfo for all lines and pg_strdup the results. Discussion: https://postgr.es/m/1315832.1599345736@sss.pgh.pa.us
* Fix typo in commentMagnus Hagander2020-09-06
| | | | Author: Hou, Zhijie
* Fix misleading error message about inconsistent moving-aggregate types.Tom Lane2020-09-06
| | | | | | | | | | | | | We reported the wrong types when complaining that an aggregate's moving-aggregate implementation is inconsistent with its regular implementation. This was wrong since the feature was introduced, so back-patch to all supported branches. Jeff Janes Discussion: https://postgr.es/m/CAMkU=1x808LH=LPhZp9mNSP0Xd1xDqEd+XeGcvEe48dfE6xV=A@mail.gmail.com
* Remove useless lstat() call in pg_rewind.Tom Lane2020-09-06
| | | | | | | | | | | | | | | This is duplicative of an lstat that was just done by the calling function (traverse_datadir), besides which we weren't really doing anything with the results. There's not much point in checking to see if someone removed the file since the previous lstat, since the FILE_ACTION_REMOVE code would have to deal with missing-file cases anyway. Moreover, the "exists = false" assignment was a dead store; nothing was done with that value later. A syscall saved is a syscall earned, so back-patch to 9.5 where this code was introduced. Discussion: https://postgr.es/m/1221796.1599329320@sss.pgh.pa.us
* Improve some ancient, crufty code in bootstrap + initdb.Tom Lane2020-09-05
| | | | | | | | | | | | | | | | | | | | | | | | At some point back in the last century, somebody felt that reading all of pg_type twice was cheaper, or at least easier, than using repalloc() to resize the Typ[] array dynamically. That seems like an entirely wacko proposition, so rewrite the code to do it the other way. (To add insult to injury, there were two not-quite-identical copies of said code.) initdb.c's readfile() function had the same disease of preferring to do double the I/O to avoid resizing its output array. Here, we can make things easier by using the just-invented pg_get_line() function to handle reading individual lines without a predetermined notion of how long they are. On my machine, it's difficult to detect any net change in the overall runtime of initdb from these changes; but they should help on slower buildfarm machines (especially since a buildfarm cycle involves a lot of initdb's these days). My attention was drawn to these places by scan-build complaints, but on inspection they needed a lot more work than just suppressing dead stores :-(
* Yet more elimination of dead stores and useless initializations.Tom Lane2020-09-05
| | | | | | | | | I'm not sure what tool Ranier was using, but the ones I contributed were found by using a newer version of scan-build than I tried before. Ranier Vilela and Tom Lane Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com
* Switch to multi-inserts when registering dependencies for many code pathsMichael Paquier2020-09-05
| | | | | | | | | | | | | | | | | This commit improves the dependency registrations by taking advantage of the preliminary work done in 63110c62, to group together the insertion of dependencies of the same type to pg_depend. With the current layer of routines available, and as only dependencies of the same type can be grouped, there are code paths still doing more than one multi-insert when it is necessary to register dependencies of multiple types (constraint and index creation are two cases doing that). While on it, this refactors some of the code to use ObjectAddressSet() when manipulating object addresses. Author: Daniel Gustafsson, Michael Paquier Reviewed-by: Andres Freund, Álvaro Herrera Discussion: https://postgr.es/m/20200807061619.GA23955@paquier.xyz
* Extend SQL function tests lightlyPeter Eisentraut2020-09-05
| | | | | | | | | | The basic tests that defined SQL functions didn't actually run the functions to see if they worked. Add that, and also fix a minor mistake in a function that was revealed by this. (This is not a question of test coverage, since there are other places where SQL functions are run, but it is a bit of a silly test design.) Discussion: https://www.postgresql.org/message-id/flat/1c11f1eb-f00c-43b7-799d-2d44132c02d7@2ndquadrant.com
* Fix typo in commentPeter Eisentraut2020-09-05
|
* Use multi-inserts for pg_dependMichael Paquier2020-09-05
| | | | | | | | | | | | | | | | | | | | | | | This is a follow-up of the work done in e3931d01. This case is a bit different than pg_attribute and pg_shdepend: the maximum number of items to insert is known in advance, but there is no need to handle pinned dependencies. Hence, the base allocation for slots is done based on the number of items and the maximum allowed with a cap at 64kB. Slots are initialized once used to minimize the overhead of the operation. The insertions can be done for dependencies of the same type. More could be done by grouping the insertion of multiple dependency types in a single batch. This is left as future work. Some of the multi-insert logic is also simplified for pg_shdepend, as per the feedback discussed for this specific patch. This also moves to indexing.h the variable capping the maximum amount of data that can be used at once for a multi-insert, instead of having separate definitions for pg_attribute, pg_depend and pg_shdepend. Author: Daniel Gustafsson, Michael Paquier Reviewed-by: Andres Freund, Álvaro Herrera Discussion: https://postgr.es/m/20200807061619.GA23955@paquier.xyz
* Make new authentication test case more robust.Tom Lane2020-09-04
| | | | | | | | | | | I happened to notice that the new test case I added in b55b4dad9 falls over if one runs "make check" repeatedly; though not in branches after v10. That's because it was assuming that tmp_check/pgpass wouldn't exist already. However, it's only been since v11 that the Makefiles forcibly remove all of tmp_check/ before starting a TAP run. This fix to unlink the file is therefore strictly necessary only in v10 ... but it seems wisest to do it across the board, rather than let the test rely on external logic to get the conditions right.
* Fix over-eager ping'ing in logical replication receiver.Tom Lane2020-09-04
| | | | | | | | | | | Commit 3f60f690f only partially fixed the broken-status-tracking issue in LogicalRepApplyLoop: we need ping_sent to have the same lifetime as last_recv_timestamp. The effects are much less serious than what that commit fixed, though. AFAICS this would just lead to extra ping requests being sent, once per second until the sender responds. Still, it's a bug, so backpatch to v10 as before. Discussion: https://postgr.es/m/959627.1599248476@sss.pgh.pa.us
* Remove still more useless assignments.Tom Lane2020-09-04
| | | | | | | | Fix some more things scan-build pointed to as dead stores. In some of these cases, rearranging the code a little leads to more readable code IMO. It's all cosmetic, though. Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com
* Fix bogus MaxAllocSize check in logtape.c.Jeff Davis2020-09-04
| | | | | | Reported-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wz=NZPZc3-fkdmvu=w2itx0PiB-G6QpxHXZOjuvFAzPdZw@mail.gmail.com Backpatch-through: 13
* Report expected contrecord length on mismatchAlvaro Herrera2020-09-04
| | | | | | | | When reading a WAL record fails to find continuation record(s) of the proper length, report what it expects, for clarity. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20200903212152.GA15319@alvherre.pgsql
* Remove some more useless assignments.Tom Lane2020-09-04
| | | | | | | | | Found with clang's scan-build tool. It also whines about a lot of other dead stores that we should *not* change IMO, either as a matter of style or future-proofing. But these places seem like clear oversights. Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com
* Collect attribute data on extension owned tables being dumpedAndrew Dunstan2020-09-04
| | | | | | | | | If this data is not collected, pg_dump segfaults if asked for column inserts. Fix by Fabrízio de Royes Mello Backpatch to release 12 where the bug was introduced.
* C comment: correct use of 64-"byte" cache line sizeBruce Momjian2020-09-04
| | | | | | | | Reported-by: Kelly Min Discussion: https://postgr.es/m/CAPSbxatOiQO90LYpSC3+svAU9-sHgDfEP4oFhcEUt_X=DqFA9g@mail.gmail.com Backpatch-through: 9.5
* Fix rare deadlock failure in create_am regression test.Tom Lane2020-09-04
| | | | | | | | | | | | | | | | | The "DROP ACCESS METHOD gist2" test will require locking the index to be dropped and then its table; while most ordinary operations lock a table first then its index. While no concurrent test scripts should be touching fast_emp4000, autovacuum might chance to be processing that table when the DROP runs, resulting in a deadlock failure. This is pretty rare but we see it in the buildfarm from time to time. To fix, acquire a lock on fast_emp4000 before issuing the DROP. Since the point of the exercise is mostly to prevent buildfarm failures, back-patch to 9.6 where this test was introduced. Discussion: https://postgr.es/m/839004.1599185607@sss.pgh.pa.us
* doc: Change table alias names to lower case in tutorial chapterPeter Eisentraut2020-09-04
| | | | | | | This is needlessly different from our usual style otherwise. Author: Jürgen Purtz <juergen@purtz.de> Discussion: https://www.postgresql.org/message-id/flat/158996922318.7035.10603922579567326239@wrigleys.postgresql.org
* Fix inline marking introduced in commit 464824323e.Amit Kapila2020-09-04
| | | | | | | | Forgot to add inline marking in changes_filename() declaration. In the passing, add inline marking for a similar function subxact_filename(). Reported-By: Nathan Bossart Discussion: https://postgr.es/m/E98FBE8F-B878-480D-A728-A60C6EED3047@amazon.com
* remove redundant initializationsBruce Momjian2020-09-03
| | | | | | | | | | Reported-by: Ranier Vilela Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com Author: Ranier Vilela Backpatch-through: master
* Remove variable "concurrent" from ReindexStmtMichael Paquier2020-09-04
| | | | | | | | | | This node already handles multiple options using a bitmask, so having a separate boolean flag is not necessary. This simplifies the code a bit with less arguments to give to the reindex routines, by replacing the boolean with an equivalent bitmask value. Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/20200902110326.GA14963@paquier.xyz
* Remove arbitrary restrictions on password length.Tom Lane2020-09-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch started out with the goal of harmonizing various arbitrary limits on password length, but after awhile a better idea emerged: let's just get rid of those fixed limits. recv_password_packet() has an arbitrary limit on the packet size, which we don't really need, so just drop it. (Note that this doesn't really affect anything for MD5 or SCRAM password verification, since those will hash the user's password to something shorter anyway. It does matter for auth methods that require a cleartext password.) Likewise remove the arbitrary error condition in pg_saslprep(). The remaining limits are mostly in client-side code that prompts for passwords. To improve those, refactor simple_prompt() so that it allocates its own result buffer that can be made as big as necessary. Actually, it proves best to make a separate routine pg_get_line() that has essentially the semantics of fgets(), except that it allocates a suitable result buffer and hence will never return a truncated line. (pg_get_line has a lot of potential applications to replace randomly-sized fgets buffers elsewhere, but I'll leave that for another patch.) I built pg_get_line() atop stringinfo.c, which requires moving that code to src/common/; but that seems fine since it was a poor fit for src/port/ anyway. This patch is mostly mine, but it owes a good deal to Nathan Bossart who pressed for a solution to the password length problem and created a predecessor patch. Also thanks to Peter Eisentraut and Stephen Frost for ideas and discussion. Discussion: https://postgr.es/m/09512C4F-8CB9-4021-B455-EF4C4F0D55A0@amazon.com
* Avoid lockup of a parallel worker when reporting a long error message.Tom Lane2020-09-03
| | | | | | | | | | | | | | | | | | | | | Because sigsetjmp() will restore the initial state with signals blocked, the code path in bgworker.c for reporting an error and exiting would execute that way. Usually this is fairly harmless; but if a parallel worker had an error message exceeding the shared-memory communication buffer size (16K) it would lock up, because it would wait for a resume-sending signal from its parallel leader which it would never detect. To fix, just unblock signals at the appropriate point. This can be shown to fail back to 9.6. The lack of parallel query infrastructure makes it difficult to provide a simple test case for 9.5; but I'm pretty sure the issue exists in some form there as well, so apply the code change there too. Vignesh C, reviewed by Bharath Rupireddy, Robert Haas, and myself Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
* Allow records to span multiple lines in pg_hba.conf and pg_ident.conf.Tom Lane2020-09-03
| | | | | | | | | | | | | | | | | A backslash at the end of a line now causes the next line to be appended to the current one (effectively, the backslash and newline are discarded). This allows long HBA entries to be created without legibility problems. While we're here, get rid of the former hard-wired length limit on pg_hba.conf lines, by using an expansible StringInfo buffer instead of a fixed-size local variable. Since the same code is used to read the ident map file, these changes apply there as well. Fabien Coelho, reviewed by Justin Pryzby and David Zhang Discussion: https://postgr.es/m/alpine.DEB.2.21.2003251906140.15243@pseudo
* 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