sort by:
Revision Author Date Message Commit Date
7384435 Translation updates Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 08f1c10dca3d7b8efc365107c737b87c1c3a82ee 08 February 2021, 16:50:22 UTC
392c530 Last-minute updates for release notes. Security: CVE-2021-3393, CVE-2021-20229 08 February 2021, 16:10:40 UTC
f50e888 Fix permission checks on constraint violation errors on partitions. If a cross-partition UPDATE violates a constraint on the target partition, and the columns in the new partition are in different physical order than in the parent, the error message can reveal columns that the user does not have SELECT permission on. A similar bug was fixed earlier in commit 804b6b6db4. The cause of the bug is that the callers of the ExecBuildSlotValueDescription() function got confused when constructing the list of modified columns. If the tuple was routed from a parent, we converted the tuple to the parent's format, but the list of modified columns was grabbed directly from the child's RTE entry. ExecUpdateLockMode() had a similar issue. That lead to confusion on which columns are key columns, leading to wrong tuple lock being taken on tables referenced by foreign keys, when a row is updated with INSERT ON CONFLICT UPDATE. A new isolation test is added for that corner case. With this patch, the ri_RangeTableIndex field is no longer set for partitions that don't have an entry in the range table. Previously, it was set to the RTE entry of the parent relation, but that was confusing. NOTE: This modifies the ResultRelInfo struct, replacing the ri_PartitionRoot field with ri_RootResultRelInfo. That's a bit risky to backpatch, because it breaks any extensions accessing the field. The change that ri_RangeTableIndex is not set for partitions could potentially break extensions, too. The ResultRelInfos are visible to FDWs at least, and this patch required small changes to postgres_fdw. Nevertheless, this seem like the least bad option. I don't think these fields widely used in extensions; I don't think there are FDWs out there that uses the FDW "direct update" API, other than postgres_fdw. If there is, you will get a compilation error, so hopefully it is caught quickly. Backpatch to 11, where support for both cross-partition UPDATEs, and unique indexes on partitioned tables, were added. Reviewed-by: Amit Langote Security: CVE-2021-3393 08 February 2021, 09:01:55 UTC
e474abd Release notes for 13.2, 12.6, 11.11, 10.16, 9.6.21, 9.5.25. 07 February 2021, 20:46:38 UTC
46f2971 Revert "Propagate CTE property flags when copying a CTE list into a rule." This reverts commit ed290896335414c6c069b9ccae1f3dcdd2fac6ba and equivalent back-branch commits. The issue is subtler than I thought, and it's far from new, so just before a release deadline is no time to be fooling with it. We'll consider what to do at a bit more leisure. Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com 07 February 2021, 17:54:08 UTC
2a7664a Propagate CTE property flags when copying a CTE list into a rule. rewriteRuleAction() neglected this step, although it was careful to propagate other similar flags such as hasSubLinks or hasRowSecurity. Omitting to transfer hasRecursive is just cosmetic at the moment, but omitting hasModifyingCTE is a live bug, since the executor certainly looks at that. The proposed test case only fails back to v10, but since the executor examines hasModifyingCTE in 9.x as well, I suspect that a test case could be devised that fails in older branches. Given the nearness of the release deadline, though, I'm not going to spend time looking for a better test. Report and patch by Greg Nancarrow, cosmetic changes by me Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com 07 February 2021, 00:28:39 UTC
f733219 Disallow converting an inheritance child table to a view. Generally, members of inheritance trees must be plain tables (or, in more recent versions, foreign tables). ALTER TABLE INHERIT rejects creating an inheritance relationship that has a view at either end. When DefineQueryRewrite attempts to convert a relation to a view, it already had checks prohibiting doing so for partitioning parents or children as well as traditional-inheritance parents ... but it neglected to check that a traditional-inheritance child wasn't being converted. Since the planner assumes that any inheritance child is a table, this led to making plans that tried to do a physical scan on a view, causing failures (or even crashes, in recent versions). One could imagine trying to support such a case by expanding the view normally, but since the rewriter runs before the planner does inheritance expansion, it would take some very fundamental refactoring to make that possible. There are probably a lot of other parts of the system that don't cope well with such a situation, too. For now, just forbid it. Per bug #16856 from Yang Lin. Back-patch to all supported branches. (In versions before v10, this includes back-patching the portion of commit 501ed02cf that added has_superclass(). Perhaps the lack of that infrastructure partially explains the missing check.) Discussion: https://postgr.es/m/16856-0363e05c6e1612fd@postgresql.org 06 February 2021, 20:17:01 UTC
9b0ce89 Fix backslash-escaping multibyte chars in COPY FROM. If a multi-byte character is escaped with a backslash in TEXT mode input, and the encoding is one of the client-only encodings where the bytes after the first one can have an ASCII byte "embedded" in the char, we didn't skip the character correctly. After a backslash, we only skipped the first byte of the next character, so if it was a multi-byte character, we would try to process its second byte as if it was a separate character. If it was one of the characters with special meaning, like '\n', '\r', or another '\\', that would cause trouble. One such exmple is the byte sequence '\x5ca45c2e666f6f' in Big5 encoding. That's supposed to be [backslash][two-byte character][.][f][o][o], but because the second byte of the two-byte character is 0x5c, we incorrectly treat it as another backslash. And because the next character is a dot, we parse it as end-of-copy marker, and throw an "end-of-copy marker corrupt" error. Backpatch to all supported versions. Reviewed-by: John Naylor, Kyotaro Horiguchi Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi 05 February 2021, 09:16:33 UTC
77e760d Avoid crash when rolling back within a prepared statement. If a portal is used to run a prepared CALL or DO statement that contains a ROLLBACK, PortalRunMulti fails because the portal's statement list gets cleared by the rollback. (Since the grammar doesn't allow CALL/DO in PREPARE, the only easy way to get to this is via extended query protocol, which treats all inputs as prepared statements.) It's difficult to avoid resetting the portal early because of resource-management issues, so work around this by teaching PortalRunMulti to be wary of portal->stmts having suddenly become NIL. The crash has only been seen to occur in v13 and HEAD (as a consequence of commit 1cff1b95a having added an extra touch of portal->stmts). But even before that, the code involved touching a List that the portal no longer has any claim on. In the test case at hand, the List will still exist because of another refcount on the cached plan; but I'm far from convinced that it's impossible for the cached plan to have been dropped by the time control gets back to PortalRunMulti. Hence, backpatch to v11 where nested transactions were added. Thomas Munro and Tom Lane, per bug #16811 from James Inform Discussion: https://postgr.es/m/16811-c1b599b2c6c2d622@postgresql.org 04 February 2021, 00:38:29 UTC
1dd6baf pg_dump: Fix dumping of inherited generated columns Generation expressions of generated columns are always inherited, so there is no need to set them separately in child tables, and there is no syntax to do so either. The code previously used the code paths for the handling of default values, for which different rules apply; in particular it might want to set a default value explicitly for an inherited column. This resulted in unrestorable dumps. For generated columns, just skip them in inherited tables. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/15830.1575468847%40sss.pgh.pa.us 03 February 2021, 10:58:15 UTC
78fab84 Remove extra increment of plpgsql's statement counter for FOR loops. This left gaps in the internal statement numbering, which is not terribly harmful (else we'd have noticed sooner), but it's not great either. Oversight in bbd5c207b; backpatch to v12 where that came in. Pavel Stehule Discussion: https://postgr.es/m/CAFj8pRDXyQaJmpotNTQVc-t-WxdWZC35V2PnmwOaV1-taidFWA@mail.gmail.com 02 February 2021, 19:35:25 UTC
7428469 Fix ancient memory leak in contrib/auto_explain. The ExecutorEnd hook is invoked in a context that could be quite long-lived, not the executor's own per-query context as I think we were sort of assuming. Thus, any cruft generated while producing the EXPLAIN output could accumulate over multiple queries. This can result in spectacular leakage if log_nested_statements is on, and even without that I'm surprised nobody complained before. To fix, just switch into the executor's context so that anything we allocate will be released when standard_ExecutorEnd frees the executor state. We might as well nuke the code's retail pfree of the explain output string, too; that's laughably inadequate to the need. Japin Li, per report from Jeff Janes. This bug is old, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAMkU=1wCVtbeRn0s9gt12KwQ7PLXovbpM8eg25SYocKW3BT4hg@mail.gmail.com 02 February 2021, 18:49:08 UTC
be843ce Fix CREATE INDEX CONCURRENTLY for simultaneous prepared transactions. In a cluster having used CREATE INDEX CONCURRENTLY while having enabled prepared transactions, queries that use the resulting index can silently fail to find rows. Fix this for future CREATE INDEX CONCURRENTLY by making it wait for prepared transactions like it waits for ordinary transactions. This expands the VirtualTransactionId structure domain to admit prepared transactions. It may be necessary to reindex to recover from past occurrences. Back-patch to 9.5 (all supported versions). Andrey Borodin, reviewed (in earlier versions) by Tom Lane and Michael Paquier. Discussion: https://postgr.es/m/2E712143-97F7-4890-B470-4A35142ABC82@yandex-team.ru 30 January 2021, 08:01:56 UTC
bd63661 Document behavior of the .** jsonpath accessor in the lax mode When the .** jsonpath accessor handles the array, it selects both array and each of its elements. When using lax mode, subsequent accessors automatically unwrap arrays. So, the content of each array element may be selected twice. Even though this behavior is counterintuitive, it's correct because everything works as designed. This commit documents it. Backpatch to 12 where the jsonpath language was introduced. Reported-by: Thomas Kellerer Bug: #16828 Discussion: https://postgr.es/m/16828-2b0229babfad2d8c%40postgresql.org Discussion: https://postgr.es/m/CAPpHfdtS-nNidT%3DEqZbAYOPcnNOWh_sd6skVdu2CAQUGdvpT8Q%40mail.gmail.com Author: Alexandex Korotkov, revised by Tom Lane Reviewed-by: Alvaro Herrera, Thomas Kellerer, Tom Lane Backpatch-through: 12 29 January 2021, 12:28:16 UTC
22f71a6 Silence another gcc 11 warning. Per buildfarm and local experimentation, bleeding-edge gcc isn't convinced that the MemSet in reorder_function_arguments() is safe. Shut it up by adding an explicit check that pronargs isn't negative, and by changing MemSet to memset. (It appears that either change is enough to quiet the warning at -O2, but let's do both to be sure.) 28 January 2021, 22:18:34 UTC
7c53a80 Fix hash partition pruning with asymmetric partition sets. perform_pruning_combine_step() was not taught about the number of partition indexes used in hash partitioning; more embarrassingly, get_matching_hash_bounds() also had it wrong. These errors are masked in the common case where all the partitions have the same modulus and no partition is missing. However, with missing or unequal-size partitions, we could erroneously prune some partitions that need to be scanned, leading to silently wrong query answers. While a minimal-footprint fix for this could be to export get_partition_bound_num_indexes and make the incorrect functions use it, I'm of the opinion that that function should never have existed in the first place. It's not reasonable data structure design that PartitionBoundInfoData lacks any explicit record of the length of its indexes[] array. Perhaps that was all right when it could always be assumed equal to ndatums, but something should have been done about it as soon as that stopped being true. Putting in an explicit "nindexes" field makes both partition_bounds_equal() and partition_bounds_copy() simpler, safer, and faster than before, and removes explicit knowledge of the number-of-partition-indexes rules from some other places too. This change also makes get_hash_partition_greatest_modulus obsolete. I left that in place in case any external code uses it, but no core code does anymore. Per bug #16840 from Michał Albrycht. Back-patch to v11 where the hash partitioning code came in. (In the back branches, add the new field at the end of PartitionBoundInfoData to minimize ABI risks.) Discussion: https://postgr.es/m/16840-571a22976f829ad4@postgresql.org 28 January 2021, 18:41:55 UTC
25f9a72 Make ecpg's rjulmdy() and rmdyjul() agree with their declarations. We had "short *mdy" in the extern declarations, but "short mdy[3]" in the actual function definitions. Per C99 these are equivalent, but recent versions of gcc have started to issue warnings about the inconsistency. Clean it up before the warnings get any more widespread. Back-patch, in case anyone wants to build older PG versions with bleeding-edge compilers. Discussion: https://postgr.es/m/2401575.1611764534@sss.pgh.pa.us 28 January 2021, 16:17:33 UTC
30854d9 pgbench: Remove dead code doConnect() never returns connections in state CONNECTION_BAD, so checking for that is pointless. Remove the code that does. This code has been dead since ba708ea3dc84, 20 years ago. Discussion: https://postgr.es/m/20210126195224.GA20361@alvherre.pgsql Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> 28 January 2021, 15:50:40 UTC
30f912a Don't add bailout adjustment for non-strict deserialize calls. When building aggregate expression steps, strict checks need a bailout jump for when a null value is encountered, so there is a list of steps that require later adjustment. Adding entries to that list for steps that aren't actually strict would be harmless, except that there is an Assert which catches them. This leads to spurious errors on asserts builds, for data sets that trigger parallel aggregation of an aggregate with a non-strict deserialization function (no such aggregates exist in the core system). Repair by not adding the adjustment entry when it's not needed. Backpatch back to 11 where the code was introduced. Per a report from Darafei (Komzpa) of the PostGIS project; analysis and patch by me. Discussion: https://postgr.es/m/87mty7peb3.fsf@news-spur.riddles.org.uk 28 January 2021, 11:09:49 UTC
d43e235 doc: Remove reference to views for TRUNCATE privilege The page about privilege rights mentioned that TRUNCATE could be applied to views or even other relation types. This is confusing as this command can be used only on tables and on partitioned tables. Oversight in afc4a78. Reported-by: Harisai Hari Reviewed-by: Laurenz Albe Discussion: https://postgr.es/m/161157636877.14625.15340884663716426087@wrigleys.postgresql.org Backpatch-through: 12 27 January 2021, 04:41:06 UTC
fdf9d00 Report the true database name on connection errors When reporting connection errors, we might show a database name in the message that's not the one we actually tried to connect to, if the database was taken from libpq defaults instead of from user parameters. Fix such error messages to use PQdb(), which reports the correct name. (But, per commit 2930c05634bc, make sure not to try to print NULL.) Apply to branches 9.5 through 13. Branch master has already been changed differently by commit 58cd8dca3de0. Reported-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/CA+TgmobssJ6rS22dspWnu-oDxXevGmhMD8VcRBjmj-b9UDqRjw@mail.gmail.com 26 January 2021, 19:42:13 UTC
82f97d3 Code review for psql's helpSQL() function. The loops to identify word boundaries could access past the end of the input string. Likely that would never result in an actual crash, but it makes valgrind unhappy. The logic to try different numbers of words didn't work when the input has two words but we only have a match to the first, eg "\h with select". (We must "continue" the pass loop, not "break".) The logic to compute nl_count was bizarrely managed, and in at least two code paths could end up calling PageOutput with nl_count = 0, resulting in failing to paginate output that should have been fed to the pager. Also, in v12 and up, the nl_count calculation hadn't been updated to account for the addition of a URL. The PQExpBuffer holding the command syntax details wasn't freed, resulting in a session-lifespan memory leak. While here, improve some comments, choose a more descriptive name for a variable, fix inconsistent datatype choice for another variable. Per bug #16837 from Alexander Lakhin. This code is very old, so back-patch to all supported branches. Kyotaro Horiguchi and Tom Lane Discussion: https://postgr.es/m/16837-479bcd56040c71b3@postgresql.org 26 January 2021, 18:04:52 UTC
820aa9e Don't clobber the calling user's credentials cache in Kerberos test. Embarrassing oversight in this test script, which fortunately is not run by default. Report and patch by Jacob Champion. Discussion: https://postgr.es/m/1fcb175bafef6560f47a8c31229fa7c938486b8d.camel@vmware.com 25 January 2021, 19:53:13 UTC
4641b2a Fix broken ruleutils support for function TRANSFORM clauses. I chanced to notice that this dumped core due to a faulty Assert. To add insult to injury, the output has been misformatted since v11. Obviously we need some regression testing here. Discussion: https://postgr.es/m/d1cc628c-3953-4209-957b-29427acc38c8@www.fastmail.com 25 January 2021, 18:03:11 UTC
06cdfe2 Fix hypothetical bug in heap backward scans Both heapgettup() and heapgettup_pagemode() incorrectly set the first page to scan in a backward scan in which the number of pages to scan was specified by heap_setscanlimits(). The code incorrectly started the scan at the end of the relation when startBlk was 0, or otherwise at startBlk - 1, neither of which is correct when only scanning a subset of pages. The fix here checks if heap_setscanlimits() has changed the number of pages to scan and if so we set the first page to scan as the final page in the specified range during backward scans. Proper adjustment of this code was forgotten when heap_setscanlimits() was added in 7516f5259 back in 9.5. However, practice, nowhere in core code performs backward scans after having used heap_setscanlimits(), yet, it is possible an extension uses the heap functions in this way, hence backpatch. An upcoming patch does use heap_setscanlimits() with backward scans, so this must be fixed before that can go in. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvpGc9h0_oVD2CtgBcxCS1N-qDYZSeBRnUh+0CWJA9cMaA@mail.gmail.com Backpatch-through: 9.5, all supported versions 25 January 2021, 06:53:27 UTC
5db6ba3 Update time zone data files to tzdata release 2021a. DST law changes in Russia (Volgograd zone) and South Sudan. Historical corrections for Australia, Bahamas, Belize, Bermuda, Ghana, Israel, Kenya, Nigeria, Palestine, Seychelles, and Vanuatu. Notably, the Australia/Currie zone has been corrected to the point where it is identical to Australia/Hobart. 24 January 2021, 21:29:47 UTC
29ba005 Doc: improve directions for building on macOS. In light of recent discussions, we should instruct people to install Apple's command line tools; installing Xcode is secondary. Also, fix sample command for finding out the default sysroot, as we now know that the command originally recommended can give a result that doesn't match your OS version. Also document the workaround to use if you really don't want configure to select a sysroot at all. Discussion: https://postgr.es/m/20210119111625.20435-1-james.hilliard1@gmail.com 22 January 2021, 23:58:45 UTC
878cf60 Doc: remove misleading claim in documentation of PQreset(). This text claimed that the reconnection would occur "to the same server", but there is no such guarantee in the code, nor would insisting on that be an improvement. Back-patch to v10 where multi-host connection strings were added. Discussion: https://postgr.es/m/1095901.1611268376@sss.pgh.pa.us 22 January 2021, 16:29:56 UTC
1cce024 Fix pull_varnos' miscomputation of relids set for a PlaceHolderVar. Previously, pull_varnos() took the relids of a PlaceHolderVar as being equal to the relids in its contents, but that fails to account for the possibility that we have to postpone evaluation of the PHV due to outer joins. This could result in a malformed plan. The known cases end up triggering the "failed to assign all NestLoopParams to plan nodes" sanity check in createplan.c, but other symptoms may be possible. The right value to use is the join level we actually intend to evaluate the PHV at. We can get that from the ph_eval_at field of the associated PlaceHolderInfo. However, there are some places that call pull_varnos() before the PlaceHolderInfos have been created; in that case, fall back to the conservative assumption that the PHV will be evaluated at its syntactic level. (In principle this might result in missing some legal optimization, but I'm not aware of any cases where it's an issue in practice.) Things are also a bit ticklish for calls occurring during deconstruct_jointree(), but AFAICS the ph_eval_at fields should have reached their final values by the time we need them. The main problem in making this work is that pull_varnos() has no way to get at the PlaceHolderInfos. We can fix that easily, if a bit tediously, in HEAD by passing it the planner "root" pointer. In the back branches that'd cause an unacceptable API/ABI break for extensions, so leave the existing entry points alone and add new ones with the additional parameter. (If an old entry point is called and encounters a PHV, it'll fall back to using the syntactic level, again possibly missing some valid optimization.) Back-patch to v12. The computation is surely also wrong before that, but it appears that we cannot reach a bad plan thanks to join order restrictions imposed on the subquery that the PlaceHolderVar came from. The error only became reachable when commit 4be058fe9 allowed trivial subqueries to be collapsed out completely, eliminating their join order restrictions. Per report from Stephan Springl. Discussion: https://postgr.es/m/171041.1610849523@sss.pgh.pa.us 21 January 2021, 20:37:23 UTC
561dd8d Further tweaking of PG_SYSROOT heuristics for macOS. It emerges that in some phases of the moon (perhaps to do with directory entry order?), xcrun will report that the SDK path is /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk which is normally a symlink to a version-numbered sibling directory. Our heuristic to skip non-version-numbered pathnames was rejecting that, which is the wrong thing to do. We'd still like to end up with a version-numbered PG_SYSROOT value, but we can have that by dereferencing the symlink. Like the previous fix, back-patch to all supported versions. Discussion: https://postgr.es/m/522433.1611089678@sss.pgh.pa.us 20 January 2021, 17:07:35 UTC
53a24fa Disable vacuum page skipping in selected test cases. By default VACUUM will skip pages that it can't immediately get exclusive access to, which means that even activities as harmless and unpredictable as checkpoint buffer writes might prevent a page from being processed. Ordinarily this is no big deal, but we have a small number of test cases that examine the results of VACUUM's processing and therefore will fail if the page of interest is skipped. This seems to be the explanation for some rare buildfarm failures. To fix, add the DISABLE_PAGE_SKIPPING option to the VACUUM commands in tests where this could be an issue. In passing, remove a duplicated query in pageinspect/sql/page.sql. Back-patch as necessary (some of these cases are as old as v10). Discussion: https://postgr.es/m/413923.1611006484@sss.pgh.pa.us 20 January 2021, 16:49:29 UTC
0326635 Fix bug in detecting concurrent page splits in GiST insert In commit 9eb5607e699, I got the condition on checking for split or deleted page wrong: I used && instead of ||. The comment correctly said "concurrent split _or_ deletion". As a result, GiST insertion could miss a concurrent split, and insert to wrong page. Duncan Sands demonstrated this with a test script that did a lot of concurrent inserts. Backpatch to v12, where this was introduced. REINDEX is required to fix indexes that were affected by this bug. Backpatch-through: 12 Reported-by: Duncan Sands Discussion: https://www.postgresql.org/message-id/a9690483-6c6c-3c82-c8ba-dc1a40848f11%40deepbluecap.com 20 January 2021, 09:58:27 UTC
5ad672f Fix ALTER DEFAULT PRIVILEGES with duplicated objects Specifying duplicated objects in this command would lead to unique constraint violations in pg_default_acl or "tuple already updated by self" errors. Similarly to GRANT/REVOKE, increment the command ID after each subcommand processing to allow this case to work transparently. A regression test is added by tweaking one of the existing queries of privileges.sql to stress this case. Reported-by: Andrus Author: Michael Paquier Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/ae2a7dc1-9d71-8cba-3bb9-e4cb7eb1f44e@hot.ee Backpatch-through: 9.5 20 January 2021, 02:39:17 UTC
6253159 Remove faulty support for MergeAppend plan with WHERE CURRENT OF. Somebody extended search_plan_tree() to treat MergeAppend exactly like Append, which is 100% wrong, because unlike Append we can't assume that only one input node is actively returning tuples. Hence a cursor using a MergeAppend across a UNION ALL or inheritance tree could falsely match a WHERE CURRENT OF query at a row that isn't actually the cursor's current output row, but coincidentally has the same TID (in a different table) as the current output row. Delete the faulty code; this means that such a case will now return an error like 'cursor "foo" is not a simply updatable scan of table "bar"', instead of silently misbehaving. Users should not find that surprising though, as the same cursor query could have failed that way already depending on the chosen plan. (It would fail like that if the sort were done with an explicit Sort node instead of MergeAppend.) Expand the clearly-inadequate commentary to be more explicit about what this code is doing, in hopes of forestalling future mistakes. It's been like this for awhile, so back-patch to all supported branches. Discussion: https://postgr.es/m/482865.1611075182@sss.pgh.pa.us 19 January 2021, 18:25:45 UTC
6b74362 doc: adjust alignment of doc file list for "pg_waldump.sgml" Backpatch-through: 10 18 January 2021, 23:48:25 UTC
ba80489 Avoid crash with WHERE CURRENT OF and a custom scan plan. execCurrent.c's search_plan_tree() assumed that ForeignScanStates and CustomScanStates necessarily have a valid ss_currentRelation. This is demonstrably untrue for postgres_fdw's remote join and remote aggregation plans, and non-leaf custom scans might not have an identifiable scan relation either. Avoid crashing by ignoring such nodes when the field is null. This solution will lead to errors like 'cursor "foo" is not a simply updatable scan of table "bar"' in cases where maybe we could have allowed WHERE CURRENT OF to work. That's not an issue for postgres_fdw's usages, since joins or aggregations would render WHERE CURRENT OF invalid anyway. But an otherwise-transparent upper level custom scan node might find this annoying. When and if someone cares to expend work on such a scenario, we could invent a custom-scan-provider callback to determine what's safe. Report and patch by David Geier, commentary by me. It's been like this for awhile, so back-patch to all supported branches. Discussion: https://postgr.es/m/0253344d-9bdd-11c4-7f0d-d88c02cd7991@swarm64.com 18 January 2021, 23:32:45 UTC
a0d31b1 Fix pg_dump for GRANT OPTION among initial privileges. The context is an object that no longer bears some aclitem that it bore initially. (A user issued REVOKE or GRANT statements upon the object.) pg_dump is forming SQL to reproduce the object ACL. Since initdb creates no ACL bearing GRANT OPTION, reaching this bug requires an extension where the creation script establishes such an ACL. No PGXN extension does that. If an installation did reach the bug, pg_dump would have omitted a semicolon, causing a REVOKE and the next SQL statement to fail. Separately, since the affected code exists to eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT OPTION FOR. Back-patch to 9.6, where commit 23f34fa4ba358671adab16773e79c17c92cbc870 first appeared. Discussion: https://postgr.es/m/20210109102423.GA160022@rfd.leadboat.com 16 January 2021, 20:21:39 UTC
66087f7 Prevent excess SimpleLruTruncate() deletion. Every core SLRU wraps around. With the exception of pg_notify, the wrap point can fall in the middle of a page. Account for this in the PagePrecedes callback specification and in SimpleLruTruncate()'s use of said callback. Update each callback implementation to fit the new specification. This changes SerialPagePrecedesLogically() from the style of asyncQueuePagePrecedes() to the style of CLOGPagePrecedes(). (Whereas pg_clog and pg_serial share a key space, pg_serial is nothing like pg_notify.) The bug fixed here has the same symptoms and user followup steps as 592a589a04bd456410b853d86bd05faa9432cbbb. Back-patch to 9.5 (all supported versions). Reviewed by Andrey Borodin and (in earlier versions) by Tom Lane. Discussion: https://postgr.es/m/20190202083822.GC32531@gust.leadboat.com 16 January 2021, 20:21:39 UTC
943a113 Disallow CREATE STATISTICS on system catalogs Add a check that CREATE STATISTICS does not add extended statistics on system catalogs, similarly to indexes etc. It can be overriden using the allow_system_table_mods GUC. This bug exists since 7b504eb282c, adding the extended statistics, so backpatch all the way back to PostgreSQL 10. Author: Tomas Vondra Reported-by: Dean Rasheed Backpatch-through: 10 Discussion: https://postgr.es/m/CAEZATCXAPrrOKwEsyZKQ4uzzJQWBCt6QAvOcgqRGdWwT1zb%2BrQ%40mail.gmail.com 15 January 2021, 22:33:16 UTC
f5d044e Improve our heuristic for selecting PG_SYSROOT on macOS. In cases where Xcode is newer than the underlying macOS version, asking xcodebuild for the SDK path will produce a pointer to the SDK shipped with Xcode, which may end up building code that does not work on the underlying macOS version. It appears that in such cases, xcodebuild's answer also fails to match the default behavior of Apple's compiler: assuming one has installed Xcode's "command line tools", there will be an SDK for the OS's own version in /Library/Developer/CommandLineTools, and the compiler will default to using that. This is all pretty poorly documented, but experimentation suggests that "xcrun --show-sdk-path" gives the sysroot path that the compiler is actually using, at least in some cases. Hence, try that first, but revert to xcodebuild if xcrun fails (in very old Xcode, it is missing or lacks the --show-sdk-path switch). Also, "xcrun --show-sdk-path" may give a path that is valid but lacks any OS version identifier. We don't really want that, since most of the motivation for wiring -isysroot into the build flags at all is to ensure that all parts of a PG installation are built against the same SDK, even when considering extensions built later and/or on a different machine. Insist on finding "N.N" in the directory name before accepting the result. (Adding "--sdk macosx" to the xcrun call seems to produce the same answer as xcodebuild, but usually more quickly because it's cached, so we also try that as a fallback.) The core reason why we don't want to use Xcode's default SDK in cases like this is that Apple's technology for introducing new syscalls does not play nice with Autoconf: for example, configure will think that preadv/pwritev exist when using a Big Sur SDK, even when building on an older macOS version where they don't exist. It'd be nice to have a better solution to that problem, but this patch doesn't attempt to fix that. Per report from Sergey Shinderuk. Back-patch to all supported versions. Discussion: https://postgr.es/m/ed3b8e5d-0da8-6ebd-fd1c-e0ac80a4b204@postgrespro.ru 15 January 2021, 16:29:13 UTC
19a1d76 Fix calculation of how much shared memory is required to store a TOC. Commit ac883ac453 refactored shm_toc_estimate() but changed its calculation of shared memory size for TOC incorrectly. Previously this could cause too large memory to be allocated. Back-patch to v11 where the bug was introduced. Author: Takayuki Tsunakawa Discussion: https://postgr.es/m/TYAPR01MB2990BFB73170E2C4921E2C4DFEA80@TYAPR01MB2990.jpnprd01.prod.outlook.com 15 January 2021, 03:45:51 UTC
0d221ec pg_dump: label PUBLICATION TABLE ArchiveEntries with an owner. This is the same fix as commit 9eabfe300 applied to INDEX ATTACH entries, but for table-to-publication attachments. As in that case, even though the backend doesn't record "ownership" of the attachment, we still ought to label it in the dump archive with the role name that should run the ALTER PUBLICATION command. The existing behavior causes the ALTER to be done by the original role that started the restore; that will usually work fine, but there may be corner cases where it fails. The bulk of the patch is concerned with changing struct PublicationRelInfo to include a pointer to the associated PublicationInfo object, so that we can get the owner's name out of that when the time comes. While at it, I rewrote getPublicationTables() to do just one query of pg_publication_rel, not one per table. Back-patch to v10 where this code was introduced. Discussion: https://postgr.es/m/1165710.1610473242@sss.pgh.pa.us 14 January 2021, 21:19:38 UTC
c9b7025 Prevent drop of tablespaces used by partitioned relations When a tablespace is used in a partitioned relation (per commits ca4103025dfe in pg12 for tables and 33e6c34c3267 in pg11 for indexes), it is possible to drop the tablespace, potentially causing various problems. One such was reported in bug #16577, where a rewriting ALTER TABLE causes a server crash. Protect against this by using pg_shdepend to keep track of tablespaces when used for relations that don't keep physical files; we now abort a tablespace if we see that the tablespace is referenced from any partitioned relations. Backpatch this to 11, where this problem has been latent all along. We don't try to create pg_shdepend entries for existing partitioned indexes/tables, but any ones that are modified going forward will be protected. Note slight behavior change: when trying to drop a tablespace that contains both regular tables as well as partitioned ones, you'd previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST. Arguably, the latter is more correct. It is possible to add protecting pg_shdepend entries for existing tables/indexes, by doing ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default; ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace; for each partitioned table/index that is not in the database default tablespace. Because these partitioned objects do not have storage, no file needs to be actually moved, so it shouldn't take more time than what's required to acquire locks. This query can be used to search for such relations: SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0 Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/16577-881633a9f9894fd5@postgresql.org Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Michael Paquier <michael@paquier.xyz> 14 January 2021, 18:32:14 UTC
259b212 Call out vacuum considerations in create index docs Backpatch to pg12, which is as far as it goes without conflicts. Author: James Coleman <jtc331@gmail.com> Reviewed-by: "David G. Johnston" <david.g.johnston@gmail.com> Discussion: https://postgr.es/m/CAAaqYe9oEfbz7AxXq7OX+FFVi5w5p1e_Of8ON8ZnKO9QqBfmjg@mail.gmail.com 13 January 2021, 20:55:41 UTC
3e214fa Disallow a digit as the first character of a variable name in pgbench. The point of this restriction is to avoid trying to substitute variables into timestamp literal values, which may contain strings like '12:34'. There is a good deal more that should be done to reduce pgbench's tendency to substitute where it shouldn't. But this is sufficient to solve the case complained of by Jaime Soler, and it's simple enough to back-patch. Back-patch to v11; before commit 9d36a3866, pgbench had a slightly different definition of what a variable name is, and anyway it seems unwise to change long-stable branches for this. Fabien Coelho Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2006291740420.805678@pseudo 13 January 2021, 19:52:49 UTC
0363b7e Doc: clarify behavior of back-half options in pg_dump. Options that change how the archive data is converted to SQL text are ignored when dumping to archive formats. The documentation previously said "not meaningful", which is not helpful. Discussion: https://postgr.es/m/161052021249.12228.9598689907884726185@wrigleys.postgresql.org 13 January 2021, 18:30:04 UTC
0b2ae3c Fix memory leak in SnapBuildSerialize. The memory for the snapshot was leaked while serializing it to disk during logical decoding. This memory will be freed only once walsender stops streaming the changes. This can lead to a huge memory increase when master logs Standby Snapshot too frequently say when the user is trying to create many replication slots. Reported-by: funnyxj.fxj@alibaba-inc.com Diagnosed-by: funnyxj.fxj@alibaba-inc.com Author: Amit Kapila Backpatch-through: 9.5 Discussion: https://postgr.es/m/033ab54c-6393-42ee-8ec9-2b399b5d8cde.funnyxj.fxj@alibaba-inc.com 13 January 2021, 03:10:06 UTC
436d9c5 pg_dump: label INDEX ATTACH ArchiveEntries with an owner. Although a partitioned index's attachment to its parent doesn't have separate ownership, the ArchiveEntry for it needs to be marked with an owner anyway, to ensure that the ALTER command is run by the appropriate role when restoring with --use-set-session-authorization. Without this, the ALTER will be run by the role that started the restore session, which will usually work but it's formally the wrong thing. Back-patch to v11 where this type of ArchiveEntry was added. In HEAD, add equivalent commentary to the just-added TABLE ATTACH case, which I'd made do the right thing already. Discussion: https://postgr.es/m/1094034.1610418498@sss.pgh.pa.us 12 January 2021, 18:37:38 UTC
a2768e4 Doc: fix description of privileges needed for ALTER PUBLICATION. Adding a table to a publication requires ownership of the table (in addition to ownership of the publication). This was mentioned nowhere. 12 January 2021, 17:52:28 UTC
ddf0be8 Fix thinko in comment This comment has been wrong since its introduction in commit 2c03216d8311. Author: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAD21AoAzz6qipFJBbGEaHmyWxvvNDp8httbwLR9tUQWaTjUs2Q@mail.gmail.com 12 January 2021, 14:48:45 UTC
931c0bd doc: expand description of how non-SELECT queries are processed The previous description of how the executor processes non-SELECT queries was very dense, causing lack of clarity. This expanded text spells it out more simply. Reported-by: fotis.koutoupas@gmail.com Discussion: https://postgr.es/m/160912275508.676.17469511338925622905@wrigleys.postgresql.org Backpatch-through: 9.5 09 January 2021, 17:11:16 UTC
8354371 Fix ancient bug in parsing of BRE-mode regular expressions. brenext(), when parsing a '*' quantifier, forgot to return any "value" for the token; per the equivalent case in next(), it should return value 1 to indicate that greedy rather than non-greedy behavior is wanted. The result is that the compiled regexp could behave like 'x*?' rather than the intended 'x*', if we were unlucky enough to have a zero in v->nextvalue at this point. That seems to happen with some reliability if we have '.*' at the beginning of a BRE-mode regexp, although that depends on the initial contents of a stack-allocated struct, so it's not guaranteed to fail. Found by Alexander Lakhin using valgrind testing. This bug seems to be aboriginal in Spencer's code, so back-patch all the way. Discussion: https://postgr.es/m/16814-6c5e3edd2bdf0d50@postgresql.org 08 January 2021, 17:16:00 UTC
29556bf Adjust createdb TAP tests to work on recent OpenBSD. We found last February that the error-case tests added by commit 008cf0409 failed on OpenBSD, because that platform doesn't really check locale names. At the time it seemed that that was only an issue for LC_CTYPE, but testing on a more recent version of OpenBSD shows that it's now equally lax about LC_COLLATE. Rather than dropping the LC_COLLATE test too, put back LC_CTYPE (reverting c4b0edb07), and adjust these tests to accept the different error message that we get if setlocale() doesn't reject a bogus locale name. The point of these tests is not really what the backend does with the locale name, but to show that createdb quotes funny locale names safely; so we're not losing test reliability this way. Back-patch as appropriate. Discussion: https://postgr.es/m/231373.1610058324@sss.pgh.pa.us 08 January 2021, 01:36:09 UTC
f2a69e9 Further second thoughts about idle_session_timeout patch. On reflection, the order of operations in PostgresMain() is wrong. These timeouts ought to be shut down before, not after, we do the post-command-read CHECK_FOR_INTERRUPTS, to guarantee that any timeout error will be detected there rather than at some ill-defined later point (possibly after having wasted a lot of work). This is really an error in the original idle_in_transaction_timeout patch, so back-patch to 9.6 where that was introduced. 07 January 2021, 16:45:09 UTC
820f882 doc: Remove reference to recovery params for divergence lookup in pg_rewind The documentation of pg_rewind mentioned the use of restore_command and primary_conninfo as options available at startup to fetch missing WAL segments that could be used to find the point of divergence for the rewind. This is confusing because when finding the point of divergence the target cluster is offline, so this option is not available. Issue introduced by 878bd9a, so backpatch down to 9.6. The documentation of 13 and HEAD was already right as this sentence has been changed by a7e8ece when introducing -c/--restore-target-wal. Reported-by: Amine Tengilimoglu Discussion: https://postgr.es/m/CADTdw-w_0MP=iQrfizeU4QU5fcZb+w8P_oPeLL+WznWf0kbn3w@mail.gmail.com Backpatch-through: 9.6 07 January 2021, 11:50:35 UTC
9f540f8 Detect the deadlocks between backends and the startup process. The deadlocks that the recovery conflict on lock is involved in can happen between hot-standby backends and the startup process. If a backend takes an access exclusive lock on the table and which finally triggers the deadlock, that deadlock can be detected as expected. On the other hand, previously, if the startup process took an access exclusive lock and which finally triggered the deadlock, that deadlock could not be detected and could remain even after deadlock_timeout passed. This is a bug. The cause of this bug was that the code for handling the recovery conflict on lock didn't take care of deadlock case at all. It assumed that deadlocks involving the startup process and backends were able to be detected by the deadlock detector invoked within backends. But this assumption was incorrect. The startup process also should have invoked the deadlock detector if necessary. To fix this bug, this commit makes the startup process invoke the deadlock detector if deadlock_timeout is reached while handling the recovery conflict on lock. Specifically, in that case, the startup process requests all the backends holding the conflicting locks to check themselves for deadlocks. Back-patch to v9.6. v9.5 has also this bug, but per discussion we decided not to back-patch the fix to v9.5. Because v9.5 doesn't have some infrastructure codes (e.g., 37c54863cf) that this bug fix patch depends on. We can apply those codes for the back-patch, but since the next minor version release is the final one for v9.5, it's risky to do that. If we unexpectedly introduce new bug to v9.5 by the back-patch, there is no chance to fix that. We determined that the back-patch to v9.5 would give more risk than gain. Author: Fujii Masao Reviewed-by: Bertrand Drouvot, Masahiko Sawada, Kyotaro Horiguchi Discussion: https://postgr.es/m/4041d6b6-cf24-a120-36fa-1294220f8243@oss.nttdata.com 06 January 2021, 03:31:23 UTC
c919ed1 doc: Fix description about default behavior of recovery_target_timeline. The default value of recovery_target_timeline was changed in v12, but the description about the default behavior of that was not updated. Back-patch to v12 where the default behavior of recovery_target_timeline was changed. Author: Benoit Lobréau Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAPE8EZ7c3aruEmM24GYkj8y8WmHKD1m9TtPtgCF0nQ3zw4LCkQ@mail.gmail.com 06 January 2021, 02:59:45 UTC
3e9cea2 doc: improve NLS instruction wording Reported-by: "Tang, Haiying" Discussion: https://postgr.es/m/bbbccf7a3c2d436e85d45869d612fd6b@G08CNEXMBPEKD05.g08.fujitsu.local Author: "Tang, Haiying" Backpatch-through: 9.5 05 January 2021, 19:26:37 UTC
740780a Add an explicit cast to double when using fabs(). Commit bc43b7c2c0 used fabs() directly on an int variable, which apparently requires an explicit cast on some platforms. Per buildfarm. 05 January 2021, 11:50:11 UTC
258b770 Fix numeric_power() when the exponent is INT_MIN. In power_var_int(), the computation of the number of significant digits to use in the computation used log(Abs(exp)), which isn't safe because Abs(exp) returns INT_MIN when exp is INT_MIN. Use fabs() instead of Abs(), so that the exponent is cast to a double before the absolute value is taken. Back-patch to 9.6, where this was introduced (by 7d9a4737c2). Discussion: https://postgr.es/m/CAEZATCVd6pMkz=BrZEgBKyqqJrt2xghr=fNc8+Z=5xC6cgWrWA@mail.gmail.com 05 January 2021, 11:07:12 UTC
7c98759 Fix integer-overflow corner cases in substring() functions. If the substring start index and length overflow when added together, substring() misbehaved, either throwing a bogus "negative substring length" error on a case that should succeed, or failing to complain that a negative length is negative (and instead returning the whole string, in most cases). Unsurprisingly, the text, bytea, and bit variants of the function all had this issue. Rearrange the logic to ensure that negative lengths are always rejected, and add an overflow check to handle the other case. Also install similar guards into detoast_attr_slice() (nee heap_tuple_untoast_attr_slice()), since it's far from clear that no other code paths leading to that function could pass it values that would overflow. Patch by myself and Pavel Stehule, per bug #16804 from Rafi Shamim. Back-patch to v11. While these bugs are old, the common/int.h infrastructure for overflow-detecting arithmetic didn't exist before commit 4d6ad3125, and it doesn't seem like these misbehaviors are bad enough to justify developing a standalone fix for the older branches. Discussion: https://postgr.es/m/16804-f4eeeb6c11ba71d4@postgresql.org 04 January 2021, 23:32:40 UTC
9a968e7 Update copyright for 2021 Backpatch-through: 9.5 02 January 2021, 18:06:24 UTC
5706c48 Doc: improve explanation of EXTRACT(EPOCH) for timestamp without tz. Try to be clearer about what computation is actually happening here. Per bug #16797 from Dana Burd. Discussion: https://postgr.es/m/16797-f264b0b980b53b8b@postgresql.org 01 January 2021, 20:51:09 UTC
5f95664 Get heap page max offset with buffer lock held. On further reflection it seems better to call PageGetMaxOffsetNumber() after acquiring a buffer lock on the page. This shouldn't really matter, but doing it this way is cleaner. Follow-up to commit 42288174. Backpatch: 12-, just like commit 42288174 31 December 2020, 01:21:39 UTC
4f70e09 Fix index deletion latestRemovedXid bug. The logic for determining the latest removed XID for the purposes of generating recovery conflicts in REDO routines was subtly broken. It failed to follow links from HOT chains, and so failed to consider all relevant heap tuple headers in some cases. To fix, expand the loop that deals with LP_REDIRECT line pointers to also deal with HOT chains. The new version of the loop is loosely based on a similar loop from heap_prune_chain(). The impact of this bug is probably quite limited, since the horizon code necessarily deals with heap tuples that are pointed to by LP_DEAD-set index tuples. The process of setting LP_DEAD index tuples (e.g. within the kill_prior_tuple mechanism) is highly correlated with opportunistic pruning of pointed-to heap tuples. Plus the question of generating a recovery conflict usually comes up some time after index tuple LP_DEAD bits were initially set, unlike heap pruning, where a latestRemovedXid is generated at the point of the pruning operation (heap pruning has no deferred "would-be page split" style processing that produces conflicts lazily). Only backpatch to Postgres 12, the first version where this logic runs during original execution (following commit 558a9165e08). The index latestRemovedXid mechanism has had the same bug since it first appeared over 10 years ago (in commit a760893d), but backpatching to all supported versions now seems like a bad idea on balance. Running the new improved code during recovery seems risky, especially given the lack of complaints from the field. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wz=Eib393+HHcERK_9MtgNS7Ew1HY=RDC_g6GL46zM5C6Q@mail.gmail.com Backpatch: 12- 31 December 2020, 00:29:01 UTC
e3bfdf2 Doc: spell out comparison behaviors for the date/time types. The behavior of cross-type comparisons among date/time data types was not really explained anywhere. You could probably infer it if you recognized the applicability of comments elsewhere about datatype conversions, but it seems worthy of explicit documentation. Per bug #16797 from Dana Burd. Discussion: https://postgr.es/m/16797-f264b0b980b53b8b@postgresql.org 30 December 2020, 22:48:43 UTC
3ca1949 Fix up usage of krb_server_keyfile GUC parameter. secure_open_gssapi() installed the krb_server_keyfile setting as KRB5_KTNAME unconditionally, so long as it's not empty. However, pg_GSS_recvauth() only installed it if KRB5_KTNAME wasn't set already, leading to a troubling inconsistency: in theory, clients could see different sets of server principal names depending on whether they use GSSAPI encryption. Always using krb_server_keyfile seems like the right thing, so make both places do that. Also fix up secure_open_gssapi()'s lack of a check for setenv() failure --- it's unlikely, surely, but security-critical actions are no place to be sloppy. Also improve the associated documentation. This patch does nothing about secure_open_gssapi()'s use of setenv(), and indeed causes pg_GSS_recvauth() to use it too. That's nominally against project portability rules, but since this code is only built with --with-gssapi, I do not feel a need to do something about this in the back branches. A fix will be forthcoming for HEAD though. Back-patch to v12 where GSSAPI encryption was introduced. The dubious behavior in pg_GSS_recvauth() goes back further, but it didn't have anything to be inconsistent with, so let it be. Discussion: https://postgr.es/m/2187460.1609263156@sss.pgh.pa.us 30 December 2020, 16:38:42 UTC
635140a In pg_upgrade cross-version test, handle lack of oldstyle_length(). This suffices for testing v12 -> v13; some other version pairs need more changes. Back-patch to v10, which removed the function. 30 December 2020, 09:43:46 UTC
9ad2689 doc: Improve some grammar and sentences 90fbf7c has taken care of that for HEAD. This includes the portion of the fixes that applies to the documentation, where needed depending on the branch. Author: Justin Pryzby Discussion: https://postgr.es/m/20201227202604.GC26311@telsasoft.com Backpatch-through: 9.5 29 December 2020, 09:19:11 UTC
7ed616f Improve log messages related to pg_hba.conf not matching a connection. Include details on whether GSS encryption has been activated; since we added "hostgssenc" type HBA entries, that's relevant info. Kyotaro Horiguchi and Tom Lane. Back-patch to v12 where GSS encryption was introduced. Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se 28 December 2020, 22:58:58 UTC
4cfdd8a Fix assorted issues in backend's GSSAPI encryption support. Unrecoverable errors detected by GSSAPI encryption can't just be reported with elog(ERROR) or elog(FATAL), because attempting to send the error report to the client is likely to lead to infinite recursion or loss of protocol sync. Instead make this code do what the SSL encryption code has long done, which is to just report any such failure to the server log (with elevel COMMERROR), then pretend we've lost the connection by returning errno = ECONNRESET. Along the way, fix confusion about whether message translation is done by pg_GSS_error() or its callers (the latter should do it), and make the backend version of that function work more like the frontend version. Avoid allocating the port->gss struct until it's needed; we surely don't need to allocate it in the postmaster. Improve logging of "connection authorized" messages with GSS enabled. (As part of this, I back-patched the code changes from dc11f31a1.) Make BackendStatusShmemSize() account for the GSS-related space that will be allocated by CreateSharedBackendStatus(). This omission could possibly cause out-of-shared-memory problems with very high max_connections settings. Remove arbitrary, pointless restriction that only GSS authentication can be used on a GSS-encrypted connection. Improve documentation; notably, document the fact that libpq now prefers GSS encryption over SSL encryption if both are possible. Per report from Mikael Gustavsson. Back-patch to v12 where this code was introduced. Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se 28 December 2020, 22:44:17 UTC
b3a5bf7 Fix bugs in libpq's GSSAPI encryption support. The critical issue fixed here is that if a GSSAPI-encrypted connection is successfully made, pqsecure_open_gss() cleared conn->allow_ssl_try, as an admittedly-hacky way of preventing us from then trying to tunnel SSL encryption over the already-encrypted connection. The problem with that is that if we abandon the GSSAPI connection because of a failure during authentication, we would not attempt SSL encryption in the next try with the same server. This can lead to unexpected connection failure, or silently getting a non-encrypted connection where an encrypted one is expected. Fortunately, we'd only manage to make a GSSAPI-encrypted connection if both client and server hold valid tickets in the same Kerberos infrastructure, which is a relatively uncommon environment. Nonetheless this is a very nasty bug with potential security consequences. To fix, don't reset the flag, instead adding a check for conn->gssenc being already true when deciding whether to try to initiate SSL. While here, fix some lesser issues in libpq's GSSAPI code: * Use the need_new_connection stanza when dropping an attempted GSSAPI connection, instead of partially duplicating that code. The consequences of this are pretty minor: AFAICS it could only lead to auth_req_received or password_needed remaining set when they shouldn't, which is not too harmful. * Fix pg_GSS_error() to not repeat the "mprefix" it's given multiple times, and to notice any failure return from gss_display_status(). * Avoid gratuitous dependency on NI_MAXHOST in pg_GSS_load_servicename(). Per report from Mikael Gustavsson. Back-patch to v12 where this code was introduced. Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se 28 December 2020, 20:43:44 UTC
d379659 Further fix thinko in plpgsql memory leak fix. There's a second call of get_eval_mcontext() that should also be get_stmt_mcontext(). This is actually dead code, since no interesting allocations happen before switching back to the original context, but we should keep it in sync with the other call to forestall possible future bugs. Discussion: https://postgr.es/m/f075f7be-c654-9aa8-3ffc-e9214622f02a@enterprisedb.com 28 December 2020, 16:55:37 UTC
fd1347f Fix thinko in plpgsql memory leak fix. Commit a6b1f5365 intended to place the transient "target" list of a CALL statement in the function's statement-lifespan context, but I fat-fingered that and used get_eval_mcontext() instead of get_stmt_mcontext(). The eval_mcontext belongs to the "simple expression" infrastructure, which is destroyed at transaction end. The net effect is that a CALL in a procedure to another procedure that has OUT or INOUT parameters would fail if the called procedure did a COMMIT. Per report from Peter Eisentraut. Back-patch to v11, like the prior patch. Discussion: https://postgr.es/m/f075f7be-c654-9aa8-3ffc-e9214622f02a@enterprisedb.com 28 December 2020, 16:41:25 UTC
190f0af Fix inconsistent code with shared invalidations of snapshots The code in charge of processing a single invalidation message has been using since 568d413 the structure for relation mapping messages. This had fortunately no consequence as both locate the database ID at the same location, but it could become a problem in the future if this area of the code changes. Author: Konstantin Knizhnik Discussion: https://postgr.es/m/8044c223-4d3a-2cdb-42bf-29940840ce94@postgrespro.ru Backpatch-through: 9.5 28 December 2020, 13:17:02 UTC
e792ca4 postgres_fdw: Fix connection leak. In postgres_fdw, the cached connections to foreign servers will not be closed until the local session exits if the user mappings or foreign servers that those connections depend on are dropped. Those connections can be leaked. To fix that connection leak issue, after a change to a pg_foreign_server or pg_user_mapping catalog entry, this commit makes postgres_fdw close the connections depending on that entry immediately if current transaction has not used those connections yet. Otherwise, mark those connections as invalid and then close them at the end of current transaction, since they cannot be closed in the midst of the transaction using them. Closed connections will be remade at the next opportunity if necessary. Back-patch to all supported branches. Author: Bharath Rupireddy Reviewed-by: Zhihong Yu, Zhijie Hou, Fujii Masao Discussion: https://postgr.es/m/CALj2ACVNcGH_6qLY-4_tXz8JLvA+4yeBThRfxMz7Oxbk1aHcpQ@mail.gmail.com 28 December 2020, 10:59:00 UTC
c966988 Invalidate acl.c caches when pg_authid changes. This makes existing sessions reflect "ALTER ROLE ... [NO]INHERIT" as quickly as they have been reflecting "GRANT role_name". Back-patch to 9.5 (all supported versions). Reviewed by Nathan Bossart. Discussion: https://postgr.es/m/20201221095028.GB3777719@rfd.leadboat.com 25 December 2020, 18:42:03 UTC
f853ffa Avoid time-of-day-dependent failure in log rotation test. Buildfarm members pogona and petalura have shown a failure when pg_ctl/t/004_logrotate.pl starts just before local midnight. The default rotate-at-midnight behavior occurs just before the Perl script examines current_logfiles, so it figures that the rotation it's already requested has occurred ... but in reality, that rotation happens just after it looks, so the expected new log data goes into a different file than the one it's examining. In HEAD, src/test/kerberos/t/001_auth.pl has acquired similar code that evidently has a related failure mode. Besides being quite new, few buildfarm critters run that test, so it's unsurprising that we've not yet seen a failure there. Fix both cases by setting log_rotation_age = 0 so that no time-based rotation can occur. Also absorb 004_logrotate.pl's decision to set lc_messages = 'C' into the kerberos test, in hopes that it will work in non-English prevailing locales. Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pogona&dt=2020-12-24%2022%3A10%3A04 Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2020-02-01%2022%3A20%3A04 25 December 2020, 02:37:46 UTC
3d8068e Fix race condition between shutdown and unstarted background workers. If a database shutdown (smart or fast) is commanded between the time some process decides to request a new background worker and the time that the postmaster can launch that worker, then nothing happens because the postmaster won't launch any bgworkers once it's exited PM_RUN state. This is fine ... unless the requesting process is waiting for that worker to finish (or even for it to start); in that case the requestor is stuck, and only manual intervention will get us to the point of being able to shut down. To fix, cancel pending requests for workers when the postmaster sends shutdown (SIGTERM) signals, and similarly cancel any new requests that arrive after that point. (We can optimize things slightly by only doing the cancellation for workers that have waiters.) To fit within the existing bgworker APIs, the "cancel" is made to look like the worker was started and immediately stopped, causing deregistration of the bgworker entry. Waiting processes would have to deal with premature worker exit anyway, so this should introduce no bugs that weren't there before. We do have a side effect that registration records for restartable bgworkers might disappear when theoretically they should have remained in place; but since we're shutting down, that shouldn't matter. Back-patch to v10. There might be value in putting this into 9.6 as well, but the management of bgworkers is a bit different there (notably see 8ff518699) and I'm not convinced it's worth the effort to validate the patch for that branch. Discussion: https://postgr.es/m/661570.1608673226@sss.pgh.pa.us 24 December 2020, 22:00:43 UTC
8985e02 docs: document which server-side languages can create procs This was missed when the feature was added. Reported-by: Daniel Westermann Discussion: https://postgr.es/m/160624532969.25818.4767632047905006142@wrigleys.postgresql.org Backpatch-through: 11 23 December 2020, 14:37:38 UTC
67a9411 Fix portability issues with parsing of recovery_target_xid The parsing of this parameter has been using strtoul(), which is not portable across platforms. On most Unix platforms, unsigned long has a size of 64 bits, while on Windows it is 32 bits. It is common in recovery scenarios to rely on the output of txid_current() or even the newer pg_current_xact_id() to get a transaction ID for setting up recovery_target_xid. The value returned by those functions includes the epoch in the computed result, which would cause strtoul() to fail where unsigned long has a size of 32 bits once the epoch is incremented. WAL records and 2PC data include only information about 32-bit XIDs and it is not possible to have XIDs across more than one epoch, so discarding the high bits from the transaction ID set has no impact on recovery. On the contrary, the use of strtoul() prevents a consistent behavior across platforms depending on the size of unsigned long. This commit changes the parsing of recovery_target_xid to use pg_strtouint64() instead, available down to 9.6. There is one TAP test stressing recovery with recovery_target_xid, where a tweak based on pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets tested, as per an idea from Alexander Lakhin. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/16780-107fd0c0385b1035@postgresql.org Backpatch-through: 9.6 23 December 2020, 03:51:39 UTC
f581e53 Improve autoprewarm's handling of early-shutdown scenarios. Bad things happen if the DBA issues "pg_ctl stop -m fast" before autoprewarm finishes loading its list of blocks to prewarm. The current worker process successfully terminates early, but (if this wasn't the last database with blocks to prewarm) the leader process will just try to launch another worker for the next database. Since the postmaster is now in PM_WAIT_BACKENDS state, it ignores the launch request, and the leader just sits until it's killed manually. This is mostly the fault of our half-baked design for launching background workers, but a proper fix for that is likely to be too invasive to be back-patchable. To ameliorate the situation, fix apw_load_buffers() to check whether SIGTERM has arrived just before trying to launch another worker. That leaves us with only a very narrow window in each worker launch where SIGTERM could occur between the launch request and successful worker start. Another issue is that if the leader process does manage to exit, it unconditionally rewrites autoprewarm.blocks with only the blocks currently in shared buffers, thus forgetting any blocks that we hadn't reached yet while prewarming. This seems quite unhelpful, since the next database start will then not have the expected prewarming benefit. Fix it to not modify the file if we shut down before the initial load attempt is complete. Per bug #16785 from John Thompson. Back-patch to v11 where the autoprewarm code was introduced. Discussion: https://postgr.es/m/16785-c0207d8c67fb5f25@postgresql.org 22 December 2020, 18:23:49 UTC
38bef9e Remove "invalid concatenation of jsonb objects" error case. The jsonb || jsonb operator arbitrarily rejected certain combinations of scalar and non-scalar inputs, while being willing to concatenate other combinations. This was of course quite undocumented. Rather than trying to document it, let's just remove the restriction, creating a uniform rule that unless we are handling an object-to-object concatenation, non-array inputs are converted to one-element arrays, resulting in an array-to-array concatenation. (This does not change the behavior for any case that didn't throw an error before.) Per complaint from Joel Jacobson. Back-patch to all supported branches. Discussion: https://postgr.es/m/163099.1608312033@sss.pgh.pa.us 21 December 2020, 18:11:29 UTC
def510c Doc: fix description of how to use src/tutorial files. The separate "cd" command before invoking psql made sense (or at least I thought so) when it was added in commit ed1939332. But 4e3a61635 removed the supporting text that explained when to use it, making it just confusing. So drop it. Also switch from four-dot to three-dot filler for the unsupplied part of the path, since at least one person has read the four-dot filler as a typo for "../..". And fix these/those inconsistency. Discussion: https://postgr.es/m/160837647714.673.5195186835607800484@wrigleys.postgresql.org 20 December 2020, 20:28:22 UTC
3989fda Doc: improve description of pgbench script weights. Point out the workaround to be used if you want to write a script file name that includes "@". Clean up the text a little. Fabien Coelho, additional wordsmithing by me Discussion: https://postgr.es/m/1c4e81550d214741827a03292222db8d@G08CNEXMBPEKD06.g08.fujitsu.local 20 December 2020, 18:37:25 UTC
ba04ced Avoid memcpy() with same source and destination during relmapper init. A narrow reading of the C standard says that memcpy(x,x,n) is undefined, although it's hard to envision an implementation that would really misbehave. However, analysis tools such as valgrind might whine about this; accordingly, let's band-aid relmapper.c to not do it. See also 5b630501e, d3f4e8a8a, ad7b48ea0, and other similar fixes. Apparently, none of those folk tried valgrinding initdb? This has been like this for long enough that I'm surprised it hasn't been reported before. Back-patch, just in case anybody wants to use a back branch on a platform that complains about this; we back-patched those earlier fixes too. Discussion: https://postgr.es/m/161790.1608310142@sss.pgh.pa.us 18 December 2020, 20:46:44 UTC
f32c687 doc: clarify COPY TO for partitioning/inheritance It was not clear how COPY TO behaved with partitioning/inheritance because the paragraphs were so far apart. Also reword to simplify. Discussion: https://postgr.es/m/20201203211723.GR24052@telsasoft.com Author: Justin Pryzby Backpatch-through: 10 16 December 2020, 00:20:15 UTC
bf88c6d Use native methods to open input in TestLib::slurp_file on Windows. This is a backport of commits 114541d58e and 6f59826f0 to the remaining live branches. 15 December 2020, 15:00:48 UTC
706d84f Revert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE." 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 15 December 2020, 07:48:04 UTC
8e456b8 initdb: complete getopt_long alphabetization Backpatch-through: 9.5 12 December 2020, 17:59:09 UTC
035c7df initdb: properly alphabetize getopt_long options in C string Backpatch-through: 9.5 12 December 2020, 17:51:16 UTC
2f1997b Teach contain_leaked_vars that assignment SubscriptingRefs are leaky. 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 08 December 2020, 22:50:54 UTC
e824ddc Doc: clarify that CREATE TABLE discards redundant unique constraints. The SQL standard says that redundant unique constraints are disallowed, but we long ago decided that throwing an error would be too user-unfriendly, so we just drop redundant ones. The docs weren't very clear about that though, as this behavior was only explained for PRIMARY KEY vs UNIQUE, not UNIQUE vs UNIQUE. While here, I couldn't resist doing some copy-editing and markup-fixing on the adjacent text about INCLUDE options. Per bug #16767 from Matthias vd Meent. Discussion: https://postgr.es/m/16767-1714a2056ca516d0@postgresql.org 08 December 2020, 18:09:48 UTC
101c8ae Doc: explain that the string types can't store \0 (ASCII NUL). This restriction was mentioned in connection with string literals, but it wasn't made clear that it's a general restriction not just a syntactic limitation in query strings. Per unsigned documentation comment. Discussion: https://postgr.es/m/160720552914.710.16625261471128631268@wrigleys.postgresql.org 08 December 2020, 17:06:19 UTC
d7ecba9 pgcrypto: Detect errors with EVP calls from OpenSSL The following routines are called within pgcrypto when handling digests but there were no checks for failures: - EVP_MD_CTX_size (can fail with -1 as of 3.0.0) - EVP_MD_CTX_block_size (can fail with -1 as of 3.0.0) - EVP_DigestInit_ex - EVP_DigestUpdate - EVP_DigestFinal_ex A set of elog(ERROR) is added by this commit to detect such failures, that should never happen except in the event of a processing failure internal to OpenSSL. Note that it would be possible to use ERR_reason_error_string() to get more context about such errors, but these refer mainly to the internals of OpenSSL, so it is not really obvious how useful that would be. This is left out for simplicity. Per report from Coverity. Thanks to Tom Lane for the discussion. Backpatch-through: 9.5 08 December 2020, 06:22:43 UTC
27b57f8 jit: Correct parameter type for generated expression evaluation functions. 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 08 December 2020, 02:39:32 UTC
b07490b jit: configure: Explicitly reference 'native' component. Until recently 'native' was implicitly included via 'orcjit', but a change included in LLVM 11 (not yet released) removed a number of such indirect component references. Reported-By: Fabien COELHO <coelho@cri.ensmp.fr> Reported-By: Andres Freund <andres@anarazel.de> Reported-By: Thomas Munro <thomas.munro@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20201201064949.mex6kvi2kygby3ni@alap3.anarazel.de Backpatch: 11-, where jit support was added 08 December 2020, 02:39:32 UTC
fa7a52f backpatch "jit: Add support for LLVM 12." 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 08 December 2020, 02:39:32 UTC
98f8cdd Fix more race conditions in the newly-added pg_rewind test. 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 07 December 2020, 12:55:22 UTC
77a94c3 Fix missed step in removal of useless RESULT RTEs in the planner. 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 05 December 2020, 21:16:13 UTC
back to top