aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix misuse of table_index_fetch_tuple_check().Peter Geoghegan2020-06-25
| | | | | | | | | | | | | Commit 0d861bbb, which added deduplication to nbtree, had _bt_check_unique() pass a TID to table_index_fetch_tuple_check() that isn't safe to mutate. table_index_fetch_tuple_check()'s tid argument is modified when the TID in question is not the latest visible tuple in a hot chain, though this wasn't documented. To fix, go back to using a local copy of the TID in _bt_check_unique(), and update comments above table_index_fetch_tuple_check(). Backpatch: 13-, where B-Tree deduplication was introduced.
* Remove erroneous assertion from pg_copy_logical_replication_slot().Fujii Masao2020-06-25
| | | | | | | | | | | | | | | | | | | | | | | | If restart_lsn of logical replication slot gets behind more than max_slot_wal_keep_size from the current LSN, the logical replication slot would be invalidated and its restart_lsn is reset to an invalid LSN. If this logical replication slot with an invalid restart_lsn was specified as the source slot in pg_copy_logical_replication_slot(), the function caused the assertion failure unexpectedly. This assertion was added because restart_lsn should not be invalid before. But in v13, it can be invalid thanks to max_slot_wal_keep_size. So since this assertion is no longer useful, this commit removes it. This commit also changes the errcode in the error message that pg_copy_logical_replication_slot() emits when the slot with an invalid restart_lsn is specified, to more appropriate one. Back-patch to v13 where max_slot_wal_keep_size was added and the assertion was no longer valid. Author: Fujii Masao Reviewed-by: Alvaro Herrera, Kyotaro Horiguchi Discussion: https://postgr.es/m/f91de4fb-a7ab-b90e-8132-74796e049d51@oss.nttdata.com
* Fix compiler warning induced by commit d8b15eeb8.Tom Lane2020-06-24
| | | | | | | | | | | | | | I forgot that INT64_FORMAT can't be used with sscanf on Windows. Use the same trick of sscanf'ing into a temp variable as we do in some other places in zic.c. The upstream IANA code avoids the portability problem by relying on <inttypes.h>'s SCNdFAST64 macro. Once we're requiring C99 in all branches, we should do likewise and drop this set of diffs from upstream. For now, though, a hack seems fine, since we do not actually care about leapseconds anyway. Discussion: https://postgr.es/m/4e5d1a5b-143e-e70e-a99d-a3b01c1ae7c3@2ndquadrant.com
* Adjust max_slot_wal_keep_size behavior per reviewAlvaro Herrera2020-06-24
| | | | | | | | | | | | | | | | | | | | | | | | | | In pg_replication_slot, change output from normal/reserved/lost to reserved/extended/unreserved/ lost, which better expresses the possible states particularly near the time where segments are no longer safe but checkpoint has not run yet. Under the new definition, reserved means the slot is consuming WAL that's still under the normal WAL size constraints; extended means it's consuming WAL that's being protected by wal_keep_segments or the slot itself, whose size is below max_slot_wal_keep_size; unreserved means the WAL is no longer safe, but checkpoint has not yet removed those files. Such as slot is in imminent danger, but can still continue for a little while and may catch up to the reserved WAL space. Also, there were some bugs in the calculations used to report the status; fixed those. Backpatch to 13. Reported-by: Fujii Masao <masao.fujii@oss.nttdata.com> Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20200616.120236.1809496990963386593.horikyota.ntt@gmail.com
* Save slot's restart_lsn when invalidated due to sizeAlvaro Herrera2020-06-24
| | | | | | | | | | | | | We put it aside as invalidated_at, which let us show "lost" in pg_replication slot. Prior to this change, the state value was reported as NULL. Backpatch to 13. Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20200617.101707.1735599255100002667.horikyota.ntt@gmail.com Discussion: https://postgr.es/m/20200407.120905.1507671100168805403.horikyota.ntt@gmail.com
* Add parens to ConvertToXSegs macroAlvaro Herrera2020-06-24
| | | | | | | The current definition is dangerous. No bugs exist in our code at present, but backpatch to 11 nonetheless where it was introduced. Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
* Fix comment in heap.cMichael Paquier2020-06-24
| | | | | | | | | | The description of InsertPgAttributeTuple() does not match its handling of pg_attribute contents with NULL values for a long time, with 911e702 making things more inconsistent. This adjusts the description to match the reality. Author: Daniel Gustafsson Discussion: https://postgr.es/m/4E4E4B33-9FDF-4D21-B77A-642D027AEAD9@yesql.se
* Undo double-quoting of index names in non-text EXPLAIN output formats.Tom Lane2020-06-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | explain_get_index_name() applied quote_identifier() to the index name. This is fine for text output, but the non-text output formats all have their own quoting conventions and would much rather start from the actual index name. For example in JSON you'd get something like "Index Name": "\"My Index\"", which is surely not desirable, especially when the same does not happen for table names. Hence, move the responsibility for applying quoting out to the callers, where it can go into already-existing special code paths for text format. This changes the API spec for users of explain_get_index_name_hook: before, they were supposed to apply quote_identifier() if necessary, now they should not. Research suggests that the only publicly available user of the hook is hypopg, and it actually forgot to apply quoting anyway, so it's fine. (In any case, there's no behavioral change for the output of a hook as seen in non-text EXPLAIN formats, so this won't break any case that programs should be relying on.) Digging in the commit logs, it appears that quoting was included in explain_get_index_name's duties when commit 604ffd280 invented it; and that was fine at the time because we only had text output format. This should have been rethought when non-text formats were invented, but it wasn't. This is a fairly clear bug for users of non-text EXPLAIN formats, so back-patch to all supported branches. Per bug #16502 from Maciek Sakrejda. Patch by me (based on investigation by Euler Taveira); thanks to Julien Rouhaud for review. Discussion: https://postgr.es/m/16502-57bd1c9f913ed1d1@postgresql.org
* Add --no-index-cleanup and --no-truncate to vacuumdb.Michael Paquier2020-06-22
| | | | | | | | | | | Both INDEX_CLEANUP and TRUNCATE have been available since v12, and are enabled by default except if respectively vacuum_index_cleanup and vacuum_truncate are disabled for a given relation. This change adds support for disabling these options from vacuumdb. Author: Nathan Bossart Reviewed-by: Michael Paquier, Masahiko Sawada Discussion: https://postgr.es/m/6F7F17EF-B1F2-4681-8D03-BA96365717C0@amazon.com
* Fix masking of SP-GiST pages during xlog consistency checkAlexander Korotkov2020-06-20
| | | | | | | | | | spg_mask() didn't take into account that pd_lower equal to SizeOfPageHeaderData is still valid value. This commit fixes that. Backpatch to 11, where spg_mask() pg_lower check was introduced. Reported-by: Michael Paquier Discussion: https://postgr.es/m/20200615131405.GM52676%40paquier.xyz Backpatch-through: 11
* Remove dead forceSync parameter of XactLogCommitRecord().Noah Misch2020-06-20
| | | | | | | | | | | | The function has been reading global variable forceSyncCommit, mirroring the intent of the caller that passed forceSync=forceSyncCommit. The other caller, RecordTransactionCommitPrepared(), passed false. Since COMMIT PREPARED can't share a transaction with any command, it certainly doesn't share a transaction with a command that sets forceSyncCommit. Reviewed by Michael Paquier. Discussion: https://postgr.es/m/20200617032615.GC2916904@rfd.leadboat.com
* Removal unused function parameter in CopyReadBinaryAttribute.Amit Kapila2020-06-20
| | | | | | | | | | | | The function parameter column_no is not used in CopyReadBinaryAttribute, this can be removed. Commit 0e319c7ad7 removed the usage of column_no parameter in function CopyReadBinaryAttribute but forgot to remove the parameter. Reported-by: Vignesh C Author: Vignesh C Discussion: https://postgr.es/m/CALDaNm1TYSNTfqx_jfz9_mwEZ2Er=dZnu++duXpC1uQo1cG=WA@mail.gmail.com
* Ensure write failure reports no-disk-spaceAlvaro Herrera2020-06-19
| | | | | | | | | | | | | | | A few places calling fwrite and gzwrite were not setting errno to ENOSPC when reporting errors, as is customary; this led to some failures being reported as "could not write file: Success" which makes us look silly. Make a few of these places in pg_dump and pg_basebackup use our customary pattern. Backpatch-to: 9.5 Author: Justin Pryzby <pryzby@telsasoft.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20200611153753.GU14879@telsasoft.com
* 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.
* Clean up includes of s_lock.h.Andres Freund2020-06-18
| | | | | | | Users of spinlocks should use spin.h, not s_lock.h. And lwlock.h hasn't utilized spinlocks for quite a while. Discussion: https://postgr.es/m/20200618183041.upyrd25eosecyf3x@alap3.anarazel.de
* 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
* Disallow factorial of negative numbersPeter Eisentraut2020-06-18
| | | | | | | The previous implementation returned 1 for all negative numbers, which is not sensible under any definition. Discussion: https://www.postgresql.org/message-id/flat/6ce1df0e-86a3-e544-743a-f357ff663f68%402ndquadrant.com
* Expand tests for factorialPeter Eisentraut2020-06-18
| | | | | | | | | Move from int4 to numeric test. (They were originally int4 functions, but were reimplemented for numeric in 04a4821adef38155b7920ba9eb83c4c3c29156f8.) Add some tests for edge cases. Discussion: https://www.postgresql.org/message-id/flat/6ce1df0e-86a3-e544-743a-f357ff663f68%402ndquadrant.com
* Remove reset of testtablespace from pg_regress on WindowsMichael Paquier2020-06-18
| | | | | | | | | | | | | | | | testtablespace is an extra path used as tablespace location in the main regression test suite, computed from --outputdir as defined by the caller of pg_regress (current directory if undefined). This special handling was introduced as of f10589e to be specific to MSVC, as we let pg_regress' Makefile handle this cleanup in other environments. This moves the cleanup to the MSVC script running regression tests instead where needed: check, installcheck and upgradecheck. I have also checked this patch on MSVC with repeated runs of each target. Author: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20200219.142519.437573253063431435.horikyota.ntt@gmail.com
* 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.
* Improve server code to read files as part of a base backup.Robert Haas2020-06-17
| | | | | | | | | | | | | | Don't use fread(), since that doesn't necessarily set errno. We could use read() instead, but it's even better to use pg_pread(), which allows us to avoid some extra calls to seek to the desired location in the file. Also, advertise a wait event while reading from a file, as we do for most other places where we're reading data from files. Patch by me, reviewed by Hamid Akhtar. Discussion: http://postgr.es/m/CA+TgmobBw-3573vMosGj06r72ajHsYeKtksT_oTxH8XvTL7DxA@mail.gmail.com
* Minor code cleanup for perform_base_backup().Robert Haas2020-06-17
| | | | | | | | | | | | Merge two calls to sendDir() that are exactly the same except for the fifth argument. Adjust comments to match. Also, don't bother checking whether tblspc_map_file is NULL. We initialize it in all cases, so it can't be. Patch by me, reviewed by Amit Kapila and Kyotaro Horiguchi. Discussion: http://postgr.es/m/CA+TgmoYq+59SJ2zBbP891ngWPA9fymOqntqYcweSDYXS2a620A@mail.gmail.com
* Don't export basebackup.c's sendTablespace().Robert Haas2020-06-17
| | | | | | | | | | | | | Commit 72d422a5227ef6f76f412486a395aba9f53bf3f0 made xlog.c call sendTablespace() with the 'sizeonly' argument set to true, which required basebackup.c to export sendTablespace(). However, that's kind of ugly, so instead defer the call to sendTablespace() until basebackup.c regains control. That way, it can still be a static function. Patch by me, reviewed by Amit Kapila and Kyotaro Horiguchi. Discussion: http://postgr.es/m/CA+TgmoYq+59SJ2zBbP891ngWPA9fymOqntqYcweSDYXS2a620A@mail.gmail.com
* Remove STATUS_WAITINGPeter Eisentraut2020-06-17
| | | | | | | Add a separate enum for use in the locking APIs, which were the only user. Discussion: https://www.postgresql.org/message-id/flat/a6f91ead-0ce4-2a34-062b-7ab9813ea308%402ndquadrant.com
* Lobotomize test for float -Inf ^ -2, at least for now.Tom Lane2020-06-16
| | | | | | | | | | | | | | | | Per POSIX this case should produce +0, but buildfarm member fossa (with icc (ICC) 19.0.5.281 20190815) is reporting -0. icc has a boatload of unsafe floating-point optimizations, with a corresponding boatload of not-too-well-documented compiler switches, and it seems our default use of "-mp1" isn't whacking it hard enough to keep it from misoptimizing the stanza in dpow() that checks whether y is odd. There's nothing wrong with that code (seeing that no other buildfarm member has trouble with it), so I'm content to blame this on the compiler. But without access to the compiler I'm not going to guess at what switches might be needed to fix it. For now, tweak the test case so it will accept either -0 or +0 as a correct answer. Discussion: https://postgr.es/m/E1jkyFX-0005RR-1Q@gemulon.postgresql.org
* Fix file reference in nls.mkPeter Eisentraut2020-06-16
| | | | Broken by move of fe_archive.c to fe_utils.
* In dpow(), remove redundant check for whether y is an integer.Tom Lane2020-06-16
| | | | | | | | | | | | | I failed to notice that we don't really need to check for y being an integer in the code path where x = -inf; we already did. Also make some further cosmetic rearrangements in that spot in hopes of dodging the seeming compiler bug that buildfarm member fossa is hitting. And be consistent about declaring variables as "float8" not "double", since the pre-existing variables in this function are like that. Discussion: https://postgr.es/m/E1jkyFX-0005RR-1Q@gemulon.postgresql.org
* Remove useless variable.Thomas Munro2020-06-16
|
* Make BufFileWrite() void.Thomas Munro2020-06-16
| | | | | | | | It now either returns after it wrote all the data you gave it, or raises an error. Not done in back-branches, because it might cause problems for external code. Discussion: https://postgr.es/m/CA%2BhUKGJE04G%3D8TLK0DLypT_27D9dR8F1RQgNp0jK6qR0tZGWOw%40mail.gmail.com
* 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 power() for large inputs yet more.Tom Lane2020-06-15
| | | | | | | | | | | | | | | | | Buildfarm results for commit e532b1d57 reveal the error in my thinking about the unexpected-EDOM case. I'd supposed this was no longer really a live issue, but it seems the fix for glibc's bug #3866 is not all that old, and we still have at least one buildfarm animal (lapwing) with the bug. Hence, resurrect essentially the previous logic (but, I hope, less opaquely presented), and explain what it is we're really doing here. Also, blindly try to fix fossa's failure by tweaking the logic that figures out whether y is an odd integer when x is -inf. This smells a whole lot like a compiler bug, but I lack access to icc to try to pin it down. Maybe doing division instead of multiplication will dodge the issue. Discussion: https://postgr.es/m/E1jkU7H-00024V-NZ@gemulon.postgresql.org
* Assorted cleanup of tar-related code.Robert Haas2020-06-15
| | | | | | | | | | | | | Introduce TAR_BLOCK_SIZE and replace many instances of 512 with the new constant. Introduce function tarPaddingBytesRequired and use it to replace numerous repetitions of (x + 511) & ~511. Add preprocessor guards against multiple inclusion to pgtar.h. Reformat the prototype for tarCreateHeader so it doesn't extend beyond 80 characters. Discussion: http://postgr.es/m/CA+TgmobWbfReO9-XFk8urR1K4wTNwqoHx_v56t7=T8KaiEoKNw@mail.gmail.com
* Fix power() for infinity inputs some more.Tom Lane2020-06-15
| | | | | | | | | | | | | | | | | | Buildfarm results for commit decbe2bfb show that AIX and illumos have non-POSIX-compliant pow() functions, as do ancient NetBSD and HPUX releases. While it's dubious how much we should care about the latter two platforms, the former two are probably enough reason to put in manual handling of infinite-input cases. Hence, do so, and clean up the post-pow() error handling to reflect its now-more-limited scope. (Notably, while we no longer expect to ever see EDOM from pow(), report it as a domain error if we do. The former coding had the net effect of expensively converting the error to ERANGE, which seems highly questionable: if pow() wanted to report ERANGE, it would have done so.) Patch by me; thanks to Michael Paquier for review. Discussion: https://postgr.es/m/E1jkU7H-00024V-NZ@gemulon.postgresql.org
* Fix some comments referring to past featuresMichael Paquier2020-06-15
| | | | | | | | Timestamp can only be an int64 since b9d092c, and support for WITH OIDS has been removed as of 578b229. Author: Justin Pryzby Discussion: https://postgr.es/m/20200612023709.GC14879@telsasoft.com
* pg_dump: Unbreak dumping of aggregates from very old server versionsPeter Eisentraut2020-06-15
| | | | Recently broken by d9fa17aa7c34dea66ce64da6fb4c643e75ba452c.
* 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
* 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