aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Revert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE."Jeff Davis2020-12-14
| | | | | | | | | | | | | This reverts commit 3a9e64aa0d96c8ffb6c682b082d0f72b1d373327. Commit 4bad60e3 fixed the root of the problem that 3a9e64aa worked around. This enables proper pipelining of commands after terminating replication, eliminating an undocumented limitation. Discussion: https://postgr.es/m/3d57bc29-4459-578b-79cb-7641baf53c57%40iki.fi Backpatch-through: 9.5
* initdb: complete getopt_long alphabetizationBruce Momjian2020-12-12
| | | | Backpatch-through: 9.5
* initdb: properly alphabetize getopt_long options in C stringBruce Momjian2020-12-12
| | | | Backpatch-through: 9.5
* Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.Tom Lane2020-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | array_get_element and array_get_slice qualify as leakproof, since they will silently return NULL for bogus subscripts. But array_set_element and array_set_slice throw errors for such cases, making them clearly not leakproof. contain_leaked_vars was evidently written with only the former case in mind, as it gave the wrong answer for assignment SubscriptingRefs (nee ArrayRefs). This would be a live security bug, were it not that assignment SubscriptingRefs can only occur in INSERT and UPDATE target lists, while we only care about leakproofness for qual expressions; so the wrong answer can't occur in practice. Still, that's a rather shaky answer for a security-related question; and maybe in future somebody will want to ask about leakproofness of a tlist. So it seems wise to fix and even back-patch this correction. (We would need some change here anyway for the upcoming generic-subscripting patch, since extensions might make different tradeoffs about whether to throw errors. Commit 558d77f20 attempted to lay groundwork for that by asking check_functions_in_node whether a SubscriptingRef contains leaky functions; but that idea fails now that the implementation methods of a SubscriptingRef are not SQL-visible functions that could be marked leakproof or not.) Back-patch to 9.6. While 9.5 has the same issue, the code's a bit different. It seems quite unlikely that we'd introduce any actual bug in the short time 9.5 has left to live, so the work/risk/reward balance isn't attractive for changing 9.5. Discussion: https://postgr.es/m/3143742.1607368115@sss.pgh.pa.us
* jit: Correct parameter type for generated expression evaluation functions.Andres Freund2020-12-07
| | | | | | | | | | | | | clang only uses the 'i1' type for scalar booleans, not for pointers to booleans (as the pointer might be pointing into a larger memory allocation). Therefore a pointer-to-bool needs to the "storage" boolean. There's no known case of wrong code generation due to this, but it seems quite possible that it could cause problems (see e.g. 72559438f92). Author: Andres Freund Discussion: https://postgr.es/m/20201207212142.wz5tnbk2jsaqzogb@alap3.anarazel.de Backpatch: 11-, where jit support was added
* backpatch "jit: Add support for LLVM 12."Andres Freund2020-12-07
| | | | | | | | | As there haven't been problem on the buildfarm due to this change, backpatch 6c57f2ed16e now. Author: Andres Freund Discussion: https://postgr.es/m/20201016011244.pmyvr3ee2gbzplq4@alap3.anarazel.de Backpatch: 11-, where jit support was added
* Fix more race conditions in the newly-added pg_rewind test.Heikki Linnakangas2020-12-07
| | | | | | | | | | | | | | | | | | | | | | | | pg_rewind looks at the control file to check what timeline a server is on. But promotion doesn't immediately write a checkpoint, it merely writes an end-of-recovery WAL record. If pg_rewind runs immediately after promotion, before the checkpoint has completed, it will think think that the server is still on the earlier timeline. We ran into this issue a long time ago already, see commit 484a848a73f. It's a bit bogus that pg_rewind doesn't determine the timeline correctly until the end-of-recovery checkpoint has completed. We probably should fix that. But for now work around it by waiting for the checkpoint to complete before running pg_rewind, like we did in commit 484a848a73f. In the passing, tidy up the new test a little bit. Rerder the INSERTs so that the comments make more sense, remove a spurious CHECKPOINT call after pg_rewind has already run, and add --debug option, so that if this fails again, we'll have more data. Per buildfarm failure at https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=rorqual&dt=2020-12-06%2018%3A32%3A19&stg=pg_rewind-check. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/1713707e-e318-761c-d287-5b6a4aa807e8@iki.fi
* Fix missed step in removal of useless RESULT RTEs in the planner.Tom Lane2020-12-05
| | | | | | | | | | | | Commit 4be058fe9 forgot that the append_rel_list would already be populated at the time we remove useless result RTEs, and it might contain PlaceHolderVars that need to be adjusted like the ones in the main parse tree. This could lead to "no relation entry for relid N" failures later on, when the planner tries to do something with an unadjusted PHV. Per report from Tom Ellis. Back-patch to v12 where the bug came in. Discussion: https://postgr.es/m/20201205173056.GF30712@cloudinit-builder
* Fix race conditions in newly-added test.Heikki Linnakangas2020-12-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Buildfarm has been failing sporadically on the new test. I was able to reproduce this by adding a random 0-10 s delay in the walreceiver, just before it connects to the primary. There's a race condition where node_3 is promoted before it has fully caught up with node_1, leading to diverged timelines. When node_1 is later reconfigured as standby following node_3, it fails to catch up: LOG: primary server contains no more WAL on requested timeline 1 LOG: new timeline 2 forked off current database system timeline 1 before current recovery point 0/30000A0 That's the situation where you'd need to use pg_rewind, but in this case it happens already when we are just setting up the actual pg_rewind scenario we want to test, so change the test so that it waits until node_3 is connected and fully caught up before promoting it, so that you get a clean, controlled failover. Also rewrite some of the comments, for clarity. The existing comments detailed what each step in the test did, but didn't give a good overview of the situation the steps were trying to create. For reasons I don't understand, the test setup had to be written slightly differently in 9.6 and 9.5 than in later versions. The 9.5/9.6 version needed node 1 to be reinitialized from backup, whereas in later versions it could be shut down and reconfigured to be a standby. But even 9.5 should support "clean switchover", where primary makes sure that pending WAL is replicated to standby on shutdown. It would be nice to figure out what's going on there, but that's independent of pg_rewind and the scenario that this test tests. Discussion: https://www.postgresql.org/message-id/b0a3b95b-82d2-6089-6892-40570f8c5e60%40iki.fi
* Fix pg_rewind bugs when rewinding a standby server.Heikki Linnakangas2020-12-03
| | | | | | | | | | | | | If the target is a standby server, its WAL doesn't end at the last checkpoint record, but at minRecoveryPoint. We must scan all the WAL from the last common checkpoint all the way up to minRecoveryPoint for modified pages, and also consider that portion when determining whether the server needs rewinding. Backpatch to all supported versions. Author: Ian Barwick and me Discussion: https://www.postgresql.org/message-id/CABvVfJU-LDWvoz4-Yow3Ay5LZYTuPD7eSjjE4kGyNZpXC6FrVQ%40mail.gmail.com
* pg_checksums: data_checksum_version is unsigned so use %u not %dBruce Momjian2020-12-01
| | | | | | | While the previous behavior didn't generate a warning, we might as well use an accurate *printf specification. Backpatch-through: 12
* Ensure that expandTableLikeClause() re-examines the same table.Tom Lane2020-12-01
| | | | | | | | | | | | | | | | | | | | | | As it stood, expandTableLikeClause() re-did the same relation_openrv call that transformTableLikeClause() had done. However there are scenarios where this would not find the same table as expected. We hold lock on the LIKE source table, so it can't be renamed or dropped, but another table could appear before it in the search path. This explains the odd behavior reported in bug #16758 when cloning a table as a temp table of the same name. This case worked as expected before commit 502898192 introduced the need to open the source table twice, so we should fix it. To make really sure we get the same table, let's re-open it by OID not name. That requires adding an OID field to struct TableLikeClause, which is a little nervous-making from an ABI standpoint, but as long as it's at the end I don't think there's any serious risk. Per bug #16758 from Marc Boeren. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16758-840e84a6cfab276d@postgresql.org
* Avoid memcpy() with a NULL source pointer and count == 0Alvaro Herrera2020-12-01
| | | | | | | | | | | | | | | | When memcpy() is called on a pointer, the compiler is entitled to assume that the pointer is not null, which can lead to optimizing nearby code in potentially undesirable ways. We still want such optimizations (gcc's -fdelete-null-pointer-checks) in cases where they're valid. Related: commit 13bba02271dc. Backpatch to pg11, where this particular instance appeared. Reported-by: Ranier Vilela <ranier.vf@gmail.com> Reported-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CAEudQApUndmQkr5fLrCKXQ7+ib44i7S+Kk93pyVThS85PnG3bQ@mail.gmail.com Discussion: https://postgr.es/m/CALNJ-vSdhwSM5f4tnNn1cdLHvXMVe_S+V3nR5GwNrmCPNB2VtQ@mail.gmail.com
* Free disk space for dropped relations on commit.Thomas Munro2020-12-01
| | | | | | | | | | | | | | | | | | | | | When committing a transaction that dropped a relation, we previously truncated only the first segment file to free up disk space (the one that won't be unlinked until the next checkpoint). Truncate higher numbered segments too, even though we unlink them on commit. This frees the disk space immediately, even if other backends have open file descriptors and might take a long time to get around to handling shared invalidation events and closing them. Also extend the same behavior to the first segment, in recovery. Back-patch to all supported releases. Bug: #16663 Reported-by: Denis Patron <denis.patron@previnet.it> Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com> Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com> Reviewed-by: David Zhang <david.zhang@highgo.ca> Discussion: https://postgr.es/m/16663-fe97ccf9932fc800%40postgresql.org
* Prevent parallel index build in a standalone backend.Tom Lane2020-11-30
| | | | | | | | | | | | | | | | | This can't work if there's no postmaster, and indeed the code got an assertion failure trying. There should be a check on IsUnderPostmaster gating the use of parallelism, as the planner has for ordinary parallel queries. Commit 40d964ec9 got this right, so follow its model of checking IsUnderPostmaster at the same place where we check for max_parallel_maintenance_workers == 0. In general, new code implementing parallel utility operations should do the same. Report and patch by Yulin Pei, cosmetically adjusted by me. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/HK0PR01MB22747D839F77142D7E76A45DF4F50@HK0PR01MB2274.apcprd01.prod.exchangelabs.com
* Fix miscomputation of direct_lateral_relids for join relations.Tom Lane2020-11-30
| | | | | | | | | | | | | | If a PlaceHolderVar is to be evaluated at a join relation, but its value is only needed there and not at higher levels, we neglected to update the joinrel's direct_lateral_relids to include the PHV's source rel. This causes problems because join_is_legal() then won't allow joining the joinrel to the PHV's source rel at all, leading to "failed to build any N-way joins" planner failures. Per report from Andreas Seltenreich. Back-patch to 9.5 where the problem originated. Discussion: https://postgr.es/m/87blfgqa4t.fsf@aurora.ydns.eu
* Remove leftover comments, left behind by removal of WITH OIDS.Heikki Linnakangas2020-11-30
| | | | | Author: Amit Langote Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGaRoF3XrhPW-Y7P%2BG7bKo84Z_h%3DkQHvMh-80%3Dav3wmOw%40mail.gmail.com
* Fix recently-introduced breakage in psql's \connect command.Tom Lane2020-11-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Through my misreading of what the existing code actually did, commits 85c54287a et al. broke psql's behavior for the case where "\c connstring" provides a password in the connstring. We should use that password in such a case, but as of 85c54287a we ignored it (and instead, prompted for a password). Commit 94929f1cf fixed that in HEAD, but since I thought it was cleaning up a longstanding misbehavior and not one I'd just created, I didn't back-patch it. Hence, back-patch the portions of 94929f1cf having to do with password management. In addition to fixing the introduced bug, this means that "\c -reuse-previous=on connstring" will allow re-use of an existing connection's password if the connstring doesn't change user/host/port. That didn't happen before, but it seems like a bug fix, and anyway I'm loath to have significant differences in this code across versions. Also fix an error with the same root cause about whether or not to override a connstring's setting of client_encoding. As of 85c54287a we always did so; restore the previous behavior of overriding only when stdin/stdout are a terminal and there's no environment setting of PGCLIENTENCODING. (I find that definition a bit surprising, but right now doesn't seem like the time to revisit it.) Per bug #16746 from Krzysztof Gradek. As with the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16746-44b30e2edf4335d4@postgresql.org
* Retry initial slurp_file("current_logfiles"), in test 004_logrotate.pl.Noah Misch2020-11-28
| | | | | | Buildfarm member topminnow failed when the test script attempted this before the syslogger would have created the file. Back-patch to v12, which introduced the test.
* 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
* 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
* Adjust DSM and DSA slot usage constants (back-patch).Thomas Munro2020-11-20
| | | | | | | | | | | | | | | 1. Previously, a DSA area would create up to four segments at each size before doubling the size. After this commit, it will create only two at each size, so it ramps up faster and therefore needs fewer slots. 2. Previously, the total limit on DSM slots allowed for 2 per connection. Switch to 5 per connection. This back-patches commit d061ea21 from release 13 into 10-12 based on a field complaint. Discussion: https://postgr.es/m/CAO03teA%2BjE1qt5iWDWzHqaufqBsF6EoOgZphnazps_tr_jDPZA%40mail.gmail.com Discussion: https://postgr.es/m/CA%2BhUKGL6H2BpGbiF7Lj6QiTjTGyTLW_vLR%3DSn2tEBeTcYXiMKw%40mail.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 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
* Stamp 12.5.REL_12_5Tom Lane2020-11-09
|
* 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: 3bbbf347254dd914c5ae4b5d0bba9a1ddc28eaa0
* 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 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
* 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
* 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
* 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
* 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
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-24 07:51:03 +0000