summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2019-12-04Offer pnstrdup to frontend codeAlvaro Herrera
We already had it on the backend. Frontend can also use it now. Discussion: https://postgr.es/m/20191204144021.GA17976@alvherre.pgsql
2019-12-04Update minimum SSL versionPeter Eisentraut
Change default of ssl_min_protocol_version to TLSv1.2 (from TLSv1, which means 1.0). Older versions are still supported, just not by default. TLS 1.0 is widely deprecated, and TLS 1.1 only slightly less so. All OpenSSL versions that support TLS 1.1 also support TLS 1.2, so there would be very little reason to, say, set the default to TLS 1.1 instead on grounds of better compatibility. The test suite overrides this new setting, so it can still run with older OpenSSL versions. Discussion: https://www.postgresql.org/message-id/flat/b327f8df-da98-054d-0cc5-b76a857cfed9%402ndquadrant.com
2019-12-04Fix whitespace.Etsuro Fujita
2019-12-04Use carriage returns for data insertion logs in pgbench on terminalMichael Paquier
This is similar to what pg_basebackup and pg_rewind do when reporting cumulative data, and that's more user-friendly. Carriage returns are now used when stderr points to a terminal, and newlines are used in other cases, like a redirection to a log file. Author: Amit Langote Reviewed-by: Fabien Coelho Discussion: https://postgr.es/m/CA+HiwqFNwEjPeVaQsp2L7DyCPv1Eg1guwhrVhzMYqUJUk8ULKg@mail.gmail.com
2019-12-04Remove unnecessary definition of CancelRequested in bin/scripts/Michael Paquier
This variable is now part of the refactored code for query cancellation in fe_utils. This fixes an oversight in commit a4fd3aa. While on it, improve some header includes in bin/scripts/. Author: Michael Paquier Reviewed-by: Fabien Coelho Discussion: https://postgr.es/m/20191203101625.GF1634@paquier.xyz
2019-12-03Fix thinkos from commit 9989d37Michael Paquier
Error messages referring to incorrect WAL segment names could have been generated for a fsync() failure or when creating a new segment at the end of recovery.
2019-12-03Fix alter_system_table testPeter Eisentraut
Add workaround for disabling ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS warning for the test that tries to create a tablespace with a reserved name. Discussion: https://www.postgresql.org/message-id/flat/E1iacW7-0003h6-6U%40gemulon.postgresql.org
2019-12-03Remove XLogFileNameP() from the treeMichael Paquier
XLogFileNameP() is a wrapper routine able to build a palloc'd string for a WAL segment name, which is used for error string generation. There were several code paths where it gets called in a critical section, where memory allocation is not allowed. This results in triggering an assertion failure instead of generating the wanted error message. Another, more annoying, problem is that if the allocation to generate the WAL segment name fails on OOM, then the failure would be escalated to a PANIC. This removes the routine and all its callers are replaced with a logic using a fixed-size buffer. This way, all the existing mistakes are fixed and future ones are prevented. Author: Masahiko Sawada Reviewed-by: Michael Paquier, Álvaro Herrera Discussion: https://postgr.es/m/CA+fd4k5gC9H4uoWMLg9K_QfNrnkkdEw+-AFveob9YX7z8JnKTA@mail.gmail.com
2019-12-03Fix failures with TAP tests of pg_ctl on WindowsMichael Paquier
On Windows, all the hosts spawned by the TAP tests bind to 127.0.0.1. Hence, if there is a port conflict, starting a cluster would immediately fail. One of the test scripts of pg_ctl initializes a node without PostgresNode.pm, using the default port 5432. This could cause unexpected startup failures in the tests if an independent server was up and running on the same host (the reverse is also possible, though more unlikely). Fix this issue by assigning properly a free port to the node configured, in the same range used as for the other nodes part of the tests. Author: Michael Paquier Reviewed-by: Andrew Dunstan Discussion: https://postgr.es/m/20191202031444.GC1696@paquier.xyz Backpatch-through: 11
2019-12-02Fix EXPLAIN's column alias output for mismatched child tables.Tom Lane
If an inheritance/partitioning parent table is assigned some column alias names in the query, EXPLAIN mapped those aliases onto the child tables' columns by physical position, resulting in bogus output if a child table's columns aren't one-for-one with the parent's. To fix, make expand_single_inheritance_child() generate a correctly re-mapped column alias list, rather than just copying the parent RTE's alias node. (We have to fill the alias field, not just adjust the eref field, because ruleutils.c will ignore eref in favor of looking at the real column names.) This means that child tables will now always have alias fields in plan rtables, where before they might not have. That results in a rather substantial set of regression test output changes: EXPLAIN will now always show child tables with aliases that match the parent table (usually with "_N" appended for uniqueness). But that seems like a net positive for understandability, since the parent alias corresponds to something that actually appeared in the original query, while the child table names didn't. (Note that this does not change anything for cases where an explicit table alias was written in the query for the parent table; it just makes cases without such aliases behave similarly to that.) Hence, while we could avoid these subsidiary changes if we made inherit.c more complicated, we choose not to. Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
2019-12-02Add a reverse-translation column number array to struct AppendRelInfo.Tom Lane
This provides for cheaper mapping of child columns back to parent columns. The one existing use-case in examine_simple_variable() would hardly justify this by itself; but an upcoming bug fix will make use of this array in a mainstream code path, and it seems likely that we'll find other uses for it as we continue to build out the partitioning infrastructure. Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
2019-12-02Add query cancellation capabilities in pgbench init phaseMichael Paquier
This can be useful to stop data generation happening on the server for long-running queries caused by large scale factors. This cannot happen by default as data is generated on the client, but it is possible to control the initialization steps of pgbench to do that. Reported-by: Fujii Masao Author: Fabien Coelho Discussion: https://postgr.es/m/alpine.DEB.2.21.1910311939430.27369@lancre Discussion: https://postgr.es/m/CAHGQGwHWEyTXxZh46qgFY8a2bDF_EYeUdp3+_Hy=qLZSzwVPKg@mail.gmail.com
2019-12-02Refactor query cancellation code into src/fe_utils/Michael Paquier
Originally, this code was duplicated in src/bin/psql/ and src/bin/scripts/, but it can be useful for other frontend applications, like pgbench. This refactoring offers the possibility to setup a custom callback which would get called in the signal handler for SIGINT or when the interruption console events happen on Windows. Author: Fabien Coelho, with contributions from Michael Paquier Reviewed-by: Álvaro Herrera, Ibrar Ahmed Discussion: https://postgr.es/m/alpine.DEB.2.21.1910311939430.27369@lancre
2019-12-01Add dummy versions of new SSL functions for non-SSL buildsAndrew Dunstan
This rectifies an oversight in commit 4dc6355210, which caused certain builds to fail, especially on Windows.
2019-12-01Fix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables.Tom Lane
We implement ON COMMIT DELETE ROWS by truncating tables marked that way, which requires also truncating/rebuilding their indexes. But RelationTruncateIndexes asks the relcache for up-to-date copies of any index expressions, which may cause execution of eval_const_expressions on them, which can result in actual execution of subexpressions. This is a bad thing to have happening during ON COMMIT. Manuel Rigger reported that use of a SQL function resulted in crashes due to expectations that ActiveSnapshot would be set, which it isn't. The most obvious fix perhaps would be to push a snapshot during PreCommit_on_commit_actions, but I think that would just open the door to more problems: CommitTransaction explicitly expects that no user-defined code can be running at this point. Fortunately, since we know that no tuples exist to be indexed, there seems no need to use the real index expressions or predicates during RelationTruncateIndexes. We can set up dummy index expressions instead (we do need something that will expose the right data type, as there are places that build index tupdescs based on this), and just ignore predicates and exclusion constraints. In a green field it'd likely be better to reimplement ON COMMIT DELETE ROWS using the same "init fork" infrastructure used for unlogged relations. That seems impractical without catalog changes though, and even without that it'd be too big a change to back-patch. So for now do it like this. Per private report from Manuel Rigger. This has been broken forever, so back-patch to all supported branches.
2019-11-30libq support for sslpassword connection param, DER format keysAndrew Dunstan
This patch providies for support for password protected SSL client keys in libpq, and for DER format keys, both encrypted and unencrypted. There is a new connection parameter sslpassword, which is supplied to the OpenSSL libraries via a callback function. The callback function can also be set by an application by calling PQgetSSLKeyPassHook(). There is also a function to retreive the connection setting, PQsslpassword(). Craig Ringer and Andrew Dunstan Reviewed by: Greg Nancarrow Discussion: https://postgr.es/m/f7ee88ed-95c4-95c1-d4bf-7b415363ab62@2ndQuadrant.com
2019-11-30Fix off-by-one error in PGTYPEStimestamp_fmt_ascTomas Vondra
When using %b or %B patterns to format a date, the code was simply using tm_mon as an index into array of month names. But that is wrong, because tm_mon is 1-based, while array indexes are 0-based. The result is we either use name of the next month, or a segfault (for December). Fix by subtracting 1 from tm_mon for both patterns, and add a regression test triggering the issue. Backpatch to all supported versions (the bug is there far longer, since at least 2003). Reported-by: Paul Spencer Backpatch-through: 9.4 Discussion: https://postgr.es/m/16143-0d861eb8688d3fef%40postgresql.org
2019-11-30Revert commits 290acac92b and 8a7e9e9dad.Amit Kapila
This commit revert the commits to add a test case that tests the 'force' option when there is an active backend connected to the database being dropped. This feature internally sends SIGTERM to all the backends connected to the database being dropped and then the same is reported to the client. We found that on Windows, the client end of the socket is not able to read the data once we close the socket in the server which leads to loss of error message which is not what we expect. We also observed similar behavior in other cases like pg_terminate_backend(), pg_ctl kill TERM <pid>. There are probably a few others like that. The fix for this requires further study. Discussion: https://postgr.es/m/E1iaD8h-0004us-K9@gemulon.postgresql.org
2019-11-29Small code simplificationPeter Eisentraut
FLOAT8PASSBYVAL can be used instead of USE_FLOAT8_BYVAL here.
2019-11-29Add a regression test for allow_system_table_modsPeter Eisentraut
Add a regression test file that exercises the kinds of commands that allow_system_table_mods allows. This is put in the "unsafe_tests" suite, so it won't accidentally create a mess if someone runs the normal regression tests against an instance that they care about. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com
2019-11-29Make allow_system_table_mods settable at run timePeter Eisentraut
Make allow_system_table_mods settable at run time by superusers. It was previously postmaster start only. We don't want to make system catalog DDL wide-open, but there are occasionally useful things to do like setting reloptions or statistics on a busy system table, and blocking those doesn't help anyone. Also, this enables the possibility of writing a test suite for this setting. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com
2019-11-29Remove any-user DML capability from allow_system_table_modsPeter Eisentraut
Previously, allow_system_table_mods allowed a non-superuser to do DML on a system table without further permission checks. This has been removed, as it was quite inconsistent with the rest of the meaning of this setting. (Since allow_system_table_mods was previously only accessible with a server restart, it is unlikely that anyone was using this possibility.) Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com
2019-11-29Add error position to an error messagePeter Eisentraut
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/6e7aa4a1-be6a-1a75-b1f9-83a678e5184a@2ndquadrant.com
2019-11-28Use memcpy instead of a byte loop in pglz_decompressTomas Vondra
The byte loop used in pglz_decompress() because of possible overlap may be quite inefficient, so this commit replaces it with memcpy. The gains do depend on the data (compressibility) and hardware, but seem to be quite significant. Author: Andrey Borodin Reviewed-by: Michael Paquier, Konstantin Knizhnik, Tels Discussion: https://postgr.es/m/469C9ED9-348C-4FE7-A7A7-B0FA671BEE4C@yandex-team.ru
2019-11-28Remove unnecessary clauses_attnums variableTomas Vondra
Commit c676e659b2 reworked how choose_best_statistics() picks the best extended statistics, but failed to remove clauses_attnums which is now unnecessary. So get rid of it and backpatch to 12, same as c676e659b2. Author: Tomas Vondra Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com Backpatch-through: 12
2019-11-28Fix choose_best_statistics to check clauses individuallyTomas Vondra
When picking the best extended statistics object for a list of clauses, it's not enough to look at attnums extracted from the clause list as a whole. Consider for example this query with OR clauses: SELECT * FROM t WHERE (t.a = 1) OR (t.b = 1) OR (t.c = 1) with a statistics defined on columns (a,b). Relying on attnums extracted from the whole OR clause, we'd consider the statistics usable. That does not work, as we see the conditions as a single OR-clause, referencing an attribute not covered by the statistic, leading to empty list of clauses to be estimated using the statistics and an assert failure. This changes choose_best_statistics to check which clauses are actually covered, and only using attributes from the fully covered ones. For the previous example this means the statistics object will not be considered as compatible with the OR-clause. Backpatch to 12, where MCVs were introduced. The issue does not affect older versions because functional dependencies don't handle OR clauses. Author: Tomas Vondra Reviewed-by: Dean Rasheed Reported-By: Manuel Rigger Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com Backpatch-through: 12
2019-11-28Remove useless "return;" linesAlvaro Herrera
Discussion: https://postgr.es/m/20191128144653.GA27883@alvherre.pgsql
2019-11-28Add tests for '-f' option in dropdb utility.Amit Kapila
This will test that the force option works when there is an active backend connected to the database being dropped. Author: Pavel Stehule and Vignesh C Reviewed-by: Amit Kapila and Vignesh C Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com
2019-11-28Move pump_until to TestLib.pm.Amit Kapila
The subroutine pump_until provides the functionality to poll until the given string is matched, or a timeout occurs.  This can be used from other places as well, so moving it to TestLib.pm.  The immediate need is for an upcoming regression test patch for dropdb utility. Author: Vignesh C Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com
2019-11-27pg_upgrade: adjust error paragraph width to be consistentBruce Momjian
Previously these error paragraphs were too wide. Backpatch-through: 13
2019-11-27pg_upgrade: improve instructions for fixing incompatible isn useBruce Momjian
This clarifies instructions on how to dump/reload databases that use incompatible isn versions. Reported-by: Alvaro Herrera Discussion: https://postgr.es/m/20191114190652.GA23586@alvherre.pgsql Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Backpatch-through: 13
2019-11-27Don't use native methods in TestLib::slurp_file on MsysAndrew Dunstan
Commit 114541d58e has upset some buildfarm members running Msys, that weren't previously having problems, so the use of native Windows methods to open files is restricted by this patch to only MSVC builds.
2019-11-27Revert "Close stdin where it's not needed in TestLib.pm procedures"Andrew Dunstan
This reverts commits 9af34f3c6b and 792dba73c8. The code has been found not to be portable. Discussion: https://postgr.es/m/2658c496-f885-02db-13bb-228423782524@2ndQuadrant.com
2019-11-27Move configure --disable-float8-byval to pg_config_manual.hPeter Eisentraut
This build option was once useful to maintain compatibility with version-0 functions, but those are no longer supported, so this option is no longer useful for end users. We keep the option available to developers in pg_config_manual.h so that it is easy to test the pass-by-reference code paths without having to fire up a 32-bit machine. Discussion: https://www.postgresql.org/message-id/flat/f3e1e576-2749-bbd7-2d57-3f9dcf75255a@2ndquadrant.com
2019-11-27Fix typo in comment.Etsuro Fujita
2019-11-26Fix closing stdin in TestLib.pmAndrew Dunstan
Commit 9af34f3c6b naively assumed that all non-windows platforms would have pseudoterminals and that perl would have IO::Pty. Such is not the case. Instead of assumung anything, test for the presence of IO::Pty and only use code that might depend on it if it's present. The test result is exposed in $TestLib::have_io_pty so that it can be conveniently used in SKIP tests. Discussion: https://postgr.es/m/20191126044110.GB5435@paquier.xyz
2019-11-26Allow access to child table statistics if user can read parent table.Tom Lane
The fix for CVE-2017-7484 disallowed use of pg_statistic data for planning purposes if the user would not be able to select the associated column and a non-leakproof function is to be applied to the statistics values. That turns out to disable use of pg_statistic data in some common cases involving inheritance/partitioning, where the user does have permission to select from the parent table that was actually named in the query, but not from a child table whose stats are needed. Since, in non-corner cases, the user *can* select the child table's data via the parent, this restriction is not actually useful from a security standpoint. Improve the logic so that we also check the permissions of the originally-named table, and allow access if select permission exists for that. When checking access to stats for a simple child column, we can map the child column number back to the parent, and perform this test exactly (including not allowing access if the child column isn't exposed by the parent). For expression indexes, the current logic just insists on whole-table select access, and this patch allows access if the user can select the whole parent table. In principle, if the child table has extra columns, this might allow access to stats on columns the user can't read. In practice, it's unlikely that the planner is going to do any stats calculations involving expressions that are not visible to the query, so we'll ignore that fine point for now. Perhaps someday we'll improve that logic to detect exactly which columns are used by an expression index ... but today is not that day. Back-patch to v11. The issue was created in 9.2 and up by the CVE-2017-7484 fix, but this patch depends on the append_rel_array[] planner data structure which only exists in v11 and up. In practice the issue is most urgent with partitioned tables, so fixing v11 and later should satisfy much of the practical need. Dilip Kumar and Amit Langote, with some kibitzing by me Discussion: https://postgr.es/m/3876.1531261875@sss.pgh.pa.us
2019-11-26Add safeguards for pg_fsync() called with incorrectly-opened fdsMichael Paquier
On some platforms, fsync() returns EBADFD when opening a file descriptor with O_RDONLY (read-only), leading ultimately now to a PANIC to prevent data corruption. This commit adds a new sanity check in pg_fsync() based on fcntl() to make sure that we don't repeat again mistakes with incorrectly-set file descriptors so as problems are detected at an early stage. Without that, such errors could only be detected after running Postgres on a specific supported platform for the culprit code path, which could take some time before being found. b8e19b93 was a fix for such a problem, which got undetected for more than 5 years, and a586cc4b fixed another similar issue. Note that the new check added works as well when fsync=off is configured, so as all regression tests would detect problems as long as assertions are enabled. fcntl() being not available on Windows, the new checks do not happen there. Author: Michael Paquier Reviewed-by: Mark Dilger Discussion: https://postgr.es/m/20191009062640.GB21379@paquier.xyz
2019-11-26Don't shut down Gather[Merge] early under Limit.Amit Kapila
Revert part of commit 19df1702f5. Early shutdown was added by that commit so that we could collect statistics from workers, but unfortunately, it interacted badly with rescans. The problem is that we ended up destroying the parallel context which is required for rescans. This leads to rescans of a Limit node over a Gather node to produce unpredictable results as it tries to access destroyed parallel context. By reverting the early shutdown code, we might lose statistics in some cases of Limit over Gather [Merge], but that will require further study to fix. Reported-by: Jerry Sievers Diagnosed-by: Thomas Munro Author: Amit Kapila, testcase by Vignesh C Backpatch-through: 9.6 Discussion: https://postgr.es/m/87ims2amh6.fsf@jsievers.enova.com
2019-11-25Use procsignal_sigusr1_handler for auxiliary processes.Robert Haas
AuxiliaryProcessMain does ProcSignalInit, so one might expect that auxiliary processes would need to respond to SendProcSignal, but none of the auxiliary processes do that. Change them to use procsignal_sigusr1_handler instead of their own private handlers so that they do. Besides seeming more correct, this is also less code. It shouldn't make any functional difference right now because, as far as we know, there are no current cases where SendProcSignal targets an auxiliary process, but there are plans to change that in the future. Andres Freund Discussion: http://postgr.es/m/20181030051643.elbxjww5jjgnjaxg@alap3.anarazel.de
2019-11-25Close stdin where it's not needed in TestLib.pm proceduresAndrew Dunstan
Where possible, do this using a pseudoterminal, so that things like openssl that want to open /dev/tty if stdin isn't a tty won't. Elsewhere, i.e. Windows, just close by providing an empty string using the standard IPC::Run pipe mechanism. Patch by Andrew Dunstan, based on an idea from Craig Ringer. Reviewed by Mark Dilger. Discussion: https://postgr.es/m/873ebb57-fc98-340d-949d-691b1810bf66@2ndQuadrant.com
2019-11-25Refactor WAL file-reading code into WALRead()Alvaro Herrera
XLogReader, walsender and pg_waldump all had their own routines to read data from WAL files to memory, with slightly different approaches according to the particular conditions of each environment. There's a lot of commonality, so we can refactor that into a single routine WALRead in XLogReader, and move the differences to a separate (simpler) callback that just opens the next WAL-segment. This results in a clearer (ahem) code flow. The error reporting needs are covered by filling in a new error-info struct, WALReadError, and it's the caller's responsibility to act on it. The backend has WALReadRaiseError() to do so. We no longer ever need to seek in this interface; switch to using pg_pread(). Author: Antonin Houska, with contributions from Álvaro Herrera Reviewed-by: Michaël Paquier, Kyotaro Horiguchi Discussion: https://postgr.es/m/14984.1554998742@spoje.net
2019-11-25Fix unportable printf format introduced in commit 9290ad198.Tom Lane
"%ld" is not an acceptable format spec for int64 variables, though it accidentally works on most non-Windows 64-bit platforms. Follow the lead of commit 6a1cd8b92, and use "%lld" with an explicit cast to long long. Per buildfarm.
2019-11-25Make the order of the header file includes consistent.Amit Kapila
Similar to commits 14aec03502, 7e735035f2 and dddf4cdc33, this commit makes the order of header file inclusion consistent in more places. Author: Vignesh C Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
2019-11-25Fix inconsistent variable name in static function of mac8.cMichael Paquier
Both argument names were reversed in the declaration of the function. Author: Ranier Vilela Discussion: https://postgr.es/m/MN2PR18MB292755AEFF9A9144B220ABEEE34B0@MN2PR18MB2927.namprd18.prod.outlook.com
2019-11-25Refactor reloption handling for index AMs in-coreMichael Paquier
This reworks the reloption parsing and build of a couple of index AMs by creating new structures for each index AM's options. This split was already done for BRIN, GIN and GiST (which actually has a fillfactor parameter), but not for hash, B-tree and SPGiST which relied on StdRdOptions due to an overlap with the default option set. This saves a couple of bytes for rd_options in each relcache entry with indexes making use of relation options, and brings more consistency between all index AMs. While on it, add a couple of AssertMacro() calls to make sure that utility macros to grab values of reloptions are used with the expected index AM. Author: Nikolay Shaplov Reviewed-by: Amit Langote, Michael Paquier, Álvaro Herrera, Dent John Discussion: https://postgr.es/m/4127670.gFlpRb6XCm@x200m
2019-11-24Use native methods to open input in TestLib::slurp_file on Windows.Andrew Dunstan
It is hoped that this will avoid some errors that we have seen before, but lately with greater frequency, in buildfarm animals. For now apply only to master. If this proves effective it can be backpatched. Discussion: https://postgr.es/m/13900.1572839580@sss.pgh.pa.us Author: Juan José Santamaría Flecha
2019-11-24Doc: improve discussion of race conditions involved in LISTEN.Tom Lane
The user docs didn't really explain how to use LISTEN safely, so clarify that. Also clean up some fuzzy-headed explanations in comments. No code changes. Discussion: https://postgr.es/m/3ac7f397-4d5f-be8e-f354-440020675694@gmail.com
2019-11-24Avoid assertion failure with LISTEN in a serializable transaction.Tom Lane
If LISTEN is the only action in a serializable-mode transaction, and the session was not previously listening, and the notify queue is not empty, predicate.c reported an assertion failure. That happened because we'd acquire the transaction's initial snapshot during PreCommit_Notify, which was called *after* predicate.c expects any such snapshot to have been established. To fix, just swap the order of the PreCommit_Notify and PreCommit_CheckForSerializationFailure calls during CommitTransaction. This will imply holding the notify-insertion lock slightly longer, but the difference could only be meaningful in serializable mode, which is an expensive option anyway. It appears that this is just an assertion failure, with no consequences in non-assert builds. A snapshot used only to scan the notify queue could not have been involved in any serialization conflicts, so there would be nothing for PreCommit_CheckForSerializationFailure to do except assign it a prepareSeqNo and set the SXACT_FLAG_PREPARED flag. And given no conflicts, neither of those omissions affect the behavior of ReleasePredicateLocks. This admittedly once-over-lightly analysis is backed up by the lack of field reports of trouble. Per report from Mark Dilger. The bug is old, so back-patch to all supported branches; but the new test case only goes back to 9.6, for lack of adequate isolationtester infrastructure before that. Discussion: https://postgr.es/m/3ac7f397-4d5f-be8e-f354-440020675694@gmail.com Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us
2019-11-24Stabilize NOTIFY behavior by transmitting notifies before ReadyForQuery.Tom Lane
This patch ensures that, if any notify messages were received during a just-finished transaction, they get sent to the frontend just before not just after the ReadyForQuery message. With libpq and other client libraries that act similarly, this guarantees that the client will see the notify messages as available as soon as it thinks the transaction is done. This probably makes no difference in practice, since in realistic use-cases the application would have to cope with asynchronous arrival of notify events anyhow. However, it makes it a lot easier to build cross-session-notify test cases with stable behavior. I'm a bit surprised now that we've not seen any buildfarm instability with the test cases added by commit b10f40bf0. Tests that I intend to add in an upcoming bug fix are definitely unstable without this. Back-patch to 9.6, which is as far back as we can do NOTIFY testing with the isolationtester infrastructure. Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us