aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix a recently-introduced race condition in LISTEN/NOTIFY handling.Tom Lane2020-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit 566372b3d fixed some race conditions involving concurrent SimpleLruTruncate calls, but it introduced new ones in async.c. A newly-listening backend could attempt to read Notify SLRU pages that were in process of being truncated, possibly causing an error. Also, the QUEUE_TAIL pointer could become set to a value that's not equal to the queue position of any backend. While that's fairly harmless in v13 and up (thanks to commit 51004c717), in older branches it resulted in near-permanent disabling of the queue truncation logic, so that continued use of NOTIFY led to queue-fill warnings and eventual inability to send any more notifies. (A server restart is enough to make that go away, but it's still pretty unpleasant.) The core of the problem is confusion about whether QUEUE_TAIL represents the "logical" tail of the queue (i.e., the oldest still-interesting data) or the "physical" tail (the oldest data we've not yet truncated away). To fix, split that into two variables. QUEUE_TAIL regains its definition as the logical tail, and we introduce a new variable to track the oldest un-truncated page. Per report from Mikael Gustavsson. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/1b8561412e8a4f038d7a491c8b922788@smhi.se
* Fix CLUSTER progress reporting of number of blocks scanned.Fujii Masao2020-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | Previously pg_stat_progress_cluster view reported the current block number in heap scan as the number of heap blocks scanned (i.e., heap_blks_scanned). This reported number could be incorrect when synchronize_seqscans is enabled, because it allowed the heap scan to start at block in middle. This could result in wraparounds in the heap_blks_scanned column when the heap scan wrapped around. This commit fixes the bug by calculating the number of blocks from the block that the heap scan starts at to the current block in scan, and reporting that number in the heap_blks_scanned column. Also, in pg_stat_progress_cluster view, previously heap_blks_scanned could not reach heap_blks_total at the end of heap scan phase if the last pages scanned were empty. This commit fixes the bug by manually updating heap_blks_scanned to the same value as heap_blks_total when the heap scan phase finishes. Back-patch to v12 where pg_stat_progress_cluster view was introduced. Reported-by: Matthias van de Meent Author: Matthias van de Meent Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAEze2WjCBWSGkVfYag001Rc4+-nNLDpWM7QbyD6yPvuhKs-gYQ@mail.gmail.com
* In psql's \d commands, don't truncate attribute default values.Tom Lane2020-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | Historically, psql has truncated the text of a column's default expression at 128 characters. This is unlike any other behavior in describe.c, and it's become particularly confusing now that the limit is only applied to the expression proper and not to the "generated always as (...) stored" text that may get wrapped around it. Excavation in our git history suggests that the original motivation for this limit was not really to limit the display width (as I'd long supposed), but to make it safe to use a fixed-width output buffer to store the result. That implementation restriction is long gone of course, but the limit remained. Let's just get rid of it. While here, rearrange the logic about when to free the output string so that it's not so dependent on unstated assumptions about the possible values of attidentity and attgenerated. Per bug #16743 from David Turon. Back-patch to v12 where GENERATED came in. (Arguably we could take it back further, but I'm hesitant to change the behavior of long-stable branches for this.) Discussion: https://postgr.es/m/16743-7b1bacc4af76e7ad@postgresql.org
* Remove obsolete comment atop ri_PlanCheck.Amit Kapila2020-11-25
| | | | | | | | | Commit 5b7ba75f7f removed the unused parameter but forgot to update the nearby comments. Author: Li Japin Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/0E2F62A2-B2F1-4052-83AE-F0BEC8A75789@hotmail.com
* Properly check index mark/restore in ExecSupportsMarkRestore.Andrew Gierth2020-11-24
| | | | | | | | | | | | | | | | Previously this code assumed that all IndexScan nodes supported mark/restore, which is not true since it depends on optional index AM support functions. This could lead to errors about missing support functions in rare edge cases of mergejoins with no sort keys, where an unordered non-btree index scan was placed on the inner path without a protecting Materialize node. (Normally, the fact that merge join requires ordered input would avoid this error.) Backpatch all the way since this bug is ancient. Per report from Eugen Konkov on irc. Discussion: https://postgr.es/m/87o8jn50be.fsf@news-spur.riddles.org.uk
* Skip allocating hash table in EXPLAIN-only mode.Heikki Linnakangas2020-11-20
| | | | | | | | This is a backpatch of commit 2cccb627f1, backpatched due to popular demand. Backpatch to all supported versions. Author: Alexey Bashtanov Discussion: https://www.postgresql.org/message-id/36823f65-050d-ae24-aa4d-a37726998240%40imap.cc
* On macOS, use -isysroot in link steps as well as compile steps.Tom Lane2020-11-20
| | | | | | | | | | | | | | | | | | | | | | | | We previously put the -isysroot switch only into CPPFLAGS, theorizing that it was only needed to find the right copies of include files. However, it seems that we also need to use it while linking programs, to find the right stub ".tbd" files for libraries. We got away without that up to now, but apparently that was mostly luck. It may also be that failures are only observed when the Xcode version is noticeably out of sync with the host macOS version; the case that's prompting action right now is that builds fail when using latest Xcode (12.2) on macOS Catalina, even though it's fine on Big Sur. Hence, add -isysroot to LDFLAGS as well. (It seems that the more common practice is to put it in CFLAGS, whence it'd be included at both compile and link steps. However, we can't mess with CFLAGS in the platform template file without confusing configure's logic for choosing default CFLAGS.) Back-patch of 49407dc32 into all supported branches. Report and patch by James Hilliard (some cosmetic mods by me) Discussion: https://postgr.es/m/20201120003314.20560-1-james.hilliard1@gmail.com
* Further fixes for CREATE TABLE LIKE: cope with self-referential FKs.Tom Lane2020-11-19
| | | | | | | | | | | | | | | | | | | | | | | Commit 502898192 was too careless about the order of execution of the additional ALTER TABLE operations generated by expandTableLikeClause. It just stuck them all at the end, which seems okay for most purposes. But it falls down in the case where LIKE is importing a primary key or unique index and the outer CREATE TABLE includes a FOREIGN KEY constraint that needs to depend on that index. Weird as that is, it used to work, so we ought to keep it working. To fix, make parse_utilcmd.c insert LIKE clauses between index-creation and FK-creation commands in the transformed list of commands, and change utility.c so that the commands generated by expandTableLikeClause are executed immediately not at the end. One could imagine scenarios where this wouldn't work either; but currently expandTableLikeClause only makes column default expressions, CHECK constraints, and indexes, and this ordering seems fine for those. Per bug #16730 from Sofoklis Papasofokli. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16730-b902f7e6e0276b30@postgresql.org
* Don't Insert() a VFD entry until it's fully built.Tom Lane2020-11-16
| | | | | | | | | | | | | | | | | | | Otherwise, if FDDEBUG is enabled, the debugging output fails because it tries to read the fileName, which isn't set up yet (and should in fact always be NULL). AFAICT, this has been wrong since Berkeley. Before 96bf88d52, it would accidentally fail to crash on platforms where snprintf() is forgiving about being passed a NULL pointer for %s; but the file name intended to be included in the debug output wouldn't ever have shown up. Report and fix by Greg Nancarrow. Although this is only visibly broken in custom-made builds, it still seems worth back-patching to all supported branches, as the FDDEBUG code is pretty useless as it stands. Discussion: https://postgr.es/m/CAJcOf-cUDgm9qYtC_B6XrC6MktMPNRby2p61EtSGZKnfotMArw@mail.gmail.com
* Do not return NULL for error cases in satisfies_hash_partition().Tom Lane2020-11-16
| | | | | | | | | | | | | | | | | | | | | | | Since this function is used as a CHECK constraint condition, returning NULL is tantamount to returning TRUE, which would have the effect of letting in a row that doesn't satisfy the hash condition. Admittedly, the cases for which this is done should be unreachable in practice, but that doesn't make it any less a bad idea. It also seems like a dartboard was used to decide which error cases should throw errors as opposed to returning NULL. For the checks for NULL input values, I just switched it to returning false. There's some argument that an error would be better; but the case really should be can't-happen in a generated hash constraint, so it's likely not worth more code for. For the parent-relation-open-failure case, it seems like we might as well let relation_open throw an error, instead of having an impossible-to-diagnose constraint failure. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/24067.1605134819@sss.pgh.pa.us
* Use "true" not "TRUE" in one ICU function call.Tom Lane2020-11-16
| | | | | | | | | | | | | This was evidently missed in commit 6337865f3, which generally did s/TRUE/true/ everywhere. It escaped notice up to now because ICU versions before ICU 68 provided definitions of "TRUE" and "FALSE" regardless. With ICU 68, it fails to compile. Per report from Condor. Back-patch to v11 where 6337865f3 came in. (I've not tested v10, where this call originated, but I imagine it's fine since we defined TRUE in c.h back then.) Discussion: https://postgr.es/m/7a6f3336165bfe3ca66abcda7966f9d0@stz-bg.com
* Fix fuzzy thinking about amcanmulticol versus amcaninclude.Tom Lane2020-11-15
| | | | | | | | | | | | | | | | | | These flags should be independent: in particular an index AM should be able to say that it supports include columns without necessarily supporting multiple key columns. The included-columns patch got this wrong, possibly aided by the fact that it didn't bother to update the documentation. While here, clarify some text about amcanreturn, which was a little vague about what should happen when amcanreturn reports that only some of the index columns are returnable. Noted while reviewing the SP-GiST included-columns patch, which quite incorrectly (and unsafely) changed SP-GiST to claim amcanmulticol = true as a workaround for this bug. Backpatch to v11 where included columns were introduced.
* doc: wire protocol data type for history file content is byteaBruce Momjian2020-11-12
| | | | | | | | | | | Document that though the history file content is marked as bytea, it is the same a text, and neither is btyea-escaped or encoding converted. Reported-by: Brar Piening Discussion: https://postgr.es/m/6a1b9cd9-17e3-df67-be55-86102af6bdf5@gmx.de Backpatch-through: 13 - 9.5 (not master)
* Remove useless SHA256 initialization when not using backup manifestsMichael Paquier2020-11-12
| | | | | | | | | | | | | | | | | | | | Attempting to take a base backup with Postgres linking to a build of OpenSSL with FIPS enabled currently fails with or even without a backup manifest requested because of this mandatory SHA256 initialization used for the manifest file itself. However, there is no need to do this initialization at all if backup manifests are not needed because there is no data to append to the manifest. Note that being able to use backup manifests with OpenSSL+FIPS requires a switch of the SHA2 implementation to use EVP, which would cause an ABI breakage so this cannot be backpatched to 13 as it has been already released, but at least avoiding this SHA256 initialization gives users the possibility to take a base backup even when specifying --no-manifest with pg_basebackup. Author: Michael Paquier Discussion: https://postgr.es/m/20201110020014.GE1887@paquier.xyz Backpatch-through: 13
* Remove duplicate code in brin_memtuple_initializeTomas Vondra2020-11-11
| | | | | | | | | | Commit 8bf74967dab moved some of the code from brin_new_memtuple to brin_memtuple_initialize, but this resulted in some of the code being duplicate. Fix by removing the duplicate lines and backpatch to 10. Author: Tomas Vondra Backpatch-through: 10 Discussion: https://postgr.es/m/5eb50c97-9a8e-b691-8c40-1b2a55611c4c%40enterprisedb.com
* Fix and simplify some usages of TimestampDifference().Tom Lane2020-11-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce TimestampDifferenceMilliseconds() to simplify callers that would rather have the difference in milliseconds, instead of the select()-oriented seconds-and-microseconds format. This gets rid of at least one integer division per call, and it eliminates some apparently-easy-to-mess-up arithmetic. Two of these call sites were in fact wrong: * pg_prewarm's autoprewarm_main() forgot to multiply the seconds by 1000, thus ending up with a delay 1000X shorter than intended. That doesn't quite make it a busy-wait, but close. * postgres_fdw's pgfdw_get_cleanup_result() thought it needed to compute microseconds not milliseconds, thus ending up with a delay 1000X longer than intended. Somebody along the way had noticed this problem but misdiagnosed the cause, and imposed an ad-hoc 60-second limit rather than fixing the units. This was relatively harmless in context, because we don't care that much about exactly how long this delay is; still, it's wrong. There are a few more callers of TimestampDifference() that don't have a direct need for seconds-and-microseconds, but can't use TimestampDifferenceMilliseconds() either because they do need microsecond precision or because they might possibly deal with intervals long enough to overflow 32-bit milliseconds. It might be worth inventing another API to improve that, but that seems outside the scope of this patch; so those callers are untouched here. Given the fact that we are fixing some bugs, and the likelihood that future patches might want to back-patch code that uses this new API, back-patch to all supported branches. Alexey Kondratov and Tom Lane Discussion: https://postgr.es/m/3b1c053a21c07c1ed5e00be3b2b855ef@postgrespro.ru
* Work around cross-version-upgrade issues created by commit 9e38c2bb5.Tom Lane2020-11-10
| | | | | | | | | | | | | Summarily changing the STYPE of regression-test aggregates that depend on array_append or array_cat is an issue for the buildfarm's cross-version-upgrade tests, because those aggregates (as defined in the back branches) now won't load into HEAD. Although this seems like only a minimal risk for genuine user-defined aggregates, we need to do something for the buildfarm. Hence, adjust the aggregate definitions, in both HEAD and the back branches. Discussion: https://postgr.es/m/1401824.1604537031@sss.pgh.pa.us Discussion: https://postgr.es/m/E1kaQ2c-0005lx-Eg@gemulon.postgresql.org
* Ignore attempts to \gset into specially treated variables.Noah Misch2020-11-09
| | | | | | | | | | | | | | | If an interactive psql session used \gset when querying a compromised server, the attacker could execute arbitrary code as the operating system account running psql. Using a prefix not found among specially treated variables, e.g. every lowercase string, precluded the attack. Fix by issuing a warning and setting no variable for the column in question. Users wanting the old behavior can use a prefix and then a meta-command like "\set HISTSIZE :prefix_HISTSIZE". Back-patch to 9.5 (all supported versions). Reviewed by Robert Haas. Reported by Nick Cleaton. Security: CVE-2020-25696
* In security-restricted operations, block enqueue of at-commit user code.Noah Misch2020-11-09
| | | | | | | | | | | | | | | | | | Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred triggers within index expressions and materialized view queries. An attacker having permission to create non-temp objects in at least one schema could execute arbitrary SQL functions under the identity of the bootstrap superuser. One can work around the vulnerability by disabling autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW. (Don't restore from pg_dump, since it runs some of those commands.) Plain VACUUM (without FULL) is safe, and all commands are fine when a trusted user owns the target object. Performance may degrade quickly under this workaround, however. Back-patch to 9.5 (all supported versions). Reviewed by Robert Haas. Reported by Etienne Stalmans. Security: CVE-2020-25695
* Translation updatesPeter Eisentraut2020-11-09
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 2ffedf5ea37677f39cdc1eb92a1e78762cd3fb0e
* Remove incorrect %s in stringMagnus Hagander2020-11-09
| | | | | | | | | Appears to have been a copy/paste error in the original commit that moved the messages to fe_utils/. Author: Tang, Haiying <tanghy.fnst@cn.fujitsu.com> Backpatch-through: 13 Discussion: https://postgr.es/m/3321cbcea76d4d2c8320a05c19b9304a@G08CNEXMBPEKD05.g08.fujitsu.local
* In INSERT/UPDATE, use the table's real tuple descriptor as target.Tom Lane2020-11-08
| | | | | | | | | | | | | | | | | | | | | | | This back-patches commit 20d3fe900 into the v12 and v13 branches. At the time I thought that commit was not fixing any observable bug, but Bertrand Drouvot showed otherwise: adding a dropped column to the previously-considered scenario crashes v12 and v13, unless the dropped column happens to be an integer. That is, of course, because the tupdesc we derive from the plan output tlist fails to describe the dropped column accurately, so that we'll do the wrong thing with a tuple in which that column isn't NULL. There is no bug in pre-v12 branches because they already did use the table's real tuple descriptor for any trigger-returned tuple. It seems that this set of bugs can be blamed on the changes that removed es_trig_tuple_slot, though I've not attempted to pin that down precisely. Although there's no code change needed in HEAD, update the test case to include a dropped column there too. Discussion: https://postgr.es/m/db5d97c8-f48a-51e2-7b08-b73d5434d425@amazon.com Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org
* Fix test for error message changePeter Eisentraut2020-11-08
| | | | fix for f3ad4fddfaf71e8f6f037cd627f398ba43625ca1
* Message style improvementsAlvaro Herrera2020-11-07
| | | | | | | | | | | | | * Avoid pointlessly highlighting that an index vacuum was executed by a parallel worker; user doesn't care. * Don't give the impression that a non-concurrent reindex of an invalid index on a TOAST table would work, because it wouldn't. * Add a "translator:" comment for a mysterious message. Discussion: https://postgr.es/m/20201107034943.GA16596@alvherre.pgsql Reviewed-by: Michael Paquier <michael@paquier.xyz>
* Fix redundant error messages in client toolsPeter Eisentraut2020-11-07
| | | | | | A few client tools duplicate error messages already provided by libpq. Discussion: https://www.postgresql.org/message-id/flat/3e937641-88a1-e697-612e-99bba4b8e5e4%40enterprisedb.com
* Avoid re-using output variables in new ecpg test case.Tom Lane2020-11-07
| | | | | | | | | The buildfarm thinks this leads to memory stomps, though annoyingly I can't duplicate that here. The existing code in strings.pgc is doing something that doesn't seem to be sanctioned at all really by the documentation, but I'm disinclined to try to make that nicer right now. Let's just declare some more output variables in hopes of working around it.
* Fix ecpg's mishandling of B'...' and X'...' literals.Tom Lane2020-11-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These were broken in multiple ways: * The xbstart and xhstart lexer actions neglected to set "state_before_str_start" before transitioning to the xb/xh states, thus possibly resulting in "internal error: unreachable state" later. * The test for valid string contents at the end of xb state was flat out wrong, as it accounted incorrectly for the "b" prefix that the xbstart action had injected. Meanwhile, the xh state had no such check at all. * The generated literal value failed to include any quote marks. * The grammar did the wrong thing anyway, typically ignoring the literal value and emitting something else, since BCONST and XCONST tokens were handled randomly differently from SCONST tokens. The first of these problems is evidently an oversight in commit 7f380c59f, but the others seem to be very ancient. The lack of complaints shows that ECPG users aren't using these syntaxes much (although I do vaguely remember one previous complaint). As written, this patch is dependent on 7f380c59f, so it can't go back further than v13. Given the shortage of complaints, I'm not excited about adapting the patch to prior branches. Report and patch by Shenhao Wang (test case adjusted by me) Discussion: https://postgr.es/m/d6402f1bacb74ecba22ef715dbba17fd@G08CNEXMBPEKD06.g08.fujitsu.local
* Plug memory leak in index_get_partitionAlvaro Herrera2020-11-06
| | | | | | | | | | | | | | | The list of indexes was being leaked when asked for an index that doesn't have an index partition in the table partition. Not a common case admittedly --and in most cases where it occurs, caller throws an error anyway-- but worth fixing for cleanliness and in case any third-party code is calling this function. While at it, remove use of lfirst_oid() to obtain a value we already have. Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20201105203606.GF22691@telsasoft.com
* Properly detoast data in brin_form_tupleTomas Vondra2020-11-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | brin_form_tuple failed to consider the values may be toasted, inserting the toast pointer into the index. This may easily result in index corruption, as the toast data may be deleted and cleaned up by vacuum. The cleanup however does not care about indexes, leaving invalid toast pointers behind, which triggers errors like this: ERROR: missing chunk number 0 for toast value 16433 in pg_toast_16426 A less severe consequence are inconsistent failures due to the index row being too large, depending on whether brin_form_tuple operated on plain or toasted version of the row. For example CREATE TABLE t (val TEXT); INSERT INTO t VALUES ('... long value ...') CREATE INDEX idx ON t USING brin (val); would likely succeed, as the row would likely include toast pointer. Switching the order of INSERT and CREATE INDEX would likely fail: ERROR: index row size 8712 exceeds maximum 8152 for index "idx" because this happens before the row values are toasted. The bug exists since PostgreSQL 9.5 where BRIN indexes were introduced. So backpatch all the way back. Author: Tomas Vondra Reviewed-by: Alvaro Herrera Backpatch-through: 9.5 Discussion: https://postgr.es/m/20201001184133.oq5uq75sb45pu3aw@development Discussion: https://postgr.es/m/20201104010544.zexj52mlldagzowv%40development
* Revert "Accept relations of any kind in LOCK TABLE".Tom Lane2020-11-06
| | | | | | | | | | Revert 59ab4ac32, as well as the followup fix 33862cb9c, in all branches. We need to think a bit harder about what the behavior of LOCK TABLE on views should be, and there's no time for that before next week's releases. We'll take another crack at this later. Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
* Revert "pg_dump: Lock all relations, not just plain tables".Tom Lane2020-11-06
| | | | | | | | | | Revert 403a3d91c, as well as the followup fix 7f4235032, in all branches. We need to think a bit harder about what the behavior of LOCK TABLE on views should be, and there's no time for that before next week's releases. We'll take another crack at this later. Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
* Don't throw an error for LOCK TABLE on a self-referential view.Tom Lane2020-11-05
| | | | | | | | | | | | | LOCK TABLE has complained about "infinite recursion" when applied to a self-referential view, ever since we made it recurse into views in v11. However, that breaks pg_dump's new assumption that it's okay to lock every relation. There doesn't seem to be any good reason to throw an error: if we just abandon the recursion, we've still satisfied the requirement of locking every referenced relation. Per bug #16703 from Andrew Bille (via Alexander Lakhin). Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
* Fix nbtree cleanup-only VACUUM stats inaccuracies.Peter Geoghegan2020-11-04
| | | | | | | | | | | | | | | | | | Logic for counting heap TIDs from posting list tuples (added by commit 0d861bbb) was faulty. It didn't count any TIDs/index tuples in the event of no callback being set. This meant that we incorrectly counted no index tuples in clean-up only VACUUMs, which could lead to pg_class.reltuples being spuriously set to 0 in affected indexes. To fix, go back to counting items from the page in cases where there is no callback. This approach isn't very accurate, but it works well enough in practice while avoiding the expense of accessing every index tuple during cleanup-only VACUUMs. Author: Peter Geoghegan <pg@bowt.ie> Reported-By: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> https://postgr.es/m/20201023174451.69e358f1@firost Backpatch: 13-, where nbtree deduplication was introduced
* Enable hash partitioning of text arraysPeter Eisentraut2020-11-04
| | | | | | | | | | | | | | | | hash_array_extended() needs to pass PG_GET_COLLATION() to the hash function of the element type. Otherwise, the hash function of a collation-aware data type such as text will error out, since the introduction of nondeterministic collation made hash functions require a collation, too. The consequence of this is that before this change, hash partitioning using an array over text in the partition key would not work. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/32c1fdae-95c6-5dc6-058a-a90330a3b621%40enterprisedb.com
* Use INT64_FORMAT to print int64 variables in sort debugTomas Vondra2020-11-03
| | | | | | | | | | Commit 6ee3b5fb99 cleaned up most of the long/int64 confusion related to incremental sort, but the sort debug messages were still using %ld for int64 variables. So fix that. Author: Haiying Tang Backpatch-through: 13, where the incremental sort code was added Discussion: https://postgr.es/m/4250be9d350c4992abb722a76e288aef%40G08CNEXMBPEKD05.g08.fujitsu.local
* Fix get_useful_pathkeys_for_relation for volatile expressionsTomas Vondra2020-11-03
| | | | | | | | | | | | | | When considering Incremental Sort below a Gather Merge, we need to be a bit more careful when matching pathkeys to EC members. It's not enough to find a member whose Vars are all in the current relation's target; volatile expressions in particular need to be contained in the target, otherwise it's too early to use the pathkey. Reported-by: Jaime Casanova Author: James Coleman Reviewed-by: Tomas Vondra Backpatch-through: 13, where the incremental sort code was added Discussion: https://postgr.es/m/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com
* Guard against core dump from uninitialized subplan.Tom Lane2020-11-03
| | | | | | | | | | | | | | If the planner erroneously puts a non-parallel-safe SubPlan into a parallelized portion of the query tree, nodeSubplan.c will fail in the worker processes because it finds a null in es_subplanstates, which it's unable to cope with. It seems worth a test-and-elog to make that an error case rather than a core dump case. This probably should have been included in commit 16ebab688, which was responsible for allowing nulls to appear in es_subplanstates to begin with. So, back-patch to v10 where that came in. Discussion: https://postgr.es/m/924226.1604422326@sss.pgh.pa.us
* Allow users with BYPASSRLS to alter their own passwords.Tom Lane2020-11-03
| | | | | | | | | | | | | | | | The intention in commit 491c029db was to require superuserness to change the BYPASSRLS property, but the actual effect of the coding in AlterRole() was to require superuserness to change anything at all about a BYPASSRLS role. Other properties of a BYPASSRLS role should be changeable under the same rules as for a normal role, though. Fix that, and also take care of some documentation omissions related to BYPASSRLS and REPLICATION role properties. Tom Lane and Stephen Frost, per bug report from Wolfgang Walther. Back-patch to all supported branches. Discussion: https://postgr.es/m/a5548a9f-89ee-3167-129d-162b5985fcf8@technowledgy.de
* Disallow ALTER TABLE ONLY / DROP EXPRESSIONPeter Eisentraut2020-11-03
| | | | | | | | | | | | | | | The current implementation cannot handle this correctly, so just forbid it for now. GENERATED clauses must be attached to the column definition and cannot be added later like DEFAULT, so if a child table has a generation expression that the parent does not have, the child column will necessarily be an attlocal column. So to implement ALTER TABLE ONLY / DROP EXPRESSION, we'd need extra code to update attislocal of the direct child tables, somewhat similar to how DROP COLUMN does it, so that the resulting state can be properly dumped and restored. Discussion: https://www.postgresql.org/message-id/flat/15830.1575468847%40sss.pgh.pa.us
* Fix unportable use of getnameinfo() in pg_hba_file_rules view.Tom Lane2020-11-02
| | | | | | | | | | | | | | | | | | | | | | | fill_hba_line() thought it could get away with passing sizeof(struct sockaddr_storage) rather than the actual addrlen previously returned by getaddrinfo(). While that appears to work on many platforms, it does not work on FreeBSD 11: you get back a failure, which leads to the view showing NULL for the address and netmask columns in all rows. The POSIX spec for getnameinfo() is pretty clearly on FreeBSD's side here: you should pass the actual address length. So it seems plausible that there are other platforms where this coding also fails, and we just hadn't noticed. Also, IMO the fact that getnameinfo() failure leads to a NULL output is pretty bogus in itself. Our pg_getnameinfo_all() wrapper is careful to emit "???" on failure, and we should use that in such cases. NULL should only be emitted in rows that don't have IP addresses. Per bug #16695 from Peter Vandivier. Back-patch to v10 where this code was added. Discussion: https://postgr.es/m/16695-a665558e2f630be7@postgresql.org
* Second thoughts on TOAST decompression.Tom Lane2020-11-02
| | | | | | | | | On detecting a corrupted match tag, pglz_decompress() should just summarily return -1. Breaking out of the loop, as I did in dfc797730, doesn't quite guarantee that will happen. Also, we can use unlikely() on that check, just in case it helps. Backpatch to v13, like the previous patch.
* Extend PageIsVerified() to handle more custom optionsMichael Paquier2020-11-02
| | | | | | | | | | | | | | | | | | | | | | This is useful for checks of relation pages without having to load the pages into the shared buffers, and two cases can make use of that: page verification in base backups and the online, lock-safe, flavor. Compatibility is kept with past versions using a routine that calls the new extended routine with the set of options compatible with the original version. Contrary to d401c576, a macro cannot be used as there may be external code relying on the presence of the original routine. This is applied down to 11, where this will be used by a follow-up commit addressing a set of issues with page verification in base backups. Extracted from a larger patch by the same author. Author: Anastasia Lubennikova Reviewed-by: Michael Paquier, Julien Rouhaud Discussion: https://postgr.es/m/608f3476-0598-2514-2c03-e05c7d2b0cbd@postgrespro.ru Backpatch-through: 11
* Fix two issues in TOAST decompression.Tom Lane2020-11-01
| | | | | | | | | | | | | | | | | | | | | | | | | | pglz_maximum_compressed_size() potentially underestimated the amount of compressed data required to produce N bytes of decompressed data; this is a fault in commit 11a078cf8. Separately from that, pglz_decompress() failed to protect itself against corrupt compressed data, particularly off == 0 in a match tag. Commit c60e520f6 turned such a situation into an infinite loop, where before it'd just have resulted in garbage output. The combination of these two bugs seems like it may explain bug #16694 from Tom Vijlbrief, though it's impossible to be quite sure without direct inspection of the failing session. (One needs to assume that the pglz_maximum_compressed_size() bug caused us to fail to fetch the second byte of a match tag, and what happened to be there instead was a zero. The reported infinite loop is hard to explain without off == 0, though.) Aside from fixing the bugs, rewrite associated comments for more clarity. Back-patch to v13 where both these commits landed. Discussion: https://postgr.es/m/16694-f107871e499ec114@postgresql.org
* Avoid null pointer dereference if error result lacks SQLSTATE.Tom Lane2020-11-01
| | | | | | | | | Although error results received from the backend should always have a SQLSTATE field, ones generated by libpq won't, making this code vulnerable to a crash after, say, untimely loss of connection. Noted by Coverity. Oversight in commit 403a3d91c. Back-patch to 9.5, as that was.
* Preserve index data in pg_statistic across REINDEX CONCURRENTLYMichael Paquier2020-11-01
| | | | | | | | | | | | | | | | | | Statistics associated to an index got lost after running REINDEX CONCURRENTLY, while the non-concurrent case preserves these correctly. The concurrent and non-concurrent operations need to be consistent for the end-user, and missing statistics would force to wait for a new analyze to happen, which could take some time depending on the activity of the existing autovacuum workers. This issue is fixed by copying any existing entries in pg_statistic associated to the old index to the new one. Note that this copy is already done with the data of the index in the stats collector. Reported-by: Fabrízio de Royes Mello Author: Michael Paquier, Fabrízio de Royes Mello Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/CAFcNs+qpFPmiHd1oTXvcPdvAHicJDA9qBUSujgAhUMJyUMb+SA@mail.gmail.com Backpatch-through: 12
* Reproduce debug_query_string==NULL on parallel workers.Noah Misch2020-10-31
| | | | | | | | | | Certain background workers initiate parallel queries while debug_query_string==NULL, at which point they attempted strlen(NULL) and died to SIGSEGV. Older debug_query_string observers allow NULL, so do likewise in these newer ones. Back-patch to v11, where commit 7de4a1bcc56f494acbd0d6e70781df877dc8ecb5 introduced the first of these. Discussion: https://postgr.es/m/20201014022636.GA1962668@rfd.leadboat.com
* Stabilize timetz test across DST transitions.Tom Lane2020-10-29
| | | | | | | | | | | | | | The timetz test cases I added in commit a9632830b were unintentionally sensitive to whether or not DST is active in the PST8PDT time zone. Thus, they'll start failing this coming weekend, as reported by Bernhard M. Wiedemann in bug #16689. Fortunately, DST-awareness is not significant to the purpose of these test cases, so we can just force them all to PDT (DST hours) to preserve stability of the results. Back-patch to v10, as the prior patch was. Discussion: https://postgr.es/m/16689-57701daa23b377bf@postgresql.org
* Use mode "r" for popen() in psql's evaluate_backtick().Tom Lane2020-10-28
| | | | | | | | | | | | | | | | | | In almost all other places, we use plain "r" or "w" mode in popen() calls (the exceptions being for COPY data). This one has been overlooked (possibly because it's buried in a ".l" flex file?), but it's using PG_BINARY_R. Kensuke Okamura complained in bug #16688 that we fail to strip \r when stripping the trailing newline from a backtick result string. That's true enough, but we'd also fail to convert embedded \r\n cleanly, which also seems undesirable. Fixing the popen() mode seems like the best way to deal with this. It's been like this for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/16688-c649c7b69cd7e6f8@postgresql.org
* Calculate extraUpdatedCols in query rewriter, not parser.Tom Lane2020-10-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's unsafe to do this at parse time because addition of generated columns to a table would not invalidate stored rules containing UPDATEs on the table ... but there might now be dependent generated columns that were not there when the rule was made. This also fixes an oversight that rewriteTargetView failed to update extraUpdatedCols when transforming an UPDATE on an updatable view. (Since the new calculation is downstream of that, rewriteTargetView doesn't actually need to do anything; but before, there was a demonstrable bug there.) In v13 and HEAD, this leads to easily-visible bugs because (since commit c6679e4fc) we won't recalculate generated columns that aren't listed in extraUpdatedCols. In v12 this bitmap is mostly just used for trigger-firing decisions, so you'd only notice a problem if a trigger cared whether a generated column had been updated. I'd complained about this back in May, but then forgot about it until bug #16671 from Michael Paul Killian revived the issue. Back-patch to v12 where this field was introduced. If existing stored rules contain any extraUpdatedCols values, they'll be ignored because the rewriter will overwrite them, so the bug will be fixed even for existing rules. (But note that if someone were to update to 13.1 or 12.5, store some rules with UPDATEs on tables having generated columns, and then downgrade to a prior minor version, they might observe issues similar to what this patch fixes. That seems unlikely enough to not be worth going to a lot of effort to fix.) Discussion: https://postgr.es/m/10206.1588964727@sss.pgh.pa.us Discussion: https://postgr.es/m/16671-2fa55851859fb166@postgresql.org
* Makefile comment: remove reference to tools/thread/thread_testBruce Momjian2020-10-27
| | | | | | | | | | You can't compile thread_test alone anymore, and the location moved too. Reported-by: Tom Lane Discussion: https://postgr.es/m/1062278.1603819969@sss.pgh.pa.us Backpatch-through: 9.5