aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* Error message refactoringPeter Eisentraut2020-06-15
| | | | | | Take some untranslatable things out of the message and replace by format placeholders, to reduce translatable strings and reduce translation mistakes.
* Bump catversion for ACL changes on replication origin functionsMichael Paquier2020-06-15
| | | | | | | Oversight in cc07264. Reported-by: Tom Lane Discussion: https://postgr.es/m/1098356.1592196242@sss.pgh.pa.us
* Doc: Add references for SI and SSI.Thomas Munro2020-06-15
| | | | | | | | | | | | | | | | | | | | | | Our documentation failed to point out that REPEATABLE READ is really snapshot isolation, which might be important to some users. Point to the standard reference paper for this complicated topic. Likewise, add a reference to the VLDB paper about PostgreSQL SSI, for technical information about our SSI implementation and how it compares to S2PL. While here, add a note about catalog access using a lower isolation level, per recent user complaint. Back-patch to all releases. Reported-by: Kyle Kingsbury <aphyr@jepsen.io> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Tatsuo Ishii <ishii@sraoss.co.jp> Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io Discussion: https://postgr.es/m/16454-9408996bb1750faf%40postgresql.org
* Fix behavior of exp() and power() for infinity inputs.Tom Lane2020-06-14
| | | | | | | | | | | | | | | | | | | | | Previously, these functions tended to throw underflow errors for negative-infinity exponents. The correct thing per POSIX is to return 0, so let's do that instead. (Note that the SQL standard is silent on such issues, as it lacks the concepts of either Inf or NaN; so our practice is to follow POSIX whenever a corresponding C-library function exists.) Also, add a bunch of test cases verifying that exp() and power() actually do follow POSIX for Inf and NaN inputs. While this patch should guarantee that exp() passes the tests, power() will not unless the platform's pow(3) is fully POSIX-compliant. I already know that gaur fails some of the tests, and I am suspicious that the Windows animals will too; the extent of compliance of other old platforms remains to be seen. We might choose to drop failing test cases, or to work harder at overriding pow(3) for these cases, but first let's see just how good or bad the situation is. Discussion: https://postgr.es/m/582552.1591917752@sss.pgh.pa.us
* Add test coverage for EXTRACT()Peter Eisentraut2020-06-14
| | | | | | | | | | The variants for time and timetz had zero test coverage, the variant for interval only very little. This adds practically full coverage for those functions. Reviewed-by: Vik Fearing <vik@postgresfriends.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/c3306ac7-fcae-a1b8-1e30-6a379d605bcb%402ndquadrant.com
* Replace superuser check by ACLs for replication origin functionsMichael Paquier2020-06-14
| | | | | | | | | | | This patch removes the hardcoded check for superuser privileges when executing replication origin functions. Instead, execution is revoked from public, meaning that those functions can be executed by a superuser and that access to them can be granted. Author: Martín Marqués Reviewed-by: Kyotaro Horiguchi, Michael Paquier, Masahiko Sawada Discussion: https:/postgr.es/m/CAPdiE1xJMZOKQL3dgHMUrPqysZkgwzSMXETfKkHYnBAB7-0VRQ@mail.gmail.com
* Sync behavior of var_samp and stddev_samp for single NaN inputs.Tom Lane2020-06-13
| | | | | | | | | | | | | | | | | | | | var_samp(numeric) and stddev_samp(numeric) disagreed with their float cousins about what to do for a single non-null input value that is NaN. The float versions return NULL on the grounds that the calculation is only defined for more than one non-null input, which seems like the right answer. But the numeric versions returned NaN, as a result of dealing with edge cases in the wrong order. Fix that. The patch also gets rid of an insignificant memory leak in such cases. This inconsistency is of long standing, but on the whole it seems best not to back-patch the change into stable branches; nobody's complained and it's such an obscure point that nobody's likely to complain. (Note that v13 and v12 now contain test cases that will notice if we accidentally back-patch this behavior change in future.) Report and patch by me; thanks to Dean Rasheed for review. Discussion: https://postgr.es/m/353062.1591898766@sss.pgh.pa.us
* Fix behavior of float aggregates for single Inf or NaN inputs.Tom Lane2020-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When there is just one non-null input value, and it is infinity or NaN, aggregates such as stddev_pop and covar_pop should produce a NaN result, because the calculation is not well-defined. They used to do so, but since we adopted Youngs-Cramer aggregation in commit e954a727f, they produced zero instead. That's an oversight, so fix it. Add tests exercising these edge cases. Affected aggregates are var_pop(double precision) stddev_pop(double precision) var_pop(real) stddev_pop(real) regr_sxx(double precision,double precision) regr_syy(double precision,double precision) regr_sxy(double precision,double precision) regr_r2(double precision,double precision) regr_slope(double precision,double precision) regr_intercept(double precision,double precision) covar_pop(double precision,double precision) corr(double precision,double precision) Back-patch to v12 where the behavior change was accidentally introduced. Report and patch by me; thanks to Dean Rasheed for review. Discussion: https://postgr.es/m/353062.1591898766@sss.pgh.pa.us
* Silence _bt_check_unique compiler warning.Peter Geoghegan2020-06-13
| | | | | Reported-By: Tom Lane Discussion: https://postgr.es/m/841649.1592065060@sss.pgh.pa.us
* Refactor AlterExtensionContentsStmt grammarPeter Eisentraut2020-06-13
| | | | | | | Make use of the general object support already used by COMMENT, DROP, and SECURITY LABEL. Discussion: https://www.postgresql.org/message-id/flat/163c00a5-f634-ca52-fc7c-0e53deda8735%402ndquadrant.com
* Grammar object type refactoringPeter Eisentraut2020-06-13
| | | | | | | | | | Unify the grammar of COMMENT, DROP, and SECURITY LABEL further. They all effectively just take an object address for later processing, so we can make the grammar more generalized. Some extra checking about which object types are supported can be done later in the statement execution. Discussion: https://www.postgresql.org/message-id/flat/163c00a5-f634-ca52-fc7c-0e53deda8735%402ndquadrant.com
* Create by default sql/ and expected/ for output directory in pg_regressMichael Paquier2020-06-13
| | | | | | | | | | | | | | | | | | | | Using --outputdir with a custom output repository has never created by default the sql/ and expected/ paths generated with contents from respectively input/ and output/ if they don't exist, while the base output directory gets created if it does not exist. If sql/ and expected/ are not present, pg_regress would fail with the path missing, requiring test scripts to create those extra paths by themselves. This commit changes pg_regress so as both get created by default if they do not exist, removing the need for external test scripts to do so. This cleans up two code paths in the tree for pg_upgrade tests in MSVC and environments able to use test.sh. sql/ and expected/ were created as part of each test script, but this is not needed anymore as pg_regress handles the work now. Author: Roman Zharkov, Daniel Gustafsson Reviewed-by: Michael Paquier, Tom Lane Discussion: https://postgr.es/m/16484-4d89e9cc11241996@postgresql.org
* Add more TAP tests for pg_dump options with range checksMichael Paquier2020-06-13
| | | | | | | | This adds two tests for --extra-float-digits and --rows-per-insert, similar to what exists for --compress. Author: Dong Wook Lee Discussion: https://postgr.es/m/CAAcByaJsgrB-qc-ALb0mALprRGLAdmcBap7SZxO4kCAU-JEHcQ@mail.gmail.com
* Have pg_itoa, pg_ltoa and pg_lltoa return the length of the stringDavid Rowley2020-06-13
| | | | | | | | | | | Core by no means makes excessive use of these functions, but quite a large number of those usages do require the caller to call strlen() on the returned string. This is quite wasteful since these functions do already have a good idea of the length of the string, so we might as well just have them return that. Reviewed-by: Andrew Gierth Discussion: https://postgr.es/m/CAApHDvrm2A5x2uHYxsqriO2cUaGcFvND%2BksC9e7Tjep0t2RK_A%40mail.gmail.com
* Add missing extern keyword for a couple of numutils functionsDavid Rowley2020-06-13
| | | | | | | | | In passing, also remove a few surplus empty lines from pg_ltoa and pg_ulltoa_n in numutils.c Reported-by: Andrew Gierth Discussion: https://postgr.es/m/87y2ou3xuh.fsf@news-spur.riddles.org.uk Backpatch-through: 13, where these changes were introduced
* Avoid using a cursor in plpgsql's RETURN QUERY statement.Tom Lane2020-06-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | plpgsql has always executed the query given in a RETURN QUERY command by opening it as a cursor and then fetching a few rows at a time, which it turns around and dumps into the function's result tuplestore. The point of this was to keep from blowing out memory with an oversized SPITupleTable result (note that while a tuplestore can spill tuples to disk, SPITupleTable cannot). However, it's rather inefficient, both because of extra data copying and because of executor entry/exit overhead. In recent versions, a new performance problem has emerged: use of a cursor prevents use of a parallel plan for the executed query. We can improve matters by skipping use of a cursor and having the executor push result tuples directly into the function's result tuplestore. However, a moderate amount of new infrastructure is needed to make that idea work: * We can use the existing tstoreReceiver.c DestReceiver code to funnel executor output to the tuplestore, but it has to be extended to support plpgsql's requirement for possibly applying a tuple conversion map. * SPI needs to be extended to allow use of a caller-supplied DestReceiver instead of its usual receiver that puts tuples into a SPITupleTable. Two new API calls are needed to handle both the RETURN QUERY and RETURN QUERY EXECUTE cases. I also felt that I didn't want these new API calls to use the legacy method of specifying query parameter values with "char" null flags (the old ' '/'n' convention); rather they should accept ParamListInfo objects containing the parameter type and value info. This required a bit of additional new infrastructure since we didn't yet have any parse analysis callback that would interpret $N parameter symbols according to type data supplied in a ParamListInfo. There seems to be no harm in letting makeParamList install that callback by default, rather than leaving a new ParamListInfo's parserSetup hook as NULL. (Indeed, as of HEAD, I couldn't find anyplace that was using the parserSetup field at all; plpgsql was using parserSetupArg for its own purposes, but parserSetup seemed to be write-only.) We can actually get plpgsql out of the business of using legacy null flags altogether, and using ParamListInfo instead of its ad-hoc PreparedParamsData structure; but this requires inventing one more SPI API call that can replace SPI_cursor_open_with_args. That seems worth doing, though. SPI_execute_with_args and SPI_cursor_open_with_args are now unused anywhere in the core PG distribution. Perhaps someday we could deprecate/remove them. But cleaning up the crufty bits of the SPI API is a task for a different patch. Per bug #16040 from Jeremy Smith. This is unfortunately too invasive to consider back-patching. Patch by me; thanks to Hamid Akhtar for review. Discussion: https://postgr.es/m/16040-eaacad11fecfb198@postgresql.org
* Fix typos and some format mistakes in commentsMichael Paquier2020-06-12
| | | | | Author: Justin Pryzby Discussion: https://postgr.es/m/20200612023709.GC14879@telsasoft.com
* Make more use of RELKIND_HAS_STORAGE()Peter Eisentraut2020-06-12
| | | | | | | | Make use of RELKIND_HAS_STORAGE() where appropriate, instead of listing out the relkinds individually. No behavior change intended. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/7a22bf51-2480-d999-1794-191ba67ff47c%402ndquadrant.com
* Improve comments for [Heap]CheckForSerializableConflictOut().Thomas Munro2020-06-12
| | | | | | | | | | | | Rewrite the documentation of these functions, in light of recent bug fix commit 5940ffb2. Back-patch to 13 where the check-for-conflict-out code was split up into AM-specific and generic parts, and new documentation was added that now looked wrong. Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io
* doc: document problems with using xreflabel in XML docsBruce Momjian2020-06-11
| | | | | | | | Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/8315c0ca-7758-8823-fcb6-f37f9413e6b6@2ndquadrant.com Backpatch-through: master
* doc: remove xreflabels from commits 75fcdd2ae2 and 85af628da5Bruce Momjian2020-06-11
| | | | | | | | | xreflabels prevent references to the chapter numbers of sections id's. It should only be used in specific cases. Discussion: https://postgr.es/m/8315c0ca-7758-8823-fcb6-f37f9413e6b6@2ndquadrant.com Backpatch-through: 9.5
* Fix mishandling of NaN counts in numeric_[avg_]combine.Tom Lane2020-06-11
| | | | | | | | | | | | | | | | | | | | | | When merging two NumericAggStates, the code missed adding the new state's NaNcount unless its N was also nonzero; since those counts are independent, this is wrong. This would only have visible effect if some partial aggregate scans found only NaNs while earlier ones found only non-NaNs; then we could end up falsely deciding that there were no NaNs and fail to return a NaN final result as expected. That's pretty improbable, so it's no surprise this hasn't been reported from the field. Still, it's a bug. I didn't try to produce a regression test that would show the bug, but I did notice that these functions weren't being reached at all in our regression tests, so I improved the tests to at least exercise them. With these additions, I see pretty complete code coverage on the aggregation-related functions in numeric.c. Back-patch to 9.6 where this code was introduced. (I only added the improved test case as far back as v10, though, since the relevant part of aggregates.sql isn't there at all in 9.6.)
* Rework HashAgg GUCs.Jeff Davis2020-06-11
| | | | | | | | | | | | | | | | | | | Eliminate enable_groupingsets_hash_disk, which was primarily useful for testing grouping sets that use HashAgg and spill. Instead, hack the table stats to convince the planner to choose hashed aggregation for grouping sets that will spill to disk. Suggested by Melanie Plageman. Rename enable_hashagg_disk to hashagg_avoid_disk_plan, and invert the meaning of on/off. The new name indicates more strongly that it only affects the planner. Also, the word "avoid" is less definite, which should avoid surprises when HashAgg still needs to use the disk. Change suggested by Justin Pryzby, though I chose a different GUC name. Discussion: https://postgr.es/m/CAAKRu_aisiENMsPM2gC4oUY1hHG3yrCwY-fXUg22C6_MJUwQdA%40mail.gmail.com Discussion: https://postgr.es/m/20200610021544.GA14879@telsasoft.com Backpatch-through: 13
* Avoid update conflict out serialization anomalies.Peter Geoghegan2020-06-11
| | | | | | | | | | | | | | | | | | | | | | | | | | SSI's HeapCheckForSerializableConflictOut() test failed to correctly handle conditions involving a concurrently inserted tuple which is later concurrently updated by a separate transaction . A SELECT statement that called HeapCheckForSerializableConflictOut() could end up using the same XID (updater's XID) for both the original tuple, and the successor tuple, missing the XID of the xact that created the original tuple entirely. This only happened when neither tuple from the chain was visible to the transaction's MVCC snapshot. The observable symptoms of this bug were subtle. A pair of transactions could commit, with the later transaction failing to observe the effects of the earlier transaction (because of the confusion created by the update to the non-visible row). This bug dates all the way back to commit dafaa3ef, which added SSI. To fix, make sure that we check the xmin of concurrently inserted tuples that happen to also have been updated concurrently. Author: Peter Geoghegan Reported-By: Kyle Kingsbury Reviewed-By: Thomas Munro Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443@jepsen.io Backpatch: All supported versions
* pg_dump: Remove dead codePeter Eisentraut2020-06-11
| | | | | | Remove some code relevant only for dumping from pre-7.1 servers, support for which had already been removed by 64f3524e2c8deebc02808aa5ebdfa17859473add.
* Fix typos.Amit Kapila2020-06-11
| | | | | | | Reported-by: John Naylor Author: John Naylor Backpatch-through: 9.5 Discussion: https://postgr.es/m/CACPNZCtRuvs6G+EYqejhVJgBq2AKeZdXRVJsbX4syhO9gn5SNQ@mail.gmail.com
* Refactor DROP LANGUAGE grammarPeter Eisentraut2020-06-11
| | | | | | Fold it into the generic DropStmt. Discussion: https://www.postgresql.org/message-id/flat/163c00a5-f634-ca52-fc7c-0e53deda8735%402ndquadrant.com
* Remove deprecated syntax from CREATE/DROP LANGUAGEPeter Eisentraut2020-06-11
| | | | | | | | | | Remove the option to specify the language name as a single-quoted string. This has been obsolete since ee8ed85da3b. Removing it allows better grammar refactoring. The syntax of the CREATE FUNCTION LANGUAGE clause is not changed. Discussion: https://www.postgresql.org/message-id/flat/163c00a5-f634-ca52-fc7c-0e53deda8735%402ndquadrant.com
* Move frontend-side archive APIs from src/common/ to src/fe_utils/Michael Paquier2020-06-11
| | | | | | | | | | fe_archive.c was compiled only for the frontend in src/common/, but as it will never share anything with the backend, it makes most sense to move this file to src/fe_utils/. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/e9766d71-8655-ac86-bdf6-77e0e7169977@2ndquadrant.com Backpatch-through: 13
* Fold AlterForeignTableStmt into AlterTableStmtPeter Eisentraut2020-06-11
| | | | | | | All other relation types are handled by AlterTableStmt, so it's unnecessary to make a different statement for foreign tables. Discussion: https://www.postgresql.org/message-id/flat/163c00a5-f634-ca52-fc7c-0e53deda8735%402ndquadrant.com
* Remove redundant grammar symbolsPeter Eisentraut2020-06-10
| | | | | | | | | | access_method, database_name, and index_name are all just name, and they are not used consistently for their alleged purpose, so remove them. They have been around since ancient times but have no current reason for existing. Removing them can simplify future grammar refactoring. Discussion: https://www.postgresql.org/message-id/flat/163c00a5-f634-ca52-fc7c-0e53deda8735%402ndquadrant.com
* Change default of password_encryption to scram-sha-256Peter Eisentraut2020-06-10
| | | | | | | | | Also, the legacy values on/true/yes/1 for password_encryption that mapped to md5 are removed. The only valid values are now scram-sha-256 and md5. Reviewed-by: Jonathan S. Katz <jkatz@postgresql.org> Discussion: https://www.postgresql.org/message-id/flat/d5b0ad33-7d94-bdd1-caac-43a1c782cab2%402ndquadrant.com
* Update description of parameter password_encryptionPeter Eisentraut2020-06-10
| | | | | | | The previous description string still described the pre-PostgreSQL 10 (pre eb61136dc75a76caef8460fa939244d8593100f2) behavior of selecting between encrypted and unencrypted, but it is now choosing between encryption algorithms.
* Fix ReorderBuffer memory overflow check.Amit Kapila2020-06-10
| | | | | | | | | | | | | | | | | | | | | Commit cec2edfa78 introduced logical_decoding_work_mem to limit ReorderBuffer memory usage. We spill the changes once the memory occupied by changes exceeds logical_decoding_work_mem. There was an assumption in the code that by evicting the largest (sub)transaction we will come under the memory limit as the selected transaction will be at least as large as the most recent change (which caused us to go over the memory limit). However, that is not true because a user can reduce the logical_decoding_work_mem to a smaller value before the most recent change. We fix it by allowing to evict the transactions until we reach under the memory limit. Reported-by: Fujii Masao Author: Amit Kapila Reviewed-by: Fujii Masao Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/2b7ba291-22e0-a187-d167-9e5309a3458d@oss.nttdata.com
* Spelling adjustmentsPeter Eisentraut2020-06-09
| | | | similar to 0fd2a79a637f9f96b9830524823df0454e962f96
* Unify drop-by-OID functionsPeter Eisentraut2020-06-09
| | | | | | | | | | | | | There are a number of Remove${Something}ById() functions that are essentially identical in structure and only different in which catalog they are working on. Refactor this to be one generic function. The information about which oid column, index, etc. to use was already available in ObjectProperty for most catalogs, in a few cases it was easily added. Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/331d9661-1743-857f-1cbb-d5728bcd62cb%402ndquadrant.com
* Fix invalid function references in a few commentsDavid Rowley2020-06-09
| | | | | | | These appear to have been forgotten when the functions were renamed in 1fd687a03. Backpatch-through: 13, where the functions were renamed
* Repair unstable regression test.Tom Lane2020-06-09
| | | | | | | | | | | | Commit 0c882e52a tried to force table atest12 to have more-accurate- than-default statistics; but transiently setting default_statistics_target isn't enough for that, because autovacuum could come along and overwrite the stats later. This evidently explains some intermittent buildfarm failures we've seen since then. Repair by disabling autovac on this table. Thanks to David Rowley for correctly diagnosing the cause. Discussion: https://postgr.es/m/CA+hUKG+OUkQSOUTg=qo=S=fWa_tbm99i7rB7mfbHz1SYm4v-jQ@mail.gmail.com
* Fix HashAgg regression from choosing too many initial buckets.Jeff Davis2020-06-08
| | | | | | | | Diagnosis by Andres. Reported-by: Pavel Stehule Discussion: https://postgr.es/m/CAFj8pRDLVakD5Aagt3yZeEQeTeEWaS3YE5h8XC3Q3qJ6TYkc2Q%40mail.gmail.com Backpatch-through: 13
* Avoid need for valgrind suppressions for pg_atomic_init_u64 on some platforms.Andres Freund2020-06-08
| | | | | | | | | | | | | | | | | | | | Previously we used pg_atomic_write_64_impl inside pg_atomic_init_u64. That works correctly, but on platforms without 64bit single copy atomicity it could trigger spurious valgrind errors about uninitialized memory, because we use compare_and_swap for atomic writes on such platforms. I previously suppressed one instance of this problem (6c878edc1df), but as Tom reports that wasn't enough. As the atomic variable cannot yet be concurrently accessible during initialization, it seems better to have pg_atomic_init_64_impl set the value directly. Change pg_atomic_init_u32_impl for symmetry. Reported-By: Tom Lane Author: Andres Freund Discussion: https://postgr.es/m/1714601.1591503815@sss.pgh.pa.us Backpatch: 9.5-
* Update documentation for snowball updatePeter Eisentraut2020-06-08
| | | | Discussion: https://www.postgresql.org/message-id/flat/a8eeabd6-2be1-43fe-401e-a97594c38478%402ndquadrant.com
* Update snowballPeter Eisentraut2020-06-08
| | | | | | | Update to snowball tag v2.0.0. Major changes are new stemmers for Basque, Catalan, and Hindi. Discussion: https://www.postgresql.org/message-id/flat/a8eeabd6-2be1-43fe-401e-a97594c38478%402ndquadrant.com
* Fix locking bugs that could corrupt pg_control.Thomas Munro2020-06-08
| | | | | | | | | | | | | | | | | | The redo routines for XLOG_CHECKPOINT_{ONLINE,SHUTDOWN} must acquire ControlFileLock before modifying ControlFile->checkPointCopy, or the checkpointer could write out a control file with a bad checksum. Likewise, XLogReportParameters() must acquire ControlFileLock before modifying ControlFile and calling UpdateControlFile(). Back-patch to all supported releases. Author: Nathan Bossart <bossartn@amazon.com> Author: Fujii Masao <masao.fujii@oss.nttdata.com> Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/70BF24D6-DC51-443F-B55A-95735803842A%40amazon.com
* Doc: Update example symptom of systemd misconfiguration.Thomas Munro2020-06-08
| | | | | | | | | | | | In PostgreSQL 10, we stopped using System V semaphores on Linux systems. Update the example we give of an error message from a misconfigured system to show what people are most likely to see these days. Back-patch to 10, where PREFERRED_SEMAPHORES=UNNAMED_POSIX arrived. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKGLmJUSwybaPQv39rB8ABpqJq84im2UjZvyUY4feYhpWMw%40mail.gmail.com
* Fix crash in WAL sender when starting physical replicationMichael Paquier2020-06-08
| | | | | | | | | | | | | | | | | | | | | Since database connections can be used with WAL senders in 9.4, it is possible to use physical replication. This commit fixes a crash when starting physical replication with a WAL sender using a database connection, caused by the refactoring done in 850196b. There have been discussions about forbidding the use of physical replication in a database connection, but this is left for later, taking care only of the crash new to 13. While on it, add a test to check for a failure when attempting logical replication if the WAL sender does not have a database connection. This part is extracted from a larger patch by Kyotaro Horiguchi. Reported-by: Vladimir Sitnikov Author: Michael Paquier, Kyotaro Horiguchi Reviewed-by: Kyotaro Horiguchi, Álvaro Herrera Discussion: https://postgr.es/m/CAB=Je-GOWMj1PTPkeUhjqQp-4W3=nW-pXe2Hjax6rJFffB5_Aw@mail.gmail.com Backpatch-through: 13
* MSVC: Avoid warning when testing a TAP suite without PROVE_FLAGS.Noah Misch2020-06-07
| | | | | | | Commit 7be5d8df1f74b78620167d3abf32ee607e728919 surfaced the logic error, which had no functional implications, by adding "use warnings". The buildfarm always customizes PROVE_FLAGS, so the warning did not appear there. Back-patch to 9.5 (all supported versions).
* Stamp HEAD as 14devel.Tom Lane2020-06-07
| | | | Let the hacking begin ...
* pgindent run prior to branching v13.Tom Lane2020-06-07
| | | | | pgperltidy and reformat-dat-files too, though those didn't find anything to change.
* Try to read data from the socket in pqSendSome's write_failed paths.Tom Lane2020-06-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Even when we've concluded that we have a hard write failure on the socket, we should continue to try to read data. This gives us an opportunity to collect any final error message that the backend might have sent before closing the connection; moreover it is the job of pqReadData not pqSendSome to close the socket once EOF is detected. Due to an oversight in 1f39a1c06, pqSendSome failed to try to collect data in the case where we'd already set write_failed. The problem was masked for ordinary query operations (which really only make one write attempt anyway), but COPY to the server would continue to send data indefinitely after a mid-COPY connection loss. Hence, add pqReadData calls into the paths where pqSendSome drops data because of write_failed. If we've lost the connection, this will eventually result in closing the socket and setting CONNECTION_BAD, which will cause PQputline and siblings to report failure, allowing the application to terminate the COPY sooner. (Basically this restores what happened before 1f39a1c06.) There are related issues that this does not solve; for example, if the backend sends an error but doesn't drop the connection, we did and still will keep pumping COPY data as long as the application sends it. Fixing that will require application-visible behavior changes though, and anyway it's an ancient behavior that we've had few complaints about. For now I'm just trying to fix the regression from 1f39a1c06. Per a complaint from Andres Freund. Back-patch into v12 where 1f39a1c06 came in. Discussion: https://postgr.es/m/20200603201242.ofvm4jztpqytwfye@alap3.anarazel.de
* Rethink definition of cancel.c's CancelRequested flag.Tom Lane2020-06-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As it stands, this flag is only set when we've successfully sent a cancel request, not if we get SIGINT and then fail to send a cancel. However, for almost all callers, that's the Wrong Thing: we'd prefer to abort processing after control-C even if no cancel could be sent. As an example, since commit 1d468b9ad "pgbench -i" fails to give up sending COPY data even after control-C, if the postmaster has been stopped, which is clearly not what the code intends and not what anyone would want. (The fact that it keeps going at all is the fault of a separate bug in libpq, but not letting CancelRequested become set is clearly not what we want here.) The sole exception, as far as I can find, is that scripts_parallel.c's ParallelSlotsGetIdle tries to consume a query result after issuing a cancel, which of course might not terminate quickly if no cancel happened. But that behavior was poorly thought out too. No user of ParallelSlotsGetIdle tries to continue processing after a cancel, so there is really no point in trying to clear the connection's state. Moreover this has the same defect as for other users of cancel.c, that if the cancel request fails for some reason then we end up with control-C being completely ignored. (On top of that, select_loop failed to distinguish clearly between SIGINT and other reasons for select(2) failing, which means that it's possible that the existing code would think that a cancel has been sent when it hasn't.) Hence, redefine CancelRequested as simply meaning that SIGINT was received. We could add a second flag with the other meaning, but in the absence of any compelling argument why such a flag is needed, I think it would just offer an opportunity for future callers to get it wrong. Also remove the consumeQueryResult call in ParallelSlotsGetIdle's failure exit. In passing, simplify the API of select_loop. It would now be possible to re-unify psql's cancel_pressed with CancelRequested, partly undoing 5d43c3c54. But I'm not really convinced that that's worth the trouble, so I left psql alone, other than fixing a misleading comment. This code is new in v13 (cf a4fd3aa71), so no need for back-patch. Per investigation of a complaint from Andres Freund. Discussion: https://postgr.es/m/20200603201242.ofvm4jztpqytwfye@alap3.anarazel.de
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-04 11:03:06 +0000