aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Future-proof regression tests against possibly-missing posixrules file.Tom Lane2020-06-19
| | | | | | | | | | | | | | | | | | | | | | | | | | The IANA time zone folk have deprecated use of a "posixrules" file in the tz database. While for now it's our choice whether to keep supplying one in our own builds, installations built with --with-system-tzdata will soon be needing to cope with that file not being present, at least on some platforms. This causes a problem for the horology test, which expected the nonstandard POSIX zone spec "CST7CDT" to apply pre-2007 US daylight savings rules. That does happen if the posixrules file supplies such information, but otherwise the test produces undesired results. To fix, add an explicit transition date rule that matches 2005 practice. (We could alternatively have switched the test to use some real time zone, but it seems useful to have coverage of this type of zone spec.) While at it, update a documentation example that also relied on "CST7CDT"; use a real-world zone name instead. Also, document why the zone names EST5EDT, CST6CDT, MST7MDT, PST8PDT aren't subject to similar failures when "posixrules" is missing. Back-patch to all supported branches, since the hazard is the same for all. Discussion: https://postgr.es/m/1665379.1592581287@sss.pgh.pa.us
* Fix deduplication "single value" strategy bug.Peter Geoghegan2020-06-19
| | | | | | | | | | | | | | | | It was possible for deduplication's single value strategy to mistakenly believe that a very small duplicate tuple counts as one of the six large tuples that it aims to leave behind after the page finally splits. This could cause slightly suboptimal space utilization with very low cardinality indexes, though only under fairly narrow conditions. To fix, be particular about what kind of tuple counts as a maxpostingsize-capped tuple. This avoids confusion in the event of a small tuple that gets "wedged" between two large tuples, where all tuples on the page are duplicates of the same value. Discussion: https://postgr.es/m/CAH2-Wz=Y+sgSFc-O3LpiZX-POx2bC+okec2KafERHuzdVa7-rQ@mail.gmail.com Backpatch: 13-, where deduplication was introduced (by commit 0d861bbb)
* Fix issues in invalidation of obsolete replication slots.Fujii Masao2020-06-19
| | | | | | | | | | | | | | | | | | | | | | This commit fixes the following issues. 1. There is the case where the slot is dropped while trying to invalidate it. InvalidateObsoleteReplicationSlots() did not handle this case, and which could cause checkpoint to fail. 2. InvalidateObsoleteReplicationSlots() could emit the same log message multiple times unnecessary. It should be logged only once. 3. When marking the slot as used, we always searched the target slot from all the replication slots even if we already found it. This could cause useless waste of cycles. Back-patch to v13 where these issues were added as a part of max_slot_wal_keep_size code. Author: Fujii Masao Reviewed-by: Kyotaro Horiguchi, Alvaro Herrera Discussion: https://postgr.es/m/66c05b67-3396-042c-1b41-bfa6c3ddcf82@oss.nttdata.com
* Fix EXPLAIN ANALYZE for parallel HashAgg plansDavid Rowley2020-06-19
| | | | | | | | | | | | | | | Since 1f39bce02, HashAgg nodes have had the ability to spill to disk when memory consumption exceeds work_mem. That commit added new properties to EXPLAIN ANALYZE to show the maximum memory usage and disk usage, however, it didn't quite go as far as showing that information for parallel workers. Since workers may have experienced something very different from the main process, we should show this information per worker, as is done in Sort. Reviewed-by: Justin Pryzby Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/CAApHDvpEKbfZa18mM1TD7qV6PG+w97pwCWq5tVD0dX7e11gRJw@mail.gmail.com Backpatch-through: 13, where the hashagg spilling code was added.
* Fix deadlock danger when atomic ops are done under spinlock.Andres Freund2020-06-18
| | | | | | | | | | | | | | | | | This was a danger only for --disable-spinlocks in combination with atomic operations unsupported by the current platform. While atomics.c was careful to signal that a separate semaphore ought to be used when spinlock emulation is active, spin.c didn't actually implement that mechanism. That's my (Andres') fault, it seems to have gotten lost during the development of the atomic operations support. Fix that issue and add test for nesting atomic operations inside a spinlock. Author: Andres Freund Discussion: https://postgr.es/m/20200605023302.g6v3ydozy5txifji@alap3.anarazel.de Backpatch: 9.5-
* Add basic spinlock tests to regression tests.Andres Freund2020-06-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | As s_lock_test, the already existing test for spinlocks, isn't run in an automated fashion (and doesn't test a normal backend environment), adding tests that are run as part of a normal regression run is a good idea. Particularly in light of several recent and upcoming spinlock related fixes. Currently the new tests are run as part of the pre-existing test_atomic_ops() test. That perhaps can be quibbled about, but for now seems ok. The only operations that s_lock_test tests but the new tests don't are the detection of a stuck spinlock and S_LOCK_FREE (which is otherwise unused, not implemented on all platforms, and will be removed). This currently contains a test for more than INT_MAX spinlocks (only run with --disable-spinlocks), to ensure the recent commit fixing a bug with more than INT_MAX spinlock initializations is correct. That test is somewhat slow, so we might want to disable it after a few days. It might be worth retiring s_lock_test after this. The added coverage of a stuck spinlock probably isn't worth the added complexity? Author: Andres Freund Discussion: https://postgr.es/m/20200606023103.avzrctgv7476xj7i@alap3.anarazel.de
* Fix oldest xmin and LSN computation across repslots after advancingMichael Paquier2020-06-18
| | | | | | | | | | | | | | | | | Advancing a replication slot did not recompute the oldest xmin and LSN values across replication slots, preventing resource removal like segments not recycled at checkpoint time. The original commit that introduced the slot advancing in 9c7d06d never did the update of those oldest values, and b0afdca removed this code. This commit adds a TAP test to check segment recycling with advancing for physical slots, enforcing an extra segment switch before advancing to check if the segment gets correctly recycled after a checkpoint. Reported-by: Andres Freund Reviewed-by: Alexey Kondratov, Kyptaro Horiguchi Discussion: https://postgr.es/m/20200609171904.kpltxxvjzislidks@alap3.anarazel.de Backpatch-through: 11
* Sync our copy of the timezone library with IANA release tzcode2020a.Tom Lane2020-06-17
| | | | | | | | | | | | | | | | | | | This absorbs a leap-second-related bug fix in localtime.c, and teaches zic to handle an expiration marker in the leapseconds file. Neither are of any interest to us (for the foreseeable future anyway), but we need to stay more or less in sync with upstream. Also adjust some over-eager changes in the README from commit 957338418. I have no intention of making changes that require C99 in this code, until such time as all the live back branches require C99. Otherwise back-patching will get too exciting. For the same reason, absorb assorted whitespace and other cosmetic changes from HEAD into the back branches; mostly this reflects use of improved versions of pgindent. All in all then, quite a boring update. But I figured I'd get it done while I was looking at this code.
* Fix nbtree.h dedup state comment.Peter Geoghegan2020-06-17
| | | | Oversight in commit 0d861bbb.
* spinlock emulation: Fix bug when more than INT_MAX spinlocks are initialized.Andres Freund2020-06-17
| | | | | | | | | | Once the counter goes negative we ended up with spinlocks that errored out on first use (due to check in tas_sema). Author: Andres Freund Reviewed-By: Robert Haas Discussion: https://postgr.es/m/20200606023103.avzrctgv7476xj7i@alap3.anarazel.de Backpatch: 9.5-
* Avoid potential spinlock in a signal handler as part of global barriers.Andres Freund2020-06-17
| | | | | | | | | | | | | | | | | | | | | | On platforms without support for 64bit atomic operations where we also cannot rely on 64bit reads to have single copy atomicity, such atomics are implemented using a spinlock based fallback. That means it's not safe to even read such atomics from within a signal handler (since the signal handler might run when the spinlock already is held). To avoid this issue defer global barrier processing out of the signal handler. Instead of checking local / shared barrier generation to determine whether to set ProcSignalBarrierPending, introduce PROCSIGNAL_BARRIER and always set ProcSignalBarrierPending when receiving such a signal. Additionally avoid redundant work in ProcessProcSignalBarrier if ProcSignalBarrierPending is unnecessarily. Also do a small amount of other polishing. Author: Andres Freund Reviewed-By: Robert Haas Discussion: https://postgr.es/m/20200609193723.eu5ilsjxwdpyxhgz@alap3.anarazel.de Backpatch: 13-, where the code was introduced.
* Fix file reference in nls.mkPeter Eisentraut2020-06-16
| | | | Broken by move of fe_archive.c to fe_utils.
* Fix buffile.c error handling.Thomas Munro2020-06-16
| | | | | | | | | | | | | | | | | | Convert buffile.c error handling to use ereport. This fixes cases where I/O errors were indistinguishable from EOF or not reported. Also remove "%m" from error messages where errno would be bogus. While we're modifying those strings, add block numbers and short read byte counts where appropriate. Back-patch to all supported releases. Reported-by: Amit Khandekar <amitdkhan.pg@gmail.com> Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CA%2BhUKGJE04G%3D8TLK0DLypT_27D9dR8F1RQgNp0jK6qR0tZGWOw%40mail.gmail.com
* pg_upgrade: set vacuum_defer_cleanup_age to zeroBruce Momjian2020-06-15
| | | | | | | | | | | | Non-zero vacuum_defer_cleanup_age values cause pg_upgrade freezing of the system catalogs to be incomplete, or do nothing. This will cause the upgrade to fail in confusing ways. Reported-by: Laurenz Albe Discussion: https://postgr.es/m/7d6f6c22ba05ce0c526e9e8b7bfa8105e7da45e6.camel@cybertec.at Backpatch-through: 9.5
* 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
* 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
* 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
* 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
* 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
* 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
* 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
* 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-
* 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
* 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).
* 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
* Fix platform-specific performance regression in logtape.c.Jeff Davis2020-06-07
| | | | | | | | | | | | | | | | Commit 24d85952 made a change that indirectly caused a performance regression by triggering a change in the way GCC optimizes memcpy() on some platforms. The behavior seemed to contradict a GCC document, so I filed a report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95556 This patch implements a narrow workaround which eliminates the regression I observed. The workaround is benign enough that it seems unlikely to cause a different regression on another platform. Discussion: https://postgr.es/m/99b2eab335c1592c925d8143979c8e9e81e1575f.camel@j-davis.com
* psql: Format \? output a little betterPeter Eisentraut2020-06-07
|
* Fix message translatabilityPeter Eisentraut2020-06-07
| | | | | Two parts of the same message shouldn't be split across two function calls.
* Spelling adjustmentsPeter Eisentraut2020-06-07
|
* Formatting and punctuation improvements in postgresql.conf.samplePeter Eisentraut2020-06-07
|
* Add missing source files to nls.mkPeter Eisentraut2020-06-06
|
* Refresh function name in CRC-associated Valgrind suppressions.Noah Misch2020-06-05
| | | | | | | | | Back-patch to 9.5, where commit 4f700bcd20c087f60346cb8aefd0e269be8e2157 first appeared. Reviewed by Tom Lane. Reported by Andrew Dunstan. Discussion: https://postgr.es/m/4dfabec2-a3ad-0546-2d62-f816c97edd0c@2ndQuadrant.com
* Improve ineq_histogram_selectivity's behavior for non-default orderings.Tom Lane2020-06-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ineq_histogram_selectivity() can be invoked in situations where the ordering we care about is not that of the column's histogram. We could be considering some other collation, or even more drastically, the query operator might not agree at all with what was used to construct the histogram. (We'll get here for anything using scalarineqsel-based estimators, so that's quite likely to happen for extension operators.) Up to now we just ignored this issue and assumed we were dealing with an operator/collation whose sort order exactly matches the histogram, possibly resulting in junk estimates if the binary search gets confused. It's past time to improve that, since the use of nondefault collations is increasing. What we can do is verify that the given operator and collation match what's recorded in pg_statistic, and use the existing code only if so. When they don't match, instead execute the operator against each histogram entry, and take the fraction of successes as our selectivity estimate. This gives an estimate that is probably good to about 1/histogram_size, with no assumptions about ordering. (The quality of the estimate is likely to degrade near the ends of the value range, since the two orderings probably don't agree on what is an extremal value; but this is surely going to be more reliable than what we did before.) At some point we might further improve matters by storing more than one histogram calculated according to different orderings. But this code would still be good fallback logic when no matches exist, so that is not an argument for not doing this. While here, also improve get_variable_range() to deal more honestly with non-default collations. This isn't back-patchable, because it requires adding another argument to ineq_histogram_selectivity, and because it might have significant impact on the estimation results for extension operators relying on scalarineqsel --- mostly for the better, one hopes, but in any case destabilizing plan choices in back branches is best avoided. Per investigation of a report from James Lucas. Discussion: https://postgr.es/m/CAAFmbbOvfi=wMM=3qRsPunBSLb8BFREno2oOzSBS=mzfLPKABw@mail.gmail.com
* Add unlikely() to CHECK_FOR_INTERRUPTS()Joe Conway2020-06-05
| | | | | | | Add the unlikely() branch hint macro to CHECK_FOR_INTERRUPTS(). Backpatch to REL_10_STABLE where we first started using unlikely(). Discussion: https://www.postgresql.org/message-id/flat/8692553c-7fe8-17d9-cbc1-7cddb758f4c6%40joeconway.com
* Use query collation, not column's collation, while examining statistics.Tom Lane2020-06-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 5e0928005 changed the planner so that, instead of blindly using DEFAULT_COLLATION_OID when invoking operators for selectivity estimation, it would use the collation of the column whose statistics we're considering. This was recognized as still being not quite the right thing, but it seemed like a good incremental improvement. However, shortly thereafter we introduced nondeterministic collations, and that creates cases where operators can fail if they're passed the wrong collation. We don't want planning to fail in cases where the query itself would work, so this means that we *must* use the query's collation when invoking operators for estimation purposes. The only real problem this creates is in ineq_histogram_selectivity, where the binary search might produce a garbage answer if we perform comparisons using a different collation than the column's histogram is ordered with. However, when the query's collation is significantly different from the column's default collation, the estimate we previously generated would be pretty irrelevant anyway; so it's not clear that this will result in noticeably worse estimates in practice. (A follow-on patch will improve this situation in HEAD, but it seems too invasive for back-patch.) The patch requires changing the signatures of mcv_selectivity and allied functions, which are exported and very possibly are used by extensions. In HEAD, I just did that, but an API/ABI break of this sort isn't acceptable in stable branches. Therefore, in v12 the patch introduces "mcv_selectivity_ext" and so on, with signatures matching HEAD, and makes the old functions into wrappers that assume DEFAULT_COLLATION_OID should be used. That does not match the prior behavior, but it should avoid risk of failure in most cases. (In practice, I think most extension datatypes aren't collation-aware, so the change probably doesn't matter to them.) Per report from James Lucas. Back-patch to v12 where the problem was introduced. Discussion: https://postgr.es/m/CAAFmbbOvfi=wMM=3qRsPunBSLb8BFREno2oOzSBS=mzfLPKABw@mail.gmail.com
* OpenSSL 3.0.0 compatibility in testsPeter Eisentraut2020-06-05
| | | | | | | | | | | | | | DES has been deprecated in OpenSSL 3.0.0 which makes loading keys encrypted with DES fail with "fetch failed". Solve by changing the cipher used to aes256 which has been supported since 1.0.1 (and is more realistic to use anyways). Note that the minimum supported OpenSSL version is 1.0.1 as of 7b283d0e1d1d79bf1c962d790c94d2a53f3bb38a, so this does not introduce any new version requirements. Author: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/FEF81714-D479-4512-839B-C769D2605F8A%40yesql.se
* Preserve pg_index.indisreplident across REINDEX CONCURRENTLYMichael Paquier2020-06-05
| | | | | | | | | | | If the flag value is lost, logical decoding would work the same way as REPLICA IDENTITY NOTHING, meaning that no old tuple values would be included in the changes anymore produced by logical decoding. Author: Michael Paquier Reviewed-by: Euler Taveira Discussion: https://postgr.es/m/20200603065340.GK89559@paquier.xyz Backpatch-through: 12
* Reject "23:59:60.nnn" in datetime input.Tom Lane2020-06-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | It's intentional that we don't allow values greater than 24 hours, while we do allow "24:00:00" as well as "23:59:60" as inputs. However, the range check was miscoded in such a way that it would accept "23:59:60.nnn" with a nonzero fraction. For time or timetz, the stored result would then be greater than "24:00:00" which would fail dump/reload, not to mention possibly confusing other operations. Fix by explicitly calculating the result and making sure it does not exceed 24 hours. (This calculation is redundant with what will happen later in tm2time or tm2timetz. Maybe someday somebody will find that annoying enough to justify refactoring to avoid the duplication; but that seems too invasive for a back-patched bug fix, and the cost is probably unmeasurable anyway.) Note that this change also rejects such input as the time portion of a timestamp(tz) value. Back-patch to v10. The bug is far older, but to change this pre-v10 we'd need to ensure that the logic behaves sanely with float timestamps, which is possibly nontrivial due to roundoff considerations. Doesn't really seem worth troubling with. Per report from Christoph Berg. Discussion: https://postgr.es/m/20200520125807.GB296739@msg.df7cb.de
* psql: Clean up terminology in \dAp commandPeter Eisentraut2020-06-04
| | | | | | The preferred terminology has been support "function", not procedure, for some time, so change that over. The command stays \dAp, since \dAf is already something else.