aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix waits of REINDEX CONCURRENTLY for indexes with predicates or expressionsMichael Paquier2024-09-09
| | | | | | | | | | | | | | | | | | | | | As introduced by f9900df5f94, a REINDEX CONCURRENTLY job done for an index with predicates or expressions would set PROC_IN_SAFE_IC in its MyProc->statusFlags, causing it to be ignored by other concurrent operations. Such concurrent index rebuilds should never be ignored, as a predicate or an expression could call a user-defined function that accesses a different table than the table where the index is rebuilt. A test that uses injection points is added, backpatched down to 17. Michail has proposed a different test, but I have added something simpler with more coverage. Oversight in f9900df5f949. Author: Michail Nikolaev Discussion: https://postgr.es/m/CANtu0oj9A3kZVduFTG0vrmGnKB+DCHgEpzOp0qAyOgmks84j0w@mail.gmail.com Backpatch-through: 14
* Fix incorrect pg_stat_io output on 32-bit machines.Tom Lane2024-09-06
| | | | | | | | | | | | | | pg_stat_get_io() applied TimestampTzGetDatum twice to the stat_reset_timestamp value. On 64-bit builds that's harmless because TimestampTzGetDatum is a no-op, but on 32-bit builds it results in displaying garbage in the stats_reset column of the pg_stat_io view. Bug dates to commit a9c70b46d which introduced pg_stat_io, so back-patch to v16 where that came in. Bertrand Drouvot Discussion: https://postgr.es/m/Ztrd+XcPTz1zorkg@ip-10-97-1-34.eu-west-3.compute.internal
* Prevent mis-encoding of "trailing junk after numeric literal" errors.Tom Lane2024-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 2549f0661, we reject an identifier immediately following a numeric literal (without separating whitespace), because that risks ambiguity with hex/octal/binary integers. However, that patch used token patterns like "{integer}{ident_start}", which is problematic because {ident_start} matches only a single byte. If the first character after the integer is a multibyte character, this ends up with flex reporting an error message that includes a partial multibyte character. That can cause assorted bad-encoding problems downstream, both in the report to the client and in the postmaster log file. To fix, use {identifier} not {ident_start} in the "junk" token patterns, so that they will match complete multibyte characters. This seems generally better user experience quite aside from the encoding problem: for "123abc" the error message will now say that the error appeared at or near "123abc" instead of "123a". While at it, add some commentary about why these patterns exist and how they work. Report and patch by Karina Litskevich; review by Pavel Borisov. Back-patch to v15 where the problem came in. Discussion: https://postgr.es/m/CACiT8iZ_diop=0zJ7zuY3BXegJpkKK1Av-PU7xh0EDYHsa5+=g@mail.gmail.com
* Stabilize 039_end_of_wal test.Thomas Munro2024-08-31
| | | | | | | | | | | | | | | The first test was sensitive to the insert LSN after setting up the catalogs, which depended on environmental things like the locales on the OS and usernames. Switch to a new WAL file before the first test, as a simple way to put every computer into the same state. Back-patch to all supported releases. Reported-by: Anton Voloshin <a.voloshin@postgrespro.ru> Reported-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/b26aeac2-cb6d-4633-a7ea-945baae83dcf%40postgrespro.ru
* Clarify restrict_nonsystem_relation_kind description.Masahiko Sawada2024-08-30
| | | | | | | | | | | | This change improves the description of the restrict_nonsystem_relation_kind parameter in guc_table.c and the documentation for better clarity. Backpatch to 12, where this GUC parameter was introduced. Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/6a96f1af-22b4-4a80-8161-1f26606b9ee2%40eisentraut.org Backpatch-through: 12
* Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not.Tom Lane2024-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 2489d76c4 removed some logic from pullup_replace_vars() that avoided wrapping a PlaceHolderVar around a pulled-up subquery output expression if the expression could be proven to go to NULL anyway (because it contained Vars or PHVs of the pulled-up relation and did not contain non-strict constructs). But removing that logic turns out to cause performance regressions in some cases, because the extra PHV blocks subexpression folding, and will do so even if outer-join reduction later turns it into a no-op with no phnullingrels bits. This can for example prevent an expression from being matched to an index. The reason for always adding a PHV was to ensure we had someplace to put the varnullingrels marker bits of the Var being replaced. However, it turns out we can optimize in exactly the same cases that the previous code did, because we can instead attach the needed varnullingrels bits to the contained Var(s)/PHV(s). This is not a complete solution --- it would be even better if we could remove PHVs after reducing them to no-ops. It doesn't look practical to back-patch such an improvement, but this change seems safe and at least gets rid of the performance-regression cases. Per complaint from Nikhil Raj. Back-patch to v16 where the problem appeared. Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com
* Fix mis-deparsing of ORDER BY lists when there is a name conflict.Tom Lane2024-08-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If an ORDER BY item in SELECT is a bare identifier, the parser first seeks it as an output column name of the SELECT (for SQL92 compatibility). However, ruleutils.c is expecting the SQL99 interpretation where such a name is an input column name. So it's possible to produce an incorrect display of a view in the (admittedly pretty ill-advised) case where some other column is renamed in the SELECT output list to match an ORDER BY column. This can be fixed by table-qualifying such names in the dumped view text. To avoid cluttering less-ill-advised queries, we'd like to do so only when there's an actual name conflict. That requires passing the current get_query_def call's resultDesc parameter down to get_variable, so that it can determine what the output column names are. In hopes of reducing rather than increasing notational clutter in ruleutils.c, I moved that value into the deparse_context struct and removed it from the parameter lists of get_query_def's other subroutines. I made a few other cosmetic changes while at it: * Likewise move the colNamesVisible parameter into deparse_context. * Rename deparse_context's windowTList field to targetList, since it's no longer used only in connection with WINDOW clauses. * Replace the special_exprkind field with a bool inGroupBy, since that was all it was being used for, and the apparent flexibility of storing a ParseExprKind proved to be illusory. (We need a separate varInOrderBy field to make this patch work.) * Remove useless save/restore logic in get_select_query_def. In principle, this bug is quite old. However, it seems unreachable before 1b4d280ea, because before that the presence of "new" and "old" entries in a view's rangetable caused us to always table-qualify every Var reference in dumped views. Hence, back-patch to v16 where that came in. Per bug #18589 from Quynh Tran. Discussion: https://postgr.es/m/18589-70091cb81db1a3f1@postgresql.org
* Disallow USING clause when altering type of generated columnPeter Eisentraut2024-08-29
| | | | | | | | | | | | | This does not make sense. It would write the output of the USING clause into the converted column, which would violate the generation expression. This adds a check to error out if this is specified. There was a test for this, but that test errored out for a different reason, so it was not effective. Reported-by: Jian He <jian.universality@gmail.com> Reviewed-by: Yugo NAGATA <nagata@sraoss.co.jp> Discussion: https://www.postgresql.org/message-id/flat/c7083982-69f4-4b14-8315-f9ddb20b9834%40eisentraut.org
* Don't advance origin during apply failure.Amit Kapila2024-08-21
| | | | | | | | | | | | | | We advance origin progress during abort on successful streaming and application of ROLLBACK in parallel streaming mode. But the origin shouldn't be advanced during an error or unsuccessful apply due to shutdown. Otherwise, it will result in a transaction loss as such a transaction won't be sent again by the server. Reported-by: Hou Zhijie Author: Hayato Kuroda and Shveta Malik Reviewed-by: Amit Kapila Backpatch-through: 16 Discussion: https://postgr.es/m/TYAPR01MB5692FAC23BE40C69DA8ED4AFF5B92@TYAPR01MB5692.jpnprd01.prod.outlook.com
* Avoid failure to open dropped detached partitionAlvaro Herrera2024-08-19
| | | | | | | | | | | | | | | | | | | | When a partition is detached and immediately dropped, a prepared statement could try to compute a new partition descriptor that includes it. This leads to this kind of error: ERROR: could not open relation with OID 457639 Avoid this by skipping the partition in expand_partitioned_rtentry if it doesn't exist. Noted by me while investigating bug #18559. Kuntal Gosh helped to identify the exact failure. Backpatch to 14, where DETACH CONCURRENTLY was introduced. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/202408122233.bo4adt3vh5bi@alvherre.pgsql
* Explain dropdb can't use syscache because of TOASTTomas Vondra2024-08-19
| | | | | | | | | | | | | Add a comment explaining dropdb() can't rely on syscache. The issue with flattened rows was fixed by commit 0f92b230f88b, but better to have a clear explanation why the systable scan is necessary. The other places doing in-place updates on pg_database have the same comment. Suggestion and patch by Yugo Nagata. Backpatch to 12, same as the fix. Author: Yugo Nagata Backpatch-through: 12 Discussion: https://postgr.es/m/CAJTYsWWNkCt+-UnMhg=BiCD3Mh8c2JdHLofPxsW3m2dkDFw8RA@mail.gmail.com
* Fix regression in TLS session ticket disablingDaniel Gustafsson2024-08-19
| | | | | | | | | | | | | | | | Commit 274bbced disabled session tickets for TLSv1.3 on top of the already disabled TLSv1.2 session tickets, but accidentally caused a regression where TLSv1.2 session tickets were incorrectly sent. Fix by unconditionally disabling TLSv1.2 session tickets and only disable TLSv1.3 tickets when the right version of OpenSSL is used. Backpatch to all supported branches. Reported-by: Cameron Vogt <cvogt@automaticcontrols.net> Reported-by: Fire Emerald <fire.github@gmail.com> Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Discussion: https://postgr.es/m/DM6PR16MB3145CF62857226F350C710D1AB852@DM6PR16MB3145.namprd16.prod.outlook.com Backpatch-through: v12
* Fix harmless LC_COLLATE[_MASK] confusion.Thomas Munro2024-08-19
| | | | | | | | Commit ca051d8b101 called newlocale(LC_COLLATE, ...) instead of newlocale(LC_COLLATE_MASK, ...), in code reached only on FreeBSD. They have the same value on that OS, explaining why it worked. Fix. Back-patch to 14, where ca051d8b101 landed.
* ci: Upgrade MacPorts version to 2.10.1.Thomas Munro2024-08-19
| | | | | | | | | | | MacPorts version 2.9.3 started failing in our ci_macports_packages.sh script, for reasons not fully determined, but plausibly linked to the release of 2.10.1. 2.10.1 seems to work, so let's switch to it. Back-patch to 15, where CI began. Reported-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/81f104e8-f0a9-43c0-85bd-2bbbf590a5b8%40eisentraut.org
* Fix DROP DATABASE for databases with many ACLsTomas Vondra2024-08-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit c66a7d75e652 modified DROP DATABASE so that if interrupted, the database is known to be in an invalid state and can only be dropped. This is done by setting a flag using an in-place update, so that it's not lost in case of rollback. For databases with many ACLs, this may however fail like this: ERROR: wrong tuple length This happens because with many ACLs, the pg_database.datacl attribute gets TOASTed. The dropdb() code reads the tuple from the syscache, which means it's detoasted. But the in-place update expects the tuple length to match the on-disk tuple. Fixed by reading the tuple from the catalog directly, not from syscache. Report and fix by Ayush Tiwari. Backpatch to 12. The DROP DATABASE fix was backpatched to 11, but 11 is EOL at this point. Reported-by: Ayush Tiwari Author: Ayush Tiwari Reviewed-by: Tomas Vondra Backpatch-through: 12 Discussion: https://postgr.es/m/CAJTYsWWNkCt+-UnMhg=BiCD3Mh8c2JdHLofPxsW3m2dkDFw8RA@mail.gmail.com
* Fix creation of partition descriptor during concurrent detach+dropAlvaro Herrera2024-08-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | If a partition undergoes DETACH CONCURRENTLY immediately followed by DROP, this could cause a problem for a concurrent transaction recomputing the partition descriptor when running a prepared statement, because it tries to dereference a pointer to a tuple that's not found in a catalog scan. The existing retry logic added in commit dbca3469ebf8 is sufficient to cope with the overall problem, provided we don't try to dereference a non-existant heap tuple. Arguably, the code in RelationBuildPartitionDesc() has been wrong all along, since no check was added in commit 898e5e3290a7 against receiving a NULL tuple from the catalog scan; that bug has only become user-visible with DETACH CONCURRENTLY which was added in branch 14. Therefore, even though there's no known mechanism to cause a crash because of this, backpatch the addition of such a check to all supported branches. In branches prior to 14, this would cause the code to fail with a "missing relpartbound for relation XYZ" error instead of crashing; that's okay, because there are no reports of such behavior anyway. Author: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/18559-b48286d2eacd9a4e@postgresql.org
* Suppress Coverity warnings about Asserts in get_name_for_var_field.Tom Lane2024-08-11
| | | | | | | | | Coverity thinks dpns->plan could be null at these points. That shouldn't really be possible, but it's easy enough to modify the Asserts so they'd not core-dump if it were true. These are new in b919a97a6. Back-patch to v13; the v12 version of the patch didn't have these Asserts.
* Allow adjusting session_authorization and role in parallel workers.Tom Lane2024-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us
* Fix "failed to find plan for subquery/CTE" errors in EXPLAIN.Tom Lane2024-08-09
| | | | | | | | | | | | | | | | | | | | | | To deparse a reference to a field of a RECORD-type output of a subquery, EXPLAIN normally digs down into the subquery's plan to try to discover exactly which anonymous RECORD type is meant. However, this can fail if the subquery has been optimized out of the plan altogether on the grounds that no rows could pass the WHERE quals, which has been possible at least since 3fc6e2d7f. There isn't anything remaining in the plan tree that would help us, so fall back to printing the field name as "fN" for the N'th column of the record. (This will actually be the right thing some of the time, since it matches the column names we assign to RowExprs.) In passing, fix a comment typo in create_projection_plan, which I noticed while experimenting with an alternative fix for this. Per bug #18576 from Vasya B. Back-patch to all supported branches. Richard Guo and Tom Lane Discussion: https://postgr.es/m/18576-9feac34e132fea9e@postgresql.org
* Refuse ATTACH of a table referenced by a foreign keyAlvaro Herrera2024-08-08
| | | | | | | | | | | | | | | | | | | | | Trying to attach a table as a partition which is already on the referenced side of a foreign key on the partitioned table that it is being attached to, leads to strange behavior: we try to clone the foreign key from the parent to the partition, but this new FK points to the partition itself, and the mix of pg_constraint rows and triggers doesn't behave well. Rather than trying to untangle the mess (which might be possible given sufficient time), I opted to forbid the ATTACH. This doesn't seem a problematic restriction, given that we already fail to create the foreign key if you do it the other way around, that is, having the partition first and the FK second. Backpatch to all supported branches. Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org
* Fix pg_rewind debug output to print the source timeline historyHeikki Linnakangas2024-08-08
| | | | | | | | | | | | getTimelineHistory() is called twice, to read the source and the target timeline history files. However, the loop to print the file with the --debug option used the wrong variable when dealing with the source. As a result, the source's history was always printed as empty. Spotted while debugging bug #18575, but this does not fix that bug, just the debugging output. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/092dd515-b7b4-4fd0-8407-ceca2f02f6ec@iki.fi
* Revert ECPG's use of pnstrdup()Peter Eisentraut2024-08-08
| | | | | | | | | | | | Commit 0b9466fce added a dependency on fe_memutils' pnstrdup() inside informix.c. This adds an exit() path in a library, which we don't want. (Unlike libpq, the ecpg libraries don't have an automated check for that, but it makes sense to keep them to a similar standard.) The ecpg code can already handle failure results from the *strdup() call by itself. Author: Jacob Champion <jacob.champion@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/CAOYmi+=pg=W5L1h=3MEP_EB24jaBu2FyATrLXqQHGe7cpuvwyg@mail.gmail.com
* Fix edge case in plpgsql's make_callstmt_target().Tom Lane2024-08-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the plancache entry for the CALL statement is already stale, it's possible for us to fetch an old procedure OID out of it, and then fail with "cache lookup failed for function NNN". In ordinary usage this never happens because make_callstmt_target is called just once immediately after building the plancache entry. It can be forced however by setting up an erroneous CALL (that causes make_callstmt_target itself to report an error), then dropping/recreating the target procedure, then repeating the erroneous CALL. To fix, use SPI_plan_get_cached_plan() to fetch the plancache's plan, rather than assuming we can use SPI_plan_get_plan_sources(). This shouldn't add any noticeable overhead in the normal case, and in the stale-plan case we'd have had to replan anyway a little further down. The other callers of SPI_plan_get_plan_sources() seem OK, because either they don't need up-to-date plans or they know that the query was just (re) planned. But add some commentary in hopes of not falling into this trap again. Per bug #18574 from Song Hongyu. Back-patch to v14 where this coding was introduced. (Older branches have comparable code, but it's run after any required replanning, so there's no issue.) Discussion: https://postgr.es/m/18574-2ce7ba3249221389@postgresql.org
* Make fallback MD5 implementation thread-safe on big-endian systemsHeikki Linnakangas2024-08-07
| | | | | | | | | | | | | Replace a static scratch buffer with a local variable, because a static buffer makes the function not thread-safe. This function is used in client-code in libpq, so it needs to be thread-safe. It was until commit b67b57a966, which replaced the implementation with the one from pgcrypto. Backpatch to v14, where we switched to the new implementation. Reviewed-by: Robert Haas, Michael Paquier Discussion: https://www.postgresql.org/message-id/dfa2015d-ad21-4802-a4cc-3850fc5fff3f@iki.fi
* Restrict accesses to non-system views and foreign tables during pg_dump.Masahiko Sawada2024-08-05
| | | | | | | | | | | | | | | | | | | | | | | | When pg_dump retrieves the list of database objects and performs the data dump, there was possibility that objects are replaced with others of the same name, such as views, and access them. This vulnerability could result in code execution with superuser privileges during the pg_dump process. This issue can arise when dumping data of sequences, foreign tables (only 13 or later), or tables registered with a WHERE clause in the extension configuration table. To address this, pg_dump now utilizes the newly introduced restrict_nonsystem_relation_kind GUC parameter to restrict the accesses to non-system views and foreign tables during the dump process. This new GUC parameter is added to back branches too, but these changes do not require cluster recreation. Back-patch to all supported branches. Reviewed-by: Noah Misch Security: CVE-2024-7348 Backpatch-through: 12
* Translation updatesPeter Eisentraut2024-08-05
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 2a4e0c192e2738ce2451e6d6970dcb2210d31800
* Update comment in portal.h.Etsuro Fujita2024-08-01
| | | | | | | | | | | We store tuples into the portal's tuple store for a PORTAL_ONE_MOD_WITH query as well. Back-patch to all supported branches. Reviewed by Andy Fan. Discussion: https://postgr.es/m/CAPmGK14HVYBZYZtHabjeCd-e31VT%3Dwx6rQNq8QfehywLcpZ2Hw%40mail.gmail.com
* Revert "Allow parallel workers to cope with a newly-created session user ID."Tom Lane2024-07-31
| | | | | | | | | | | | | | | | This reverts commit 849326e49a5dd56941eb8fb4699130c301bff303. Some buildfarm animals are failing with "cannot change "client_encoding" during a parallel operation". It looks like assign_client_encoding is unhappy at being asked to roll back a client_encoding setting after a parallel worker encounters a failure. There must be more to it though: why didn't I see this during local testing? In any case, it's clear that moving the RestoreGUCState() call is not as side-effect-free as I thought. Given that the bug f5f30c22e intended to fix has gone unreported for years, it's not something that's urgent to fix; I'm not willing to risk messing with it further with only days to our next release wrap.
* Allow parallel workers to cope with a newly-created session user ID.Tom Lane2024-07-31
| | | | | | | | | | | | | | | | | | | | | | | | Parallel workers failed after a sequence like BEGIN; CREATE USER foo; SET SESSION AUTHORIZATION foo; because check_session_authorization could not see the uncommitted pg_authid row for "foo". This is because we ran RestoreGUCState() in a separate transaction using an ordinary just-created snapshot. The same disease afflicts any other GUC that requires catalog lookups and isn't forgiving about the lookups failing. To fix, postpone RestoreGUCState() into the worker's main transaction after we've set up a snapshot duplicating the leader's. This affects check_transaction_isolation and check_transaction_deferrable, which think they should only run during transaction start. Make them act like check_transaction_read_only, which already knows it should silently accept the value when InitializingParallelWorker. Per bug #18545 from Andrey Rachitskiy. Back-patch to all supported branches, because this has been wrong for awhile. Discussion: https://postgr.es/m/18545-feba138862f19aaa@postgresql.org
* Relax check for return value from second call of pg_strnxfrm().Jeff Davis2024-07-30
| | | | | | | | | strxfrm() is not guaranteed to return the exact number of bytes needed to store the result; it may return a higher value. Discussion: https://postgr.es/m/32f85d88d1f64395abfe5a10dd97a62a4d3474ce.camel@j-davis.com Reviewed-by: Heikki Linnakangas Backpatch-through: 16
* Add accidentally omitted test to meson build fileMelanie Plageman2024-07-29
| | | | | | | | | 01e2b7f0fd02a44e introduced a test that vacuum correctly removes tuples older than OldestXmin. The same commit was backpatched on 14-16, but 16 is the only version with meson and the test was mistakenly left off of the recovery test meson build file. Add it now. Discussion: https://postgr.es/m/CAAKRu_bWmMjmqL%2BOZ2duEQ80u7cRvpsExLNZNjzk-pXX5skwMQ%40mail.gmail.com
* Use DELETE instead of UPDATE to speed up vacuum testMelanie Plageman2024-07-29
| | | | | | | | | | | | | | | 01e2b7f0fd02a44e introduced a test which generated dead tuples for vacuum with an UPDATE. The test only required enough dead TIDs for two rounds of index vacuuming. This can be accomplished with a DELETE instead of an UPDATE -- which generates about 50% less WAL and makes the test 20% faster in many cases. The test takes several seconds (more on slow buildfarm animals) because we need quite a few tuples to trigger two rounds of index vacuuming; so it is worth a follow-on commit to speed it up. Suggested-by: Masahiko Sawada Discussion: https://postgr.es/m/CAAKRu_bWmMjmqL%2BOZ2duEQ80u7cRvpsExLNZNjzk-pXX5skwMQ%40mail.gmail.com Backpatch-through: 14, the first version containing this test.
* Fix incorrect return value for pg_size_pretty(bigint)David Rowley2024-07-28
| | | | | | | | | | | | | | | | | pg_size_pretty(bigint) would return the value in bytes rather than PB for the smallest-most bigint value. This happened due to an incorrect assumption that the absolute value of -9223372036854775808 could be stored inside a signed 64-bit type. Here we fix that by instead storing that value in an unsigned 64-bit type. This bug does exist in versions prior to 15 but the code there is sufficiently different and the bug seems sufficiently non-critical that it does not seem worth risking backpatching further. Author: Joseph Koshakow <koshy44@gmail.com> Discussion: https://postgr.es/m/CAAvxfHdTsMZPWEHUrZ=h3cky9Ccc3Mtx2whUHygY+ABP-mCmUw@mail.gmail.com Backpatch-through: 15
* libpq: Use strerror_r instead of strerrorPeter Eisentraut2024-07-28
| | | | | | | | | | | | Commit 453c4687377 introduced a use of strerror() into libpq, but that is not thread-safe. Fix by using strerror_r() instead. In passing, update some of the code comments added by 453c4687377, as we have learned more about the reason for the change in OpenSSL that started this. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca
* Fix building with MSVC for TLS session disablingDaniel Gustafsson2024-07-26
| | | | | | | Commit 274bbced85 omitted the required changes for the MSVC build system in v16 through v12. Per buildfarm animal hamerkop. Discussion: https://postgr.es/m/7919238F-723C-4113-9742-EBCE7A76A6B4@yesql.se
* Fix macro placement in pg_config.h.inDaniel Gustafsson2024-07-26
| | | | | | | | | | | Commit 274bbced85383e831dde accidentally placed the pg_config.h.in for SSL_CTX_set_num_tickets on the wrong line wrt where autoheader places it. Fix by re-arranging and backpatch to the same level as the original commit. Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru> Discussion: https://postgr.es/m/48cebe8c3eaf308bae253b1dbf4e4a75@postgrespro.ru Backpatch-through: v12
* Disable all TLS session ticketsDaniel Gustafsson2024-07-26
| | | | | | | | | | | | | | OpenSSL supports two types of session tickets for TLSv1.3, stateless and stateful. The option we've used only turns off stateless tickets leaving stateful tickets active. Use the new API introduced in 1.1.1 to disable all types of tickets. Backpatch to all supported versions. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20240617173803.6alnafnxpiqvlh3g@awork3.anarazel.de Backpatch-through: v12
* ci: Pin MacPorts version to 2.9.3.Thomas Munro2024-07-25
| | | | | | | | | | | | | | | | | | | Commit d01ce180 invented a new way to find the latest MacPorts version. By bad luck, a new beta release has just been published, and it seems to lack some packages we need. Go back to searching for this specific version for now. We still search with a pattern so that we can find the package for the running version of macOS, but for now we always look for 2.9.3. The code to do that had been anticipated already in a commented out line, I just didn't expect to have to use it so soon... Also include the whole MacPorts installation script in the cache key, so that changes to the script cause a fresh installation. This should make it a bit easier to reason about the effect of changes on cached state in github accounts using CI, when we make adjustments. Back-patch to 15, like d01ce180. Discussion: https://postgr.es/m/CA%2BhUKGLqJdv6RcwyZ_0H7khxtLTNJyuK%2BvDFzv3uwYbn8hKH6A%40mail.gmail.com
* ci: Upgrade macOS version from 13 to 14.Thomas Munro2024-07-25
| | | | | | | | | | | | | | | | | | | | | | | 1. Previously we were using ghcr.io/cirruslabs/macos-XXX-base:latest images, but Cirrus has started ignoring that and using a particular image, currently ghcr.io/cirruslabs/macos-runner:sonoma, for github accounts using free CI resources (as opposed to dedicated runner machines, as cfbot uses). Let's just ask for that image anyway, to stay in sync. 2. Instead of hard-coding a MacPorts installation URL, deduce it from the running macOS version and the available releases. This removes the need to keep the ci_macports_packages.sh in sync with .cirrus.task.yml, and to advance the MacPorts version from time to time. 3. Change the cache key we use to cache the whole macports installation across builds to include the OS major version, to trigger a fresh installation when appropriate. Back-patch to 15 where CI began. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA%2BhUKGLqJdv6RcwyZ_0H7khxtLTNJyuK%2BvDFzv3uwYbn8hKH6A%40mail.gmail.com
* Reset relhassubclass upon attaching table as a partitionAlvaro Herrera2024-07-24
| | | | | | | | | | | | | | | | | We don't allow inheritance parents as partitions, and have checks to prevent this; but if a table _was_ in the past an inheritance parents and all their children are removed, the pg_class.relhassubclass flag may remain set, which confuses the partition pruning code (most obviously, it results in an assertion failure; in production builds it may be worse.) Fix by resetting relhassubclass on attach. Backpatch to all supported versions. Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18550-d5e047e9a897a889@postgresql.org
* Detect integer overflow in array_set_slice().Nathan Bossart2024-07-23
| | | | | | | | | | | | | | | | | | | | | When provided an empty initial array, array_set_slice() fails to check for overflow when computing the new array's dimensions. While such overflows are ordinarily caught by ArrayGetNItems(), commands with the following form are accepted: INSERT INTO t (i[-2147483648:2147483647]) VALUES ('{}'); To fix, perform the hazardous computations using overflow-detecting arithmetic routines. As with commit 18b585155a, the added test cases generate errors that include a platform-dependent value, so we again use psql's VERBOSITY parameter to suppress printing the message text. Reported-by: Alexander Lakhin Author: Joseph Koshakow Reviewed-by: Jian He Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com Backpatch-through: 12
* Doc: improve description of plpgsql's FETCH and MOVE commands.Tom Lane2024-07-22
| | | | | | | | | | | | | | | | | | | | | We were not being clear about which variants of the "direction" clause are permitted in MOVE. Also, the text seemed to be written with only the FETCH/MOVE NEXT case in mind, so it didn't apply very well to other variants. Also, document that "MOVE count IN cursor" only works if count is a constant. This is not the whole truth, because some other cases such as a parenthesized expression will also work, but we want to push people to use "MOVE FORWARD count" instead. The constant case is enough to cover what we allow in plain SQL, and that seems sufficient to claim support for. Update a comment in pl_gram.y claiming that we don't document that point. Per gripe from Philipp Salvisberg. Discussion: https://postgr.es/m/172155553388.702.7932496598218792085@wrigleys.postgresql.org
* Correctly check updatability of columns targeted by INSERT...DEFAULT.Tom Lane2024-07-20
| | | | | | | | | | | | | | | If a view has some updatable and some non-updatable columns, we failed to verify updatability of any columns for which an INSERT or UPDATE on the view explicitly specifies a DEFAULT item (unless the view has a declared default for that column, which is rare anyway, and one would almost certainly not write one for a non-updatable column). This would lead to an unexpected "attribute number N not found in view targetlist" error rather than the intended error. Per bug #18546 from Alexander Lakhin. This bug is old, so back-patch to all supported branches. Discussion: https://postgr.es/m/18546-84a292e759a9361d@postgresql.org
* Add overflow checks to money type.Nathan Bossart2024-07-19
| | | | | | | | | | | | | | | None of the arithmetic functions for the the money type handle overflow. This commit introduces several helper functions with overflow checking and makes use of them in the money type's arithmetic functions. Fixes bug #18240. Reported-by: Alexander Lakhin Author: Joseph Koshakow Discussion: https://postgr.es/m/18240-c5da758d7dc1ecf0%40postgresql.org Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com Backpatch-through: 12
* Test that vacuum removes tuples older than OldestXminMelanie Plageman2024-07-19
| | | | | | | | | | | | | | | | | | | | | If vacuum fails to prune a tuple killed before OldestXmin, it will later find that tuple dead in lazy_scan_prune() and loop infinitely. Add a test reproducing this scenario to the recovery suite which creates a table on a primary, updates the table to generate dead tuples for vacuum, and then, during the vacuum, uses a replica to force GlobalVisState->maybe_needed on the primary to move backwards and precede the value of OldestXmin set at the beginning of vacuuming the table. This commit is separate from the fix in case there are test stability issues. Discussion of the bug: https://postgr.es/m/CAAKRu_Y_NJzF4-8gzTTeaOuUL3CcGoXPjXcAHbTTygT8AyVqag%40mail.gmail.com Discussion of the test: https://postgr.es/m/CAAKRu_apNU2MPBK96V%2BbXjTq0RiZ-%3DA4ZTaysakpx9jxbq1dbQ%40mail.gmail.com Author: Melanie Plageman Reviewed-by: Peter Geoghegan
* Ensure vacuum removes all visibly dead tuples older than OldestXminMelanie Plageman2024-07-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | If vacuum fails to remove a tuple with xmax older than VacuumCutoffs->OldestXmin and younger than GlobalVisState->maybe_needed, it will loop infinitely in lazy_scan_prune(), which compares tuples' visibility information to OldestXmin. Starting in version 14, which uses GlobalVisState for visibility testing during pruning, it is possible for GlobalVisState->maybe_needed to precede OldestXmin if maybe_needed is forced to go backward while vacuum is running. This can happen if a disconnected standby with a running transaction older than VacuumCutoffs->OldestXmin reconnects to the primary after vacuum initially calculates GlobalVisState and OldestXmin. Fix this by having vacuum always remove tuples older than OldestXmin during pruning. This is okay because the standby won't replay the tuple removal until the tuple is removable. Thus, the worst that can happen is a recovery conflict. Fixes BUG# 17257 Back-patched in versions 14-17 Author: Melanie Plageman Reviewed-by: Noah Misch, Peter Geoghegan, Robert Haas, Andres Freund, and Heikki Linnakangas Discussion: https://postgr.es/m/CAAKRu_Y_NJzF4-8gzTTeaOuUL3CcGoXPjXcAHbTTygT8AyVqag%40mail.gmail.com
* Propagate query IDs of utility statements in functionsMichael Paquier2024-07-19
| | | | | | | | | | | | | | | | | | | For utility statements defined within a function, the query tree is copied to a PlannedStmt as utility commands do not require planning. However, the query ID was missing from the information passed down. This leads to plugins relying on the query ID like pg_stat_statements to not be able to track utility statements within function calls. Tests are added to check this behavior, depending on pg_stat_statements.track. This is an old bug. Now, query IDs for utilities are compiled using their parsed trees rather than the query string since v16 (3db72ebcbe20), leading to less bloat with utilities, so backpatch down only to this version. Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/CAO6_XqrGp-uwBqi3vBPLuRULKkddjC7R5QZCgsFren=8E+m2Sg@mail.gmail.com Backpatch-through: 16
* Avoid error in recovery test if history file is not yet presentAndrew Dunstan2024-07-17
| | | | | | | | | Error was detected when testing use of libpq sessions instead of psql for polling queries. Discussion: https://postgr.es/m/e86b6d2d-20d8-4ac9-9a98-165fff7db886@dunslane.net Backpatch to all live branches
* Fix bad indentation introduced in 43cd30bcd1cAndres Freund2024-07-15
| | | | | | | | Oops. Reported-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/ZpVZB9rH5tHllO75@nathan Backpatch: 12-, like 43cd30bcd1c
* Fix type confusion in guc_var_compare()Andres Freund2024-07-15
| | | | | | | | | | | | | | | | | Before this change guc_var_compare() cast the input arguments to const struct config_generic *. That's not quite right however, as the input on one side is often just a char * on one side. Instead just use char *, the first field in config_generic. This fixes a -Warray-bounds warning with some versions of gcc. While the warning is only known to be triggered for <= 15, the issue the warning points out seems real, so apply the fix everywhere. Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Reported-by: Erik Rijkers <er@xs4all.nl> Suggested-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/a74a1a0d-0fd2-3649-5224-4f754e8f91aa%40xs4all.nl