aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Add condition variable for recovery resume.Thomas Munro2021-03-12
| | | | | | | | | Replace a sleep loop with a CV, to get a fast reaction time when recovery is resumed or the postmaster exits via standard infrastructure. Unfortunately we still need to wake up every second to perform extra polling during the recovery pause loop. Discussion: https://postgr.es/m/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com
* Send statistics collected during shutdown checkpoint to the stats collector.Fujii Masao2021-03-12
| | | | | | | | | | | | | | When shutdown is requested, checkpointer performs checkpoint or restartpoint, and updates the statistics, before it exits. But previously checkpointer didn't send those statistics to the stats collector. Shutdown checkpoint and restartpoint are treated as requested ones instead of scheduled ones, so the number of them are counted in pg_stat_bgwriter.checkpoints_req column. Author: Masahiro Ikeda Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/0509ad67b585a5b86a83d445dfa75392@oss.nttdata.com
* Force to send remaining WAL stats to the stats collector at walwriter exit.Fujii Masao2021-03-12
| | | | | | | | | | | | | | | | | | | In walwriter's main loop, WAL stats message is only sent if enough time has passed since last one was sent to reach PGSTAT_STAT_INTERVAL msecs. This is necessary to avoid overloading to the stats collector. But this can cause recent WAL stats to be unsent when walwriter exits. To ensure that all the WAL stats are sent, this commit makes walwriter force to send remaining WAL stats to the collector when it exits because of shutdown request. Note that those remaining WAL stats can still be unsent when walwriter exits with non-zero exit code (e.g., FATAL error). This is OK because that walwriter exit leads to server crash and subsequent recovery discards all the stats. So there is no need to send remaining stats in that case. Author: Masahiro Ikeda Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/0509ad67b585a5b86a83d445dfa75392@oss.nttdata.com
* Minor modernization for README.barrier.Thomas Munro2021-03-12
| | | | | Itanium is very uncommon and being discontinued. ARM is everywhere. Prefer ARM as an example of an architecture with weak memory ordering.
* Save a few cycles during nbtree VACUUM.Peter Geoghegan2021-03-11
| | | | | | | | | | | Avoid calling RelationGetNumberOfBlocks() unnecessarily in the common case where there are no deleted but not yet recycled pages to recycle during a cleanup-only nbtree VACUUM operation. Follow-up to commit e5d8a999, which (among other things) taught the "skip full scan" nbtree VACUUM mechanism to only trigger a full index scan when the absolute number of deleted pages in the index is considered excessive.
* Add back vacuum_cleanup_index_scale_factor parameter.Peter Geoghegan2021-03-11
| | | | | | | | | | | | | | | | | Commit 9f3665fb removed the vacuum_cleanup_index_scale_factor storage parameter. However, that creates dump/reload hazards when moving across major versions. Add back the vacuum_cleanup_index_scale_factor parameter (though not the GUC of the same name) purely to avoid problems when using tools like pg_upgrade. The parameter remains disabled and undocumented. No backpatch to Postgres 13, since vacuum_cleanup_index_scale_factor was only disabled by REL_13_STABLE's version of master branch commit 9f3665fb in the first place -- the parameter already looks like this on REL_13_STABLE. Discussion: https://postgr.es/m/YEm/a3Ko3nKnBuVq@paquier.xyz
* Be clear about whether a recovery pause has taken effect.Robert Haas2021-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the code and documentation seem to have essentially assumed than a call to pg_wal_replay_pause() would take place immediately, but that's not the case, because we only check for a pause in certain places. This means that a tool that uses this function and then wants to do something else afterward that is dependent on the pause having taken effect doesn't know how long it needs to wait to be sure that no more WAL is going to be replayed. To avoid that, add a new function pg_get_wal_replay_pause_state() which returns either 'not paused', 'paused requested', or 'paused'. After calling pg_wal_replay_pause() the status will immediate change from 'not paused' to 'pause requested'; when the startup process has noticed this, the status will change to 'pause'. For backward compatibility, pg_is_wal_replay_paused() still exists and returns the same thing as before: true if a pause has been requested, whether or not it has taken effect yet; and false if not. The documentation is updated to clarify. To improve the changes that a pause request is quickly confirmed effective, adjust things so that WaitForWALToBecomeAvailable will swiftly reach a call to recoveryPausesHere() when a pause request is made. Dilip Kumar, reviewed by Simon Riggs, Kyotaro Horiguchi, Yugo Nagata, Masahiko Sawada, and Bharath Rupireddy. Discussion: http://postgr.es/m/CAFiTN-vcLLWEm8Zr%3DYK83rgYrT9pbC8VJCfa1kY9vL3AUPfu6g%40mail.gmail.com
* Re-simplify management of inStart in pqParseInput3's subroutines.Tom Lane2021-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 92785dac2 copied some logic related to advancement of inStart from pqParseInput3 into getRowDescriptions and getAnotherTuple, because it wanted to allow user-defined row processor callbacks to potentially longjmp out of the library, and inStart would have to be updated before that happened to avoid an infinite loop. We later decided that that API was impossibly fragile and reverted it, but we didn't undo all of the related code changes, and this bit of messiness survived. Undo it now so that there's just one place in pqParseInput3's processing where inStart is advanced; this will simplify addition of better tracing support. getParamDescriptions had grown similar processing somewhere along the way (not in 92785dac2; I didn't track down just when), but it's actually buggy because its handling of corrupt-message cases seems to have been copied from the v2 logic where we lacked a known message length. The cases where we "goto not_enough_data" should not simply return EOF, because then we won't consume the message, potentially creating an infinite loop. That situation now represents a definitively corrupt message, and we should report it as such. Although no field reports of getParamDescriptions getting stuck in a loop have been seen, it seems appropriate to back-patch that fix. I chose to back-patch all of this to keep the logic looking more alike in supported branches. Discussion: https://postgr.es/m/2217283.1615411989@sss.pgh.pa.us
* Refactor and generalize the ParallelSlot machinery.Robert Haas2021-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | Create a wrapper object, ParallelSlotArray, to encapsulate the number of slots and the slot array itself, plus some other relevant bits of information. This reduces the number of parameters we have to pass around all over the place. Allow for a ParallelSlotArray to contain slots connected to different databases within a single cluster. The current clients of this mechanism don't need this, but it is expected to be used by future patches. Defer connecting to databases until we actually need the connection for something. This is a slight behavior change for vacuumdb and reindexdb. If you specify a number of jobs that is larger than the number of objects, the extra connections will now not be used. But, on the other hand, if you specify a number of jobs that is so large that it's going to fail, the failure would previously have happened before any operations were actually started, and now it won't. Mark Dilger, reviewed by me. Discussion: http://postgr.es/m/12ED3DA8-25F0-4B68-937D-D907CFBF08E7@enterprisedb.com Discussion: http://postgr.es/m/BA592F2D-F928-46FF-9516-2B827F067F57@enterprisedb.com
* Set libcrypto callbacks for all connection threads in libpqMichael Paquier2021-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Based on an analysis of the OpenSSL code with Jacob, moving to EVP for the cryptohash computations makes necessary the setup of the libcrypto callbacks that were getting set only for SSL connections, but not for connections without SSL. Not setting the callbacks makes the use of threads potentially unsafe for connections calling cryptohashes during authentication, like MD5 or SCRAM, if a failure happens during a cryptohash computation. The logic setting the libssl and libcrypto states is then split into two parts, both using the same locking, with libcrypto being set up for SSL and non-SSL connections, while SSL connections set any libssl state afterwards as needed. Prior to this commit, only SSL connections would have set libcrypto callbacks that are necessary to ensure a proper thread locking when using multiple concurrent threads in libpq (ENABLE_THREAD_SAFETY). Note that this is only required for OpenSSL 1.0.2 and 1.0.1 (oldest version supported on HEAD), as 1.1.0 has its own internal locking and it has dropped support for CRYPTO_set_locking_callback(). Tests with up to 300 threads with OpenSSL 1.0.1 and 1.0.2, mixing SSL and non-SSL connection threads did not show any performance impact after some micro-benchmarking. pgbench can be used here with -C and a mostly-empty script (with one \set meta-command for example) to stress authentication requests, and we have mixed that with some custom programs for testing. Reported-by: Jacob Champion Author: Michael Paquier Reviewed-by: Jacob Champion Discussion: https://postgr.es/m/fd3ba610085f1ff54623478cf2f7adf5af193cbb.camel@vmware.com
* Improve comment for struct BufferDesc.Thomas Munro2021-03-11
| | | | | | | | Add a note that per-buffer I/O condition variables currently live outside the BufferDesc struct. Follow-up for commit d8725104. Reported-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://postgr.es/m/20210311031118.hucytmrgwlktjxgq%40nol
* tutorial: land height is "elevation", not "altitude"Bruce Momjian2021-03-10
| | | | | | | | | | | | | | | This is a follow-on patch to 92c12e46d5. In that patch, we renamed "altitude" to "elevation" in the docs, based on these details: https://mapscaping.com/blogs/geo-candy/what-is-the-difference-between-elevation-relief-and-altitude This renames the tutorial SQL files to match the documentation. Reported-by: max1@inbox.ru Discussion: https://postgr.es/m/161512392887.1046.3137472627109459518@wrigleys.postgresql.org Backpatch-through: 9.6
* VACUUM ANALYZE: Always update pg_class.reltuples.Peter Geoghegan2021-03-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | vacuumlazy.c sometimes fails to update pg_class entries for each index (to ensure that pg_class.reltuples is current), even though analyze.c assumed that that must have happened during VACUUM ANALYZE. There are at least a couple of reasons for this. For example, vacuumlazy.c could fail to update pg_class when the index AM indicated that its statistics are merely an estimate, per the contract for amvacuumcleanup() routines established by commit e57345975cf back in 2006. Stop assuming that pg_class must have been updated with accurate statistics within VACUUM ANALYZE -- update pg_class for indexes at the same time as the table relation in all cases. That way VACUUM ANALYZE will never fail to keep pg_class.reltuples reasonably accurate. The only downside of this approach (compared to the old approach) is that it might inaccurately set pg_class.reltuples for indexes whose heap relation ends up with the same inaccurate value anyway. This doesn't seem too bad. We already consistently called vac_update_relstats() (to update pg_class) for the heap/table relation twice during any VACUUM ANALYZE -- once in vacuumlazy.c, and once in analyze.c. We now make sure that we call vac_update_relstats() at least once (though often twice) for each index. This is follow up work to commit 9f3665fb, which dealt with issues in btvacuumcleanup(). Technically this fixes an unrelated issue, though. btvacuumcleanup() no longer provides an accurate num_index_tuples value following commit 9f3665fb (when there was no btbulkdelete() call during the VACUUM operation in question), but hashvacuumcleanup() has worked in the same way for many years now. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAH2-WzknxdComjhqo4SUxVFk_Q1171GJO2ZgHZ1Y6pion6u8rA@mail.gmail.com Backpatch: 13-, just like commit 9f3665fb.
* Don't consider newly inserted tuples in nbtree VACUUM.Peter Geoghegan2021-03-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the entire idea of "stale stats" within nbtree VACUUM (stop caring about stats involving the number of inserted tuples). Also remove the vacuum_cleanup_index_scale_factor GUC/param on the master branch (though just disable them on postgres 13). The vacuum_cleanup_index_scale_factor/stats interface made the nbtree AM partially responsible for deciding when pg_class.reltuples stats needed to be updated. This seems contrary to the spirit of the index AM API, though -- it is not actually necessary for an index AM's bulk delete and cleanup callbacks to provide accurate stats when it happens to be inconvenient. The core code owns that. (Index AMs have the authority to perform or not perform certain kinds of deferred cleanup based on their own considerations, such as page deletion and recycling, but that has little to do with pg_class.reltuples/num_index_tuples.) This issue was fairly harmless until the introduction of the autovacuum_vacuum_insert_threshold feature by commit b07642db, which had an undesirable interaction with the vacuum_cleanup_index_scale_factor mechanism: it made insert-driven autovacuums perform full index scans, even though there is no real benefit to doing so. This has been tied to a regression with an append-only insert benchmark [1]. Also have remaining cases that perform a full scan of an index during a cleanup-only nbtree VACUUM indicate that the final tuple count is only an estimate. This prevents vacuumlazy.c from setting the index's pg_class.reltuples in those cases (it will now only update pg_class when vacuumlazy.c had TIDs for nbtree to bulk delete). This arguably fixes an oversight in deduplication-related bugfix commit 48e12913. [1] https://smalldatum.blogspot.com/2021/01/insert-benchmark-postgres-is-still.html Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAD21AoA4WHthN5uU6+WScZ7+J_RcEjmcuH94qcoUPuB42ShXzg@mail.gmail.com Backpatch: 13-, where autovacuum_vacuum_insert_threshold was added.
* C comments: improve description of GiST NSN and GistBuildLSNBruce Momjian2021-03-10
| | | | | | | GiST indexes are complex, so adding more details in the code might help someone. Discussion: https://postgr.es/m/20210302164021.GA364@momjian.us
* Replace buffer I/O locks with condition variables.Thomas Munro2021-03-11
| | | | | | | | | | | | | | | | | | | 1. Backends waiting for buffer I/O are now interruptible. 2. If something goes wrong in a backend that is currently performing I/O, waiting backends no longer wake up until that backend reaches AbortBufferIO() and broadcasts on the CV. Previously, any waiters would wake up (because the I/O lock was automatically released) and then busy-loop until AbortBufferIO() cleared BM_IO_IN_PROGRESS. 3. LWLockMinimallyPadded is removed, as it would now be unused. Author: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (earlier version, 2016) Discussion: https://postgr.es/m/CA%2BhUKGJ8nBFrjLuCTuqKN0pd2PQOwj9b_jnsiGFFMDvUxahj_A%40mail.gmail.com Discussion: https://postgr.es/m/CA+Tgmoaj2aPti0yho7FeEf2qt-JgQPRWb0gci_o1Hfr=C56Xng@mail.gmail.com
* Avoid creating duplicate cached plans for inherited FK constraints.Tom Lane2021-03-10
| | | | | | | | | | | | | | | | | When a foreign key constraint is applied to a partitioned table, each leaf partition inherits a similar FK constraint. We were processing all of those constraints independently, meaning that in large partitioning trees we'd build up large collections of cached FK-checking query plans. However, in all cases but one, the generated queries are actually identical for all members of the inheritance tree (because, in most cases, the query only mentions the topmost table of the other side of the FK relationship). So we can share a single cached plan among all the partitions, saving memory, not to mention time to build and maintain the cached plans. Keisuke Kuroda and Amit Langote Discussion: https://postgr.es/m/cab4b85d-9292-967d-adf2-be0d803c3e23@nttcom.co.jp_1
* Add bound check before bsearch() for performancePeter Eisentraut2021-03-10
| | | | | | | | | | | | | | | In the current lazy vacuum implementation, some index AMs such as btree indexes call lazy_tid_reaped() for each index tuple during ambulkdelete to check if the index tuple points to the (collected) garbage tuple. In that function, we simply call bsearch(), but we should be able to know the result without bsearch() if the index tuple points to the heap tuple that is out of range of the collected garbage tuples. Therefore, add a simple bound check before resorting to bsearch(). Testing has shown that this can give significant performance benefits. Author: Masahiko Sawada <masahiko.sawada@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/CA+fd4k76j8jKzJzcx8UqEugvayaMSnQz0iLUt_XgBp-_-bd22A@mail.gmail.com
* Fix another portability bug in recent pgbench commit.Thomas Munro2021-03-10
| | | | | | | | | | | | | | Commit 547f04e7 produced errors on AIX/xlc while building plpython. The new code appears to be incompatible with the hack installed by commit a11cf433. Without access to an AIX system to check, my guess is that _POSIX_C_SOURCE may be required for <time.h> to declare the things the header needs to see, but plpython.h undefines it. For now, to unbreak build farm animal hoverfly, just move the new pg_time_usec_t support into pgbench.c. Perhaps later we could figure out what to rearrange to put it back into a header for wider use. Discussion: https://postgr.es/m/CA%2BhUKG%2BP%2BjcD%3Dx9%2BagyTdWtjpOT64MYiGic%2Bcbu_TD8CV%3D6A3w%40mail.gmail.com
* Try to fix portability bugs in recent pgbench commits.Thomas Munro2021-03-10
| | | | | | | | 1. pg_time_usec_t needs to be printed with INT64_FORMAT, not %ld, or 32 bit systems complain, per lapwing. 2. Some Windows compilers didn't like a thread function not marked with __stdcall, per whelk; let's see if this fixes the problem.
* Small debug message tweakPeter Eisentraut2021-03-10
| | | | This makes the wording of the delete case match the update case.
* Move tablespace path re-creation from the makefiles to pg_regressMichael Paquier2021-03-10
| | | | | | | | | | | | | | | | | | Moving this logic into pg_regress fixes a potential failure with parallel tests when pg_upgrade and the main regression test suite both trigger the makefile rule that cleaned up testtablespace/ under src/test/regress. Even if pg_upgrade was triggering this rule, it has no need to do so as it uses a different tablespace path. So if pg_upgrade triggered the makefile rule for the tablespace setup while the main regression test suite ran the tablespace cases, it would fail. 61be85a was a similar attempt at achieving that, but that broke cases where the regression tests require to run under an Administrator account, like with Appveyor. Reported-by: Andres Freund, Kyotaro Horiguchi Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/20201209012911.uk4d6nxcnkp7ehrx@alap3.anarazel.de
* pgbench: Synchronize client threads.Thomas Munro2021-03-10
| | | | | | | | | | | | | | | Wait until all pgbench threads are connected before benchmarking begins. This fixes a problem where some connections could take a very long time to be established because of lock contention from earlier connections, making results unstable and bogus with high connection counts. Author: Andres Freund <andres@anarazel.de> Author: Fabien COELHO <coelho@cri.ensmp.fr> Reviewed-by: Marina Polyakova <m.polyakova@postgrespro.ru> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
* Add missing pthread_barrier_t.Thomas Munro2021-03-10
| | | | | | | Supply a simple implementation of the missing pthread_barrier_t type and functions, for macOS. Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
* pgbench: Improve time logic.Thomas Munro2021-03-10
| | | | | | | | | | | Instead of instr_time (struct timespec) and the INSTR_XXX macros, introduce pg_time_usec_t and use integer arithmetic. Don't include the connection time in TPS unless using -C mode, but report it separately. Author: Fabien COELHO <coelho@cri.ensmp.fr> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
* pgbench: Refactor thread portability support.Thomas Munro2021-03-10
| | | | | | | | | | | | | | | Instead of maintaining an incomplete emulation of POSIX threads for Windows, let's use an extremely minimalist macro-based abstraction for now. A later patch will extend this, without the need to supply more complicated pthread emulation code. (There may be a need for a more serious portable thread abstraction in later projects, but this is not it.) Minor incidental problems fixed: it wasn't OK to use (pthread_t) 0 as a special value, it wasn't OK to compare thread_t values with ==, and we incorrectly assumed that pthread functions set errno. Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
* Fix valgrind issue in commit 05c8482f7f.Amit Kapila2021-03-10
| | | | | | | | | | Initialize other newly added variables in max_parallel_hazard_context via is_parallel_safe() because we don't check the parallel-safety of target relations in that function. Reported-by: Tom Lane as per buildfarm Author: Amit Kapila Discussion: https://postgr.es/m/2060179.1615347455@sss.pgh.pa.us
* Enable parallel SELECT for "INSERT INTO ... SELECT ...".Amit Kapila2021-03-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Parallel SELECT can't be utilized for INSERT in the following cases: - INSERT statement uses the ON CONFLICT DO UPDATE clause - Target table has a parallel-unsafe: trigger, index expression or predicate, column default expression or check constraint - Target table has a parallel-unsafe domain constraint on any column - Target table is a partitioned table with a parallel-unsafe partition key expression or support function The planner is updated to perform additional parallel-safety checks for the cases listed above, for determining whether it is safe to run INSERT in parallel-mode with an underlying parallel SELECT. The planner will consider using parallel SELECT for "INSERT INTO ... SELECT ...", provided nothing unsafe is found from the additional parallel-safety checks, or from the existing parallel-safety checks for SELECT. While checking parallel-safety, we need to check it for all the partitions on the table which can be costly especially when we decide not to use a parallel plan. So, in a separate patch, we will introduce a GUC and or a reloption to enable/disable parallelism for Insert statements. Prior to entering parallel-mode for the execution of INSERT with parallel SELECT, a TransactionId is acquired and assigned to the current transaction state. This is necessary to prevent the INSERT from attempting to assign the TransactionId whilst in parallel-mode, which is not allowed. This approach has a disadvantage in that if the underlying SELECT does not return any rows, then the TransactionId is not used, however that shouldn't happen in practice in many cases. Author: Greg Nancarrow, Amit Langote, Amit Kapila Reviewed-by: Amit Langote, Hou Zhijie, Takayuki Tsunakawa, Antonin Houska, Bharath Rupireddy, Dilip Kumar, Vignesh C, Zhihong Yu, Amit Kapila Tested-by: Tang, Haiying Discussion: https://postgr.es/m/CAJcOf-cXnB5cnMKqWEp2E2z7Mvcd04iLVmV=qpFJrR3AcrTS3g@mail.gmail.com Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com
* Revert changes for SSL compression in libpqMichael Paquier2021-03-10
| | | | | | | | | | | | | | | This partially reverts 096bbf7 and 9d2d457, undoing the libpq changes as it could cause breakages in distributions that share one single libpq version across multiple major versions of Postgres for extensions and applications linking to that. Note that the backend is unchanged here, and it still disables SSL compression while simplifying the underlying catalogs that tracked if compression was enabled or not for a SSL connection. Per discussion with Tom Lane and Daniel Gustafsson. Discussion: https://postgr.es/m/YEbq15JKJwIX+S6m@paquier.xyz
* libpq: Remove deprecated connection parameters authtype and ttyPeter Eisentraut2021-03-09
| | | | | | | | | | | | | | The authtype parameter was deprecated and made inactive in commit d5bbe2aca55bc8, but the environment variable was left defined and thus tested with a getenv call even though the value is of no use. Also, if it would exist it would be copied but never freed as the cleanup code had been removed. tty was deprecated in commit cb7fb3ca958ec8bd5a14e7 but most of the infrastructure around it remained in place. Author: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/DDDF36F3-582A-4C02-8598-9B464CC42B34@yesql.se
* Switch back sslcompression to be a normal input field in libpqMichael Paquier2021-03-09
| | | | | | | | | | | | Per buildfarm member crake, any servers including a postgres_fdw server with this option set would fail to do a pg_upgrade properly as the option got hidden in f9264d1 by becoming a debug option, making the restore of the FDW server fail. This changes back the option in libpq to be visible, but still inactive to fix this upgrade issue. Discussion: https://postgr.es/m/YEbq15JKJwIX+S6m@paquier.xyz
* Track total amounts of times spent writing and syncing WAL data to disk.Fujii Masao2021-03-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds new GUC track_wal_io_timing. When this is enabled, the total amounts of time XLogWrite writes and issue_xlog_fsync syncs WAL data to disk are counted in pg_stat_wal. This information would be useful to check how much WAL write and sync affect the performance. Enabling track_wal_io_timing will make the server query the operating system for the current time every time WAL is written or synced, which may cause significant overhead on some platforms. To avoid such additional overhead in the server with track_io_timing enabled, this commit introduces track_wal_io_timing as a separate parameter from track_io_timing. Note that WAL write and sync activity by walreceiver has not been tracked yet. This commit makes the server also track the numbers of times XLogWrite writes and issue_xlog_fsync syncs WAL data to disk, in pg_stat_wal, regardless of the setting of track_wal_io_timing. This counters can be used to calculate the WAL write and sync time per request, for example. Bump PGSTAT_FILE_FORMAT_ID. Bump catalog version. Author: Masahiro Ikeda Reviewed-By: Japin Li, Hayato Kuroda, Masahiko Sawada, David Johnston, Fujii Masao Discussion: https://postgr.es/m/0509ad67b585a5b86a83d445dfa75392@oss.nttdata.com
* Add support for more progress reporting in COPYMichael Paquier2021-03-09
| | | | | | | | | | | | | | | | | The command (TO or FROM), its type (file, pipe, program or callback), and the number of tuples excluded by a WHERE clause in COPY FROM are added to the progress reporting already available. The column "lines_processed" is renamed to "tuples_processed" to disambiguate the meaning of this column in the cases of CSV and BINARY COPY and to be more consistent with the other catalog progress views. Bump catalog version, again. Author: Matthias van de Meent Reviewed-by: Michael Paquier, Justin Pryzby, Bharath Rupireddy, Josef Šimánek, Tomas Vondra Discussion: https://postgr.es/m/CAEze2WiOcgdH4aQA8NtZq-4dgvnJzp8PohdeKchPkhMY-jWZXA@mail.gmail.com
* Remove support for SSL compressionMichael Paquier2021-03-09
| | | | | | | | | | | | | | | | | | | | | | | | | | PostgreSQL disabled compression as of e3bdb2d and the documentation recommends against using it since. Additionally, SSL compression has been disabled in OpenSSL since version 1.1.0, and was disabled in many distributions long before that. The most recent TLS version, TLSv1.3, disallows compression at the protocol level. This commit removes the feature itself, removing support for the libpq parameter sslcompression (parameter still listed for compatibility reasons with existing connection strings, just ignored), and removes the equivalent field in pg_stat_ssl and de facto PgBackendSSLStatus. Note that, on top of removing the ability to activate compression by configuration, compression is actively disabled in both frontend and backend to avoid overrides from local configurations. A TAP test is added for deprecated SSL parameters to check after backwards compatibility. Bump catalog version. Author: Daniel Gustafsson Reviewed-by: Peter Eisentraut, Magnus Hagander, Michael Paquier Discussion: https://postgr.es/m/7E384D48-11C5-441B-9EC3-F7DB1F8518F6@yesql.se
* Complain if a function-in-FROM returns a set when it shouldn't.Tom Lane2021-03-08
| | | | | | | | | | | | | | | Throw a "function protocol violation" error if a function in FROM tries to return a set though it wasn't marked proretset. Although such cases work at the moment, it doesn't seem like something we want to guarantee will keep working. Besides, there are other negative consequences of not setting the proretset flag, such as potentially bad plans. No back-patch, since if there is any third-party code violating this expectation, people wouldn't appreciate us breaking it in a minor release. Discussion: https://postgr.es/m/1636062.1615141782@sss.pgh.pa.us
* Properly mark pg_stat_get_subscription() as returning a set.Tom Lane2021-03-08
| | | | | | | | | | | | | | | The initial catalog data for this function failed to set proretset or provide a prorows estimate. It accidentally worked anyway when invoked in the FROM clause, because the executor isn't too picky about this; but the planner didn't expect the function to return multiple rows, which could lead to bad plans. Also the function would fail if invoked in the SELECT list. We can't easily back-patch this fix, but fortunately the bug's consequences aren't awful in most cases. Getting this right is mainly an exercise in future-proofing. Discussion: https://postgr.es/m/1636062.1615141782@sss.pgh.pa.us
* Validate the OID argument of pg_import_system_collations().Tom Lane2021-03-08
| | | | | | | | | | | | | "SELECT pg_import_system_collations(0)" caused an assertion failure. With a random nonzero argument --- or indeed with zero, in non-assert builds --- it would happily make pg_collation entries with garbage values of collnamespace. These are harmless as far as I can tell (unless maybe the OID happens to become used for a schema, later on?). In any case this isn't a security issue, since the function is superuser-only. But it seems like a gotcha for unwary DBAs, so let's add a check that the given OID belongs to some schema. Back-patch to v10 where this function was introduced.
* Further tweak memory management for regex DFAs.Tom Lane2021-03-08
| | | | | | | | | | | | | | | | | | | | Coverity is still unhappy after commit 190c79884, and after looking closer I think it might be onto something. The callers of newdfa() typically drop out if v->err has been set nonzero, which newdfa() is faithfully doing if it fails. However, what if v->err was already nonzero before we entered newdfa()? Then newdfa() could succeed and the caller would promptly leak its result. I don't think this scenario can actually happen, but the predicate "v->err is always zero when newdfa() is called" seems difficult to be entirely sure of; there's a good deal of code that potentially could get that wrong. It seems better to adjust the callers to directly check for a null result instead of relying on ISERR() tests. This is slightly cheaper than the previous coding anyway. Lacking evidence that there's any real bug, no back-patch.
* Track replication origin progress for rollbacks.Amit Kapila2021-03-08
| | | | | | | | | | | | | | | | Commit 1eb6d6527a allowed to track replica origin replay progress for 2PC but it was not complete. It misses to properly track the progress for rollback prepared especially it missed updating the code for recovery. Additionally, we need to allow tracking it on subscriber nodes where wal_level might not be logical. It is required to track decoding of 2PC which is committed in PG14 (a271a1b50e) and also nobody complained about this till now so not backpatching it. Author: Amit Kapila Reviewed-by: Michael Paquier and Ajin Cherian Discussion: https://postgr.es/m/CAA4eK1L-kHmMnSdrRW6UhRbCjR7cgh04c+6psY15qzT6ktcd+g@mail.gmail.com
* Add bit_xor aggregate functionPeter Eisentraut2021-03-06
| | | | | | | | | This can be used as a checksum for unordered sets. bit_and and bit_or already exist. Author: Alexey Bashtanov <bashtanov@imap.cc> Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/9d4582ae-ecfc-3a13-2238-6ab5a37c1f41@imap.cc
* pgbench: Simplify some port, host, user and dbname assignmentsMichael Paquier2021-03-06
| | | | | | | | | | | | | | | | | | | | | Using pgbench in an environment with both PGPORT and PGUSER set would have caused the generation of a debug log with an incorrect database name due to an oversight in 412893b. Not specifying user, port and/or database using the option switches, without their respective environment variables, generated a log entry with empty strings, which was rather useless. This commit fixes this set of issues by simplifying the logic grabbing the connection information, removing a set of getenv() calls that emulated what libpq already does. The faulty debug log now directly uses the information from the libpq connection, and it gets generated after the connection to the backend is completed, not before it (in the event of a failure libpq would complain with more information about the connection attempt so the log is not really useful before anyway). Author: Kota Miyake Reviewed-by: Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/026b3ae6fc339a18394d053c32a4463d@oss.nttdata.com
* Add support for PROVE_TESTS and PROVE_FLAGS in MSVC scriptsMichael Paquier2021-03-05
| | | | | | | | | | | | | | | | These can be set in buildenv.pl or a "set" command within a Windows terminal. The MSVC script vcregress.pl parses the values available in the environment to build the resulting prove commands, and the parsing of PROVE_TESTS is able to handle name patterns in the same way as other platforms. Not specifying those environment values makes vcregress.pl fall back to the previous default, with no extra flags for the prove command, and all the tests run within t/. Author: Michael Paquier Reviewed-by: Juan José Santamaría Flecha, Julien Rouhaud Discussion: https://postgr.es/m/YD9GigwHoL6lFY2y@paquier.xyz
* Close psql processes gracefully in recovery testsAndrew Dunstan2021-03-04
| | | | | | | | | | | | Under windows, psql processes need to be ended explicitly, or the TAP tests hang. However, the recovery tests were doing this via IPC::Run::kill_kill(), which causes other major problems on Windows. We solve this by instead sending '\q' to psql so it quits of its own accord, and then simply waiting for it. This means we can now run almost all the recovery tests on all Windows platforms. Discussion: https://postgr.es/m/20210301200715.tdjpuesfzebpffgn@alap3.anarazel.de
* pg_upgrade: Fix oversight in version checkingPeter Eisentraut2021-03-04
| | | | | | | | | Mistake in f06b1c598254f8adb2b7f51d6a7685618a7fb121: We should only check the version of the binaries in the target installation. The source installation can of course be of a different version. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/E1lHNKN-0005IC-V6%40gemulon.postgresql.org
* Remove redundant getenv() for PGUSER, in psql help.Fujii Masao2021-03-04
| | | | | | | | | Previously psql obtained the value of PGUSER twice to display a default user in its help message. Author: Kota Miyake Reviewed-by: Nitin Jadhav, Fujii Masao Discussion: https://postgr.es/m/2a3c612babdd6ed63a9d877bb575d793@oss.nttdata.com
* Avoid extra newline in errors received in FE protocol version 2.Heikki Linnakangas2021-03-04
| | | | | | | | Contrary to what the comment said, the postmaster does in fact end all its messages in a newline, since server version 7.2. Be tidy and don't add an extra newline if the error message already has one. Discussion: https://www.postgresql.org/message-id/CAFBsxsEdgMXc%2BtGfEy9Y41i%3D5pMMjKeH8t8vSAypR3ZnAoQnHg%40mail.gmail.com
* Remove server and libpq support for old FE/BE protocol version 2.Heikki Linnakangas2021-03-04
| | | | | | | | | | | | | | | | | Protocol version 3 was introduced in PostgreSQL 7.4. There shouldn't be many clients or servers left out there without version 3 support. But as a courtesy, I kept just enough of the old protocol support that we can still send the "unsupported protocol version" error in v2 format, so that old clients can display the message properly. Likewise, libpq still understands v2 ErrorResponse messages when establishing a connection. The impetus to do this now is that I'm working on a patch to COPY FROM, to always prefetch some data. We cannot do that safely with the old protocol, because it requires parsing the input one byte at a time to detect the end-of-copy marker. Reviewed-by: Tom Lane, Alvaro Herrera, John Naylor Discussion: https://www.postgresql.org/message-id/9ec25819-0a8a-d51a-17dc-4150bb3cca3b%40iki.fi
* Add trim_array() function.Tom Lane2021-03-03
| | | | | | | | | | This has been in the SQL spec since 2008. It's a pretty thin wrapper around the array slice functionality, but the spec says we should have it, so here it is. Vik Fearing, reviewed by Dian Fay Discussion: https://postgr.es/m/fc92ce17-9655-8ff1-c62a-4dc4c8ccd815@postgresfriends.org
* Make test_target_session_attrs more robust against connection failure.Tom Lane2021-03-03
| | | | | | | | | | | | | Feed the desired command to psql via "-c" not stdin, else Perl may complain that it can't push stdin to an already-failed psql process, as seen in intermittent buildfarm failures. Make some minor cosmetic improvements while at it. Before commit ee28cacf6 we had no tests here that expected failure to connect, so there seems no need for a back-patch. Discussion: https://postgr.es/m/CALDaNm2mo8YED=M2ZJKGf1U3F3mw6SaQuLXWCK8rZP6sECYcrA@mail.gmail.com
* pg_upgrade: Check version of target cluster binariesPeter Eisentraut2021-03-03
| | | | | | | | | | | | This expands the binary validation in pg_upgrade with a version check per binary to ensure that the target cluster installation only contains binaries from the target version. In order to reduce duplication, validate_exec is exported from port.h and the local copy in pg_upgrade is removed. Author: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/9328.1552952117@sss.pgh.pa.us