aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Use C library functions instead of Abs() for int64Peter Eisentraut2022-10-10
| | | | | | | | | | Instead of Abs() for int64, use the C standard functions labs() or llabs() as appropriate. Define a small wrapper around them that matches our definition of int64. (labs() is C90, llabs() is C99.) Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
* pgstat: Prevent stats reset from corrupting slotname by removing slotnameAndres Freund2022-10-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously PgStat_StatReplSlotEntry contained the slotname, which was mainly used when writing out the stats during shutdown, to identify the slot in the serialized data (at runtime the index in ReplicationSlotCtl->replication_slots is used, but that can change during a restart). Unfortunately the slotname was overwritten when the slot's stats were reset. That turned out to only cause "real" problems if the slot was active during the reset, triggering an assertion failure at the next pgstat_report_replslot(). In other paths the stats were re-initialized during pgstat_acquire_replslot(). Fix this by removing slotname from PgStat_StatReplSlotEntry. Instead we can get the slot's name from the slot itself. Besides fixing a bug, this also is architecturally cleaner (a name is not really statistics). This is safe because stats, for a slot removed while shut down, will not be restored at startup. In 15 the slotname is not removed, but renamed, to avoid changing the stats format. In master, bump PGSTAT_FILE_FORMAT_ID. This commit does not contain a test for the fix. I think this can only be tested by a tap test starting pg_recvlogical in the background and checking pg_recvlogical's output. That type of test is notoriously hard to be reliable, so committing it shortly before the release is wrapped seems like a bad idea. Reported-by: Jaime Casanova <jcasanov@systemguards.com.ec> Author: Andres Freund <andres@anarazel.de> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/YxfagaTXUNa9ggLb@ahch-to Backpatch: 15-, where the bug was introduced in 5891c7a8ed8f
* Use fabsf() instead of Abs() or fabs() where appropriatePeter Eisentraut2022-10-08
| | | | | | | | This function is new in C99. Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
* Fix self-referencing foreign keys with partitioned tablesAlvaro Herrera2022-10-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | There are a number of bugs in this area. Two of them are fixed here, namely: 1. get_relation_idx_constraint_oid does not restrict the type of constraint that's returned, so with sufficient bad luck it can return the OID of a foreign key constraint. This has the effect that a primary key in a partition can end up as a child of a foreign key, which makes no sense (it needs to be the child of the equivalent primary key.) Change the API contract so that only index-backed constraints are returned, mimicking get_constraint_index(). 2. Both CloneFkReferenced and CloneFkReferencing clone a self-referencing foreign key, so the partition ends up with a duplicate foreign key. Change the former function to ignore such constraints. Add some tests to verify that things are better now. (However, these new tests show some additional misbehavior that will be fixed later -- namely that there's a constraint marked NOT VALID.) Backpatch to 12, where these constraints are possible at all. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20220603154232.1715b14c@karst
* Remove unnecessary uses of Abs()Peter Eisentraut2022-10-07
| | | | | | | | Use C standard abs() or fabs() instead. Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
* Improve our ability to detect bogus pointers passed to pfree et al.Tom Lane2022-10-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit c6e0fe1f2 was a shade too trusting that any pointer passed to pfree, repalloc, etc will point at a valid chunk. Notably, passing a pointer that was actually obtained from malloc tended to result in obscure assertion failures, if not worse. (On FreeBSD I've seen such mistakes take down the entire cluster, seemingly as a result of clobbering shared memory.) To improve matters, extend the mcxt_methods[] array so that it has entries for every possible MemoryContextMethodID bit-pattern, with the currently unassigned ID codes pointing to error-reporting functions. Then, fiddle with the ID assignments so that patterns likely to be associated with bad pointers aren't valid ID codes. In particular, we should avoid assigning bit patterns 000 (zeroed memory) and 111 (wipe_mem'd memory). It turns out that on glibc (Linux), malloc uses chunk headers that have flag bits in the same place we keep MemoryContextMethodID, and that the bit patterns 000, 001, 010 are the only ones we'll see as long as the backend isn't threaded. So we can have very robust detection of pfree'ing a malloc-assigned block on that platform, at least so long as we can refrain from using up those ID codes. On other platforms, we don't have such a good guarantee, but keeping 000 reserved will be enough to catch many such cases. While here, make GetMemoryChunkMethodID() local to mcxt.c, as there seems no need for it to be exposed even in memutils_internal.h. Patch by me, with suggestions from Andres Freund and David Rowley. Discussion: https://postgr.es/m/2910981.1665080361@sss.pgh.pa.us
* meson: Add support for building with precompiled headersAndres Freund2022-10-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This substantially speeds up building for windows, due to the vast amount of headers included via windows.h. A cross build from linux targetting mingw goes from 994.11user 136.43system 0:31.58elapsed 3579%CPU to 422.41user 89.05system 0:14.35elapsed 3562%CPU The wins on windows are similar-ish (but I don't have a system at hand just now for actual numbers). Targetting other operating systems the wins are far smaller (tested linux, macOS, FreeBSD). For now precompiled headers are disabled by default, it's not clear how well they work on all platforms. E.g. on FreeBSD gcc doesn't seem to have working support, but clang does. When doing a full build precompiled headers are only beneficial for targets with multiple .c files, as meson builds a separate precompiled header for each target (so that different compilation options take effect). This commit therefore only changes target with at least two .c files to use precompiled headers. Because this commit adds b_pch=false to the default_options new build directories will have precompiled headers disabled by default, however existing build directories will continue use the default value of b_pch, which is true. Note that using precompiled headers with ccache requires setting CCACHE_SLOPPINESS=pch_defines,time_macros to get hits. Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/CA+hUKG+50eOUbN++ocDc0Qnp9Pvmou23DSXu=ZA6fepOcftKqA@mail.gmail.com Discussion: https://postgr.es/m/c5736f70-bb6d-8d25-e35c-e3d886e4e905@enterprisedb.com Discussion: https://postgr.es/m/20190826054000.GE7005%40paquier.xyz
* Create subscription stats entry at CREATE SUBSCRIPTION timeAndres Freund2022-10-06
| | | | | | | | | | | | | | | | Previously, the subscription stats entry was created when the first stats, i.e., an error on apply worker or tablesync worker, were reported. Therefore, the stats_reset field was not updated by pg_stat_reset_subscription_stats() if the stats entry was not populated yet, which was different behavior than other statistics. This change creates the subscription stats entry and initializes it at CREATE SUBSCRIPTION time. Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Author: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAAKRu_Zqd-e5imT_3-ZiQv1cfsWuy16OJTiUaCvqpq4V7GVdSg@mail.gmail.com
* meson: Fix two commentsAndres Freund2022-10-06
| | | | | Author: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/CAEG8a3KxObc9g8NTzx1kX0Auf=J7FNiubYZXSK6G5wv5ShmP6A@mail.gmail.com
* Remove MemoryContextContains().Tom Lane2022-10-06
| | | | | | | | | | | | | | | | | MemoryContextContains is no longer reliable in the wake of c6e0fe1f2, because there's no longer very much redundancy in chunk headers. (It wasn't *completely* reliable even before that, as there was a chance of a false positive if you passed it something that didn't point to an mcxt chunk at all. But it was generally good enough.) Hence, remove it. There is no remaining core code that requires it. Extensions that have been using it might be able to substitute a test like "GetMemoryChunkContext(ptr) == context", recognizing that this explicitly requires that the pointer point to some chunk. Tom Lane and David Rowley Discussion: https://postgr.es/m/1913788.1664898906@sss.pgh.pa.us
* Remove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.Tom Lane2022-10-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | MemoryContextContains is no longer reliable in the wake of c6e0fe1f2, so we need to get rid of these uses. It appears that there's no really good reason to force the result of an aggregate's finalfn or serialfn to be allocated in the per-tuple context. The only other plausible case is that the result points to or into the aggregate's transition value, and that's fine because it will last as long as we need it to. (This conclusion depends on the assumption that finalfns are not allowed to scribble on the transition value, but we've long required that.) So we can just drop the MemoryContextContains plus datumCopy business, although we do need to take care to not return a read-write pointer when the transition value is an expanded datum. Likewise, we don't really need to force the result of a window function to be in the output context. In this case, the plausible alternative is that it's pointing into the temporary tuple slot used by WinGetFuncArgInPartition or WinGetFuncArgInFrame (since those functions could return such a pointer, which might become the window function's result). That will hold still for long enough, unless there is another window function using the same WindowObject. I'm content to always perform a datumCopy when there's more than one such function. On net, these changes should provide small speed improvements as well as removing problematic code. Tom Lane and David Rowley Discussion: https://postgr.es/m/1913788.1664898906@sss.pgh.pa.us
* Take care to de-duplicate entries in standby.c's table of locks.Tom Lane2022-10-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The RecoveryLockLists data structure, which tracks all exclusive locks that the startup process is holding on behalf of transactions being replayed, did not have any provision for avoiding duplicate entries for the same lock. Maybe that was okay when the code was first written. However, modern practice is for checkpoints to write fresh lists of all active exclusive locks into the WAL. Thus, an exclusive lock that survives across multiple checkpoints causes bloat in standbys' startup processes. If there are a lot of such locks this can look like a memory leak, and it's even possible to drive the startup process into a palloc failure from an over-length List. To fix, use a hash table instead of simple lists to track the locks being held. Allowing for dynahash overhead, this requires a little more space per lock than the old way (although it's the same size as what we were allocating prior to c6e0fe1f2). It's probably a shade slower too. However, testing indicates that the penalty is negligible on ordinary workloads, so let's make this change to improve robustness in extreme cases. Patch by me, per report from Dmitriy Kuzmin. No back-patch (for now anyway), since it seems that a significant improvement would only occur in corner cases. Discussion: https://postgr.es/m/CAHLDt=_ts0A7Agn=hCpUh+RCFkxd+G6uuT=kcTfqFtGur0dp=A@mail.gmail.com
* Introduce t_isalnum() to replace t_isalpha() || t_isdigit() tests.Tom Lane2022-10-06
| | | | | | | | | | | | ts_locale.c omitted support for "isalnum" tests, perhaps on the grounds that there were initially no use-cases for that. However, both ltree and pg_trgm need such tests, and we do also have one use-case now in the core backend. The workaround of testing isalpha and isdigit separately seems quite inefficient, especially when dealing with multibyte characters; so let's fill in the missing support. Discussion: https://postgr.es/m/2548310.1664999615@sss.pgh.pa.us
* Fix comment in xlogprefetcher.cMichael Paquier2022-10-06
| | | | | Author: Sho Kato Discussion: https://postgr.es/m/TYCPR01MB684954052EC534A3261B29249F5C9@TYCPR01MB6849.jpnprd01.prod.outlook.com
* Add optional parameter to PG_TRY() macrosDavid Rowley2022-10-06
| | | | | | | | | | | | | | | | | | | This optional parameter can be specified in cases where there are nested PG_TRY() statements within a function in order to stop the compiler from issuing warnings about shadowed local variables when compiling with -Wshadow. The optional parameter is used as a suffix on the variable names declared within the PG_TRY(), PG_CATCH(), PG_FINALLY() and PG_END_TRY() macros. The parameter, if specified, must be the same in each component macro of the given PG_TRY() block. This also adjusts the single case where we have nested PG_TRY() statements to add a parameter to the inner-most PG_TRY(). This reduces the number of compiler warnings when compiling with -Wshadow=compatible-local from 5 down to 1. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvqWGMdB_pATeUqE=JCtNqNxObPOJ00jFEa2_sZ20j_Wvg@mail.gmail.com
* meson: Add windows resource filesAndres Freund2022-10-05
| | | | | | | | | | | | | The generated resource files aren't exactly the same ones as the old buildsystems generate. Previously "InternalName" and "OriginalFileName" were mostly wrong / not set (despite being required), but that was hard to fix in at least the make build. Additionally, the meson build falls back to a "auto-generated" description when not set, and doesn't set it in a few cases - unlikely that anybody looks at these descriptions in detail. Author: Andres Freund <andres@anarazel.de> Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
* Rename shadowed local variablesDavid Rowley2022-10-05
| | | | | | | | | | | | In a similar effort to f01592f91, here we mostly rename shadowed local variables to remove the warnings produced when compiling with -Wshadow=compatible-local. This fixes 63 warnings and leaves just 5. Author: Justin Pryzby, David Rowley Reviewed-by: Justin Pryzby Discussion https://postgr.es/m/20220817145434.GC26426%40telsasoft.com
* Fix comment in guc_tables.cMichael Paquier2022-10-04
| | | | | | | s/ERROR_HANDLING/ERROR_HANDLING_OPTIONS/. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+PtDj3CV+f0pVisc0XYMi2LHGBpQxQWtF0FjiSVN_nV17Q@mail.gmail.com
* Cleanup useless assignments and checksMichael Paquier2022-10-04
| | | | | | | | | | | | | | This cleans up a couple of areas: - Remove XLogSegNo calculation for the last WAL segment in backup in xlog.c (7d70809 has moved this logic entirely to xlogbackup.c when building the contents of the backup history file). - Remove check on log_min_duration in analyze.c, as it is already true where this code path is reached. - Simplify call to find_option() in guc.c. Author: Ranier Vilela Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAEudQArCDQQiPiFR16=yu9k5s2tp4tgEe1U1ZbkW4ofx81AWWQ@mail.gmail.com
* meson: llvm: Use llvm-config's --cxxflags when building llvmjitAndres Freund2022-10-03
| | | | | | | Otherwise we don't use LLVM's flags when building llvmjit_wrap.cpp and llvmjit_inline.cpp. That can cause compile time failures if the C++ compiler doesn't default to a new enough C++ standards version and link time failures due to ABI influencing flags like -fno-rtti.
* Revert "Optimize order of GROUP BY keys".Tom Lane2022-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit db0d67db2401eb6238ccc04c6407a4fd4f985832 and several follow-on fixes. The idea of making a cost-based choice of the order of the sorting columns is not fundamentally unsound, but it requires cost information and data statistics that we don't really have. For example, relying on procost to distinguish the relative costs of different sort comparators is pretty pointless so long as most such comparator functions are labeled with cost 1.0. Moreover, estimating the number of comparisons done by Quicksort requires more than just an estimate of the number of distinct values in the input: you also need some idea of the sizes of the larger groups, if you want an estimate that's good to better than a factor of three or so. That's data that's often unknown or not very reliable. Worse, to arrive at estimates of the number of calls made to the lower-order-column comparison functions, the code needs to make estimates of the numbers of distinct values of multiple columns, which are necessarily even less trustworthy than per-column stats. Even if all the inputs are perfectly reliable, the cost algorithm as-implemented cannot offer useful information about how to order sorting columns beyond the point at which the average group size is estimated to drop to 1. Close inspection of the code added by db0d67db2 shows that there are also multiple small bugs. These could have been fixed, but there's not much point if we don't trust the estimates to be accurate in-principle. Finally, the changes in cost_sort's behavior made for very large changes (often a factor of 2 or so) in the cost estimates for all sorting operations, not only those for multi-column GROUP BY. That naturally changes plan choices in many situations, and there's precious little evidence to show that the changes are for the better. Given the above doubts about whether the new estimates are really trustworthy, it's hard to summon much confidence that these changes are better on the average. Since we're hard up against the release deadline for v15, let's revert these changes for now. We can always try again later. Note: in v15, I left T_PathKeyInfo in place in nodes.h even though it's unreferenced. Removing it would be an ABI break, and it seems a bit late in the release cycle for that. Discussion: https://postgr.es/m/TYAPR01MB586665EB5FB2C3807E893941F5579@TYAPR01MB5866.jpnprd01.prod.outlook.com
* Fix tiny memory leaksPeter Eisentraut2022-10-01
| | | | | | | | | | | | | Both check_application_name() and check_cluster_name() use pg_clean_ascii() but didn't release the memory. Depending on when the GUC is set, this might be cleaned up at some later time or it would leak postmaster memory once. In any case, it seems better not to have to rely on such analysis and make the code locally robust. Also, this makes Valgrind happier. Author: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Jacob Champion <jchampion@timescale.com> Discussion: https://www.postgresql.org/message-id/CAD21AoBmFNy9MPfA0UUbMubQqH3AaK5U3mrv6pSeWrwCk3LJ8g@mail.gmail.com
* doc: Fix some grammar and typosMichael Paquier2022-10-01
| | | | | | | | This fixes some areas related to logical replication and custom RMGRs. Author: Ekaterina Kiryanova Discussion: https://postgr.es/m/fa4773f1-1396-384a-bcd7-85b5e013f399@postgrespro.ru Backpatch-through: 15
* Avoid improbable PANIC during heap_update, redux.Tom Lane2022-09-30
| | | | | | | | | | | | | | | | | | | | Commit 34f581c39 intended to ensure that RelationGetBufferForTuple would acquire a visibility-map page pin in case the otherBuffer's all-visible bit had become set since we last had lock on that page. But I missed a case: when we're extending the relation, VM concerns were dealt with only in the relatively-less-likely case that we fail to conditionally lock the otherBuffer. I think I'd believed that we couldn't need to worry about it if the conditional lock succeeds, which is true for the target buffer; but the otherBuffer was unlocked for awhile so its bit might be set anyway. So we need to do the GetVisibilityMapPins dance, and then also recheck the page's free space, in both cases. Per report from Jaime Casanova. Back-patch to v12 as the previous patch was (although there's still no evidence that the bug is reachable pre-v14). Discussion: https://postgr.es/m/E1lWLjP-00006Y-Ml@gemulon.postgresql.org
* Fix tab-completion after commit 790bf615ddbaAlvaro Herrera2022-09-30
| | | | | | | | | | | | | | | | | I (Álvaro) broke tab-completion for GRANT .. ALL TABLES IN SCHEMA while removing ALL from the publication syntax for schemas in the aforementioned commit. I also missed to update a bunch of tab-completion rules for ALTER/CREATE PUBLICATION that match each individual piece of ALL TABLES IN SCHEMA. Repair those bugs. While fixing up that commit, update a couple of outdated comments related to the same change. Backpatch to 15. Author: Shi yu <shiy.fnst@fujitsu.com> Reviewed-by: Peter Smith <smithpb2250@gmail.com> Discussion: https://postgr.es/m/OSZPR01MB6310FCE8609185A56344EED2FD559@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Remove useless argument from UnpinBuffer()Michael Paquier2022-09-30
| | | | | | | | | | | The last caller of UnpinBuffer() that did not want to adjust CurrentResourceOwner was removed in 2d115e4, and nothing has been introduced in bufmgr.c to do the same thing since. This simplifies 10 code paths. Author: Aleksander Alekseev Reviewed-by: Nathan Bossart, Zhang Mingli, Bharath Rupireddy Discussion: https://postgr.es/m/CAJ7c6TOmmFpb6ohurLhTC7hKNJWGzdwf8s4EAtAZxD48g-e6Jw@mail.gmail.com
* Improve wording of log messages triggered by max_slot_wal_keep_size.Tom Lane2022-09-29
| | | | | | | | | | | | | | | | | | | The one about "terminating process to release replication slot" told you nothing about why that was happening. The one about "invalidating slot because its restart_lsn exceeds max_slot_wal_keep_size" told you what was happening, but violated our message style guideline about keeping the primary message short. Add DETAIL/HINT lines to carry the appropriate detail and make the two cases more uniform. While here, fix bogus test logic in 019_replslot_limit.pl: if it timed out without seeing the expected log message, no test failure would be reported. This is flat broken since commit 549ec201d removed the test counts; even before that it was horribly bad style, since you'd only get told that not all tests had been run. Kyotaro Horiguchi, reviewed by Bertrand Drouvot; test fixes by me Discussion: https://postgr.es/m/20211214.130456.2233153190058148084.horikyota.ntt@gmail.com
* Use actual backend IDs in pg_stat_get_backend_idset() and friends.Tom Lane2022-09-29
| | | | | | | | | | | | | | | | | | | | | | | Up to now, the ID values returned by pg_stat_get_backend_idset() and used by pg_stat_get_backend_activity() and allied functions were just indexes into a local array of sessions seen by the last stats refresh. This is problematic for a few reasons. The "ID" of a session can vary over its existence, which is surprising. Also, while these numbers often match the "backend ID" used for purposes like temp schema assignment, that isn't reliably true. We can fairly cheaply switch things around to make these numbers actually be the sessions' backend IDs. The added test case illustrates that with this definition, the temp schema used by a given session can be obtained given its PID. While here, delete some dead code that guarded against getting a NULL return from pgstat_fetch_stat_local_beentry(). That can't happen as long as the caller is careful to pass an in-range array index, as all the callers are. (This code may not have been dead when written, but it surely is now.) Nathan Bossart Discussion: https://postgr.es/m/20220815205811.GA250990@nathanxps13
* Update comment in ExecInsert() regarding batch insertion.Etsuro Fujita2022-09-29
| | | | | | | | | | | | Remove the stale text that is a leftover from an earlier version of the patch to add support for batch insertion, and adjust the wording in the remaining text. Back-patch to v14 where batch insertion came in. Review and wording adjustment by Tom Lane. Discussion: https://postgr.es/m/CAPmGK14goatHPHQv2Aeu_UTKqZ%2BBO%2BP%2Bzd3HKv5D%2BdyyfWKDSw%40mail.gmail.com
* Introduce SYSTEM_USERMichael Paquier2022-09-29
| | | | | | | | | | | | | | | | | | | | | | | | SYSTEM_USER is a reserved keyword of the SQL specification that, roughly described, is aimed at reporting some information about the system user who has connected to the database server. It may include implementation-specific information about the means by the user connected, like an authentication method. This commit implements SYSTEM_USER as of auth_method:identity, where "auth_method" is a keyword about the authentication method used to log into the server (like peer, md5, scram-sha-256, gss, etc.) and "identity" is the authentication identity as introduced by 9afffcb (peer sets authn to the OS user name, gss to the user principal, etc.). This format has been suggested by Tom Lane. Note that thanks to d951052, SYSTEM_USER is available to parallel workers. Bump catalog version. Author: Bertrand Drouvot Reviewed-by: Jacob Champion, Joe Conway, Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/7e692b8c-0b11-45db-1cad-3afc5b57409f@amazon.com
* Restore pg_pread and friends.Thomas Munro2022-09-29
| | | | | | | | | | | | | | | | | | | | Commits cf112c12 and a0dc8271 were a little too hasty in getting rid of the pg_ prefixes where we use pread(), pwrite() and vectored variants. We dropped support for ancient Unixes where we needed to use lseek() to implement replacements for those, but it turns out that Windows also changes the current position even when you pass in an offset to ReadFile() and WriteFile() if the file handle is synchronous, despite its documentation saying otherwise. Switching to asynchronous file handles would fix that, but have other complications. For now let's just put back the pg_ prefix and add some comments to highlight the non-standard side-effect, which we can now describe as Windows-only. Reported-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/20220923202439.GA1156054%40nathanxps13
* Restrict Datum sort optimization to byval types onlyDavid Rowley2022-09-29
| | | | | | | | | | | | | | | | | | | 91e9e89dc modified nodeSort.c so that it used datum sorts when the targetlist of the outer node contained only a single column. That commit failed to recognise that the Datum returned by tuplesort_getdatum() must be pfree'd when the type is a byref type. Ronan Dunklau did originally propose the patch with that restriction, but that, probably through my own fault, got lost during further development work. Due to the timing of this report (PG15 RC1 is almost out the door), let's just restrict the datum sort optimization to apply for byval types only. We might want to look harder into making this work for byref types in PG16. Reported-by: Önder Kalacı Diagnosis-by: Tom Lane Discussion: https://postgr.es/m/CACawEhVxe0ufR26UcqtU7GYGRuubq3p6ZWPGXL4cxy_uexpAAQ@mail.gmail.com Backpatch-through: 15, where 91e9e89dc was introduced.
* Allow callback functions to deregister themselves during a call.Tom Lane2022-09-28
| | | | | | | | | | | | | | | | | | Fetch the next-item pointer before the call not after, so that we aren't dereferencing a dangling pointer if the callback deregistered itself during the call. The risky coding pattern appears in CallXactCallbacks, CallSubXactCallbacks, and ResourceOwnerReleaseInternal. (There are some other places that might be at hazard if they offered deregistration functionality, but they don't.) I (tgl) considered back-patching this, but desisted because it wouldn't be very safe for extensions to rely on this working in pre-v16 branches. Hao Wu Discussion: https://postgr.es/m/CAH+9SWXTiERkmhRke+QCcc+jRH8d5fFHTxh8ZK0-Yn4BSpyaAg@mail.gmail.com
* Change some errdetail() to errdetail_internal()Alvaro Herrera2022-09-28
| | | | | | | | | | | | This prevents marking the argument string for translation for gettext, and it also prevents the given string (which is already translated) from being translated at runtime. Also, mark the strings used as arguments to check_rolespec_name for translation. Backpatch all the way back as appropriate. None of this is caught by any tests (necessarily so), so I verified it manually.
* Fix bug in DROP OWNED BY.Robert Haas2022-09-28
| | | | | | | | | | Commit 6566133c5f52771198aca07ed18f84519fac1be7 broke the case where the role passed to DROP OWNED BY owns a database. Report by Rushabh Lathia, who also provided a patch, but this patch takes a slightly different approach to fixing the problem. Discussion: http://postgr.es/m/CAGPqQf2vO+nbo=3yAdZ8v26Rbug7bY4YjPaPLZx=L1NZ9-CC3w@mail.gmail.com
* Revert 56-bit relfilenode change and follow-up commits.Robert Haas2022-09-28
| | | | | | | | There are still some alignment-related failures in the buildfarm, which might or might not be able to be fixed quickly, but I've also just realized that it increased the size of many WAL records by 4 bytes because a block reference contains a RelFileLocator. The effect of that hasn't been studied or discussed, so revert for now.
* Fix InitializeRelfilenumberMap for 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7cRobert Haas2022-09-28
| | | | | | | | | | | Since relfilenodes are now 56-bits, we use bigint as the SQL type to represent them, which means F_INT8EQ must be used here rather than F_OIDEQ. On 64-bit machines this doesn't matter, but 32-bit machines are unhappy. Dilip Kumar Discussion: http://postgr.es/m/CAFiTN-t71ciSckMzixAhrF9py7oRO6xszKi4mTRwjuucXr5tpw@mail.gmail.com
* Fix alignment problems with SharedInvalSmgrMsg.Robert Haas2022-09-28
| | | | | | | | | | | | SharedInvalSmgrMsg can't require 8-byte alignment, because then SharedInvalidationMessage will require 8-byte alignment, which will then cause ParseCommitRecord to fail on machines that are picky about alignment, because it assumes that everything that gets packed into a commit record requires only 4-byte alignment. Another problem with 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c. Discussion: http://postgr.es/m/3825454.1664310917@sss.pgh.pa.us
* Remove publicationcmds.c's expr_allowed_in_node as a functionAlvaro Herrera2022-09-28
| | | | | | | | Its API is quite strange, and since there's only one caller, there's no reason for it to be a separate function in the first place. Inline it instead. Discussion: https://postgr.es/m/20220927124249.4zdzzlz6had7k3x2@alvherre.pgsql
* Fix some comments of do_pg_backup_start() and do_pg_backup_stop()Michael Paquier2022-09-28
| | | | | | | | | | Both functions referred to an incorrect variable name, so make the whole more consistent. Oversight in 7d70809. Author: Kyotaro Horiguchi, Bharath Rupireddy Discussion: https://postgr.es/m/20220927.172427.467118514018439476.horikyota.ntt@gmail.com
* Fix typos in commit 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c.Robert Haas2022-09-27
| | | | | | Reported by Justin Pryzby. Discussion: http://postgr.es/m/20220927185121.GE6256@telsasoft.com
* Convert *GetDatum() and DatumGet*() macros to inline functionsPeter Eisentraut2022-09-27
| | | | | | | | | | | | | | The previous macro implementations just cast the argument to a target type but did not check whether the input type was appropriate. The function implementation can do better type checking of the input type. For the *GetDatumFast() macros, converting to an inline function doesn't work in the !USE_FLOAT8_BYVAL case, but we can use AssertVariableIsOfTypeMacro() to get a similar level of type checking. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8528fb7e-0aa2-6b54-85fb-0c0886dbd6ed%40enterprisedb.com
* Increase width of RelFileNumbers from 32 bits to 56 bits.Robert Haas2022-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | RelFileNumbers are now assigned using a separate counter, instead of being assigned from the OID counter. This counter never wraps around: if all 2^56 possible RelFileNumbers are used, an internal error occurs. As the cluster is limited to 2^64 total bytes of WAL, this limitation should not cause a problem in practice. If the counter were 64 bits wide rather than 56 bits wide, we would need to increase the width of the BufferTag, which might adversely impact buffer lookup performance. Also, this lets us use bigint for pg_class.relfilenode and other places where these values are exposed at the SQL level without worrying about overflow. This should remove the need to keep "tombstone" files around until the next checkpoint when relations are removed. We do that to keep RelFileNumbers from being recycled, but now that won't happen anyway. However, this patch doesn't actually change anything in this area; it just makes it possible for a future patch to do so. Dilip Kumar, based on an idea from Andres Freund, who also reviewed some earlier versions of the patch. Further review and some wordsmithing by me. Also reviewed at various points by Ashutosh Sharma, Vignesh C, Amul Sul, Álvaro Herrera, and Tom Lane. Discussion: http://postgr.es/m/CA+Tgmobp7+7kmi4gkq7Y+4AM9fTvL+O1oQ4-5gFTT+6Ng-dQ=g@mail.gmail.com
* Introduce GUC_NO_RESET flag.Tom Lane2022-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the transaction-property GUCs such as transaction_isolation could be reset after starting a transaction, because we marked them as GUC_NO_RESET_ALL but still allowed a targeted RESET. That leads to assertion failures or worse, because those properties aren't supposed to change after we've acquired a transaction snapshot. There are some NO_RESET_ALL variables for which RESET is okay, so we can't just redefine the semantics of that flag. Instead introduce a separate GUC_NO_RESET flag. Mark "seed", as well as the transaction property GUCs, as GUC_NO_RESET. We have to disallow GUC_ACTION_SAVE as well as straight RESET, because otherwise a function having a "SET transaction_isolation" clause can still break things: the end-of-function restore action is equivalent to a RESET. No back-patch, as it's conceivable that someone is doing something this patch will forbid (like resetting one of these GUCs at transaction start, or "CREATE FUNCTION ... SET transaction_read_only = 1") and not running into problems with it today. Given how long we've had this issue and not noticed, the side effects in non-assert builds can't be too serious. Per bug #17385 from Andrew Bille. Masahiko Sawada Discussion: https://postgr.es/m/17385-9ee529fb091f0ce5@postgresql.org
* Improve some publication-related error messagesAlvaro Herrera2022-09-27
| | | | | | | | | | | While at it, remove an unused queryString parameter from CheckPubRelationColumnList() and make other minor stylistic changes. Backpatch to 15. Reported by Kyotaro Horiguchi <horikyota.ntt@gmail.com> Co-authored-by: Hou zj <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/20220926.160426.454497059203258582.horikyota.ntt@gmail.com
* Fix pg_stat_statements for MERGEAlvaro Herrera2022-09-27
| | | | | | | | | | | | We weren't jumbling the merge action list, so wildly different commands would be considered to use the same query ID. Add that, mention it in the docs, and some test lines. Backpatch to 15. Author: Tatsu <bt22nakamorit@oss.nttdata.com> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://postgr.es/m/d87e391694db75a038abc3b2597828e8@oss.nttdata.com
* Mark ParallelMessagePending as sig_atomic_tMichael Paquier2022-09-27
| | | | | | | | | ParallelMessagePending was previously marked as a boolean which should be fine on modern platforms, but the C standard recommends the use of sig_atomic_t for variables manipulated in signal handlers. Author: Hayato Kuroda Discussion: https://postgr.es/m/TYAPR01MB58667C15A95A234720F4F876F5529@TYAPR01MB5866.jpnprd01.prod.outlook.com
* Remove dependency to StringInfo in xlogbackup.{c.h}Michael Paquier2022-09-27
| | | | | | | | | | This was used as the returned result type of the generated contents for the backup_label and backup history files. This is replaced by a simple string, reducing the cleanup burden of all the callers of build_backup_content(). Reviewed-by: Bharath Rupireddy Discussion: https://postgr.es/m/YzERvNPaZivHEKZJ@paquier.xyz
* Enable WRITE_READ_PARSE_PLAN_TREES of rewritten utility statementsTom Lane2022-09-26
| | | | | | | | This was previously disabled because we lacked outfuncs/readfuncs support for most utility statement types. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us
* Implement WRITE_READ_PARSE_PLAN_TREES for raw parse treesTom Lane2022-09-26
| | | | | Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-07 19:55:14 +0000