aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Get rid of XLogCtlInsert->forcePageWritesAlvaro Herrera2022-10-19
| | | | | | | | | After commit 39969e2a1e4d, ->forcePageWrites is no longer very interesting: we can just test whether runningBackups is different from 0. This simplifies some code, so do away with it. Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/39969e2a1e4d7f5a37f3ef37d53bbfe171e7d77a
* Track LLVM 15 changes.Thomas Munro2022-10-19
| | | | | | | | | | | | Per https://llvm.org/docs/OpaquePointers.html, support for non-opaque pointers still exists and we can request that on our context. We have until LLVM 16 to move to opaque pointers, a much larger change. Back-patch to 11, where LLVM support arrived. Author: Thomas Munro <thomas.munro@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CAMHz58Sf_xncdyqsekoVsNeKcruKootLtVH6cYXVhhUR1oKPCg%40mail.gmail.com
* Remove pg_backup_start_callback and reuse similar codeAlvaro Herrera2022-10-19
| | | | | | | | | | | | | | | | | | | | | We had two copies of almost identical logic to revert shared memory state when a running backup aborts; we can remove pg_backup_start_callback if we adapt do_pg_abort_backup so that it can be used for this purpose too. However, in order for this to work, we have to repurpose the flag passed to do_pg_abort_backup. It used to indicate whether to throw a warning (and the only caller always passed true). It now indicates whether the callback is being called at start time (in which case the session backup state is known not to have been set to RUNNING yet, so action is always taken) or shmem time (in which case action is only taken if the session backup state is RUNNING). Thus the meaning of the flag is no longer superfluous, but it's actually quite critical to get right. I (Álvaro) chose to change the polarity and the code flow re. the flag from what Bharath submitted, for coding clarity. Co-authored-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://www.postgresql.org/message-id/20221013111330.564fk5tkwe3ha77l%40alvherre.pgsql
* Rework shutdown callback of archiver modulesMichael Paquier2022-10-19
| | | | | | | | | | | | | | | | | | | | | | | As currently designed, with a callback registered in a ERROR_CLEANUP block, the shutdown callback would get called twice when updating archive_library on SIGHUP, which is something that we want to avoid to ease the life of extension writers. Anyway, an ERROR in the archiver process is treated as a FATAL, stopping it immediately, hence there is no need for a ERROR_CLEANUP block. Instead of that, the shutdown callback is not called upon before_shmem_exit(), giving to the modules the opportunity to do any cleanup actions before the server shuts down its subsystems. While on it, this commit adds some testing coverage for the shutdown callback. Neither shell_archive nor basic_archive have been using it, and one is added to shell_archive, whose trigger is checked in a TAP test through a shutdown sequence. Author: Nathan Bossart, Bharath Rupireddy Reviewed-by: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20221015221328.GB1821022@nathanxps13 Backpatch-through: 15
* Enhance make_ctags and make_etags.Tatsuo Ishii2022-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | make_ctags did not include field members of structs since the commit 964d01ae90c314eb31132c2e7712d5d9fc237331. For example, in the following field of RestrictInfo: Selectivity norm_selec pg_node_attr(equal_ignore); pg_node_attr was mistakenly interpreted to be the name of the field. To fix this, add -I option to ctags command if the command is Exuberant ctags or Universal ctags (for plain old ctags, struct members are not included in the tags file anyway). Also add "-e" and "-n" options to make_ctags. The -e option invokes ctags command with -e option, which produces TAGS file for emacs. This allows to eliminate duplicate codes in make_etags so that make_etags just exec make_ctags with -e option. The -n option allows not to produce symbolic links in each sub directory (the default is producing symbolic links). Author: Yugo Nagata Reviewers: Alvaro Herrera, Tatsuo Ishii Discussion: https://postgr.es/m/flat/20221007154442.76233afc7c5b255c4de6528a%40sraoss.co.jp
* Fix typos in logical/launcher.cMichael Paquier2022-10-19
| | | | | Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pvbma5HCc7==-B1ycyLQVyu7Fqq-qV=jhC5Zx4pWqk3uw@mail.gmail.com
* Refactor regular expression handling in hba.cMichael Paquier2022-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | AuthToken gains a regular expression, and IdentLine is changed so as it uses an AuthToken rather than tracking separately the ident user string used for the regex compilation and its generated regex_t. In the case of pg_ident.conf, a set of AuthTokens is built in the pre-parsing phase of the file, and an extra regular expression is compiled when building the list of IdentLines, after checking the sanity of the fields in a pre-parsed entry. The logic in charge of computing and executing regular expressions is now done in a new set of routines called respectively regcomp_auth_token() and regexec_auth_token() that are wrappers around pg_regcomp() and pg_regexec(), working on AuthTokens. While on it, this patch adds a routine able to free an AuthToken, free_auth_token(), to simplify a bit the logic around the requirement of using a specific free routine for computed regular expressions. Note that there are no functional or behavior changes introduced by this commit. The goal of this patch is to ease the use of regular expressions with more items of pg_hba.conf (user list, database list, potentially hostnames) where AuthTokens are used extensively. This will be tackled later in a separate patch. Author: Bertrand Drouvot, Michael Paquier Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com
* Fix confusion about havingQual vs hasHavingQual in planner.Tom Lane2022-10-18
| | | | | | | | | | | | | | | | | | | | | | | Preprocessing of the HAVING clause will reduce havingQual to NIL if the clause is constant-TRUE. This is one case where that convention is rather unfortunate, because "HAVING TRUE" is not at all the same as not having any HAVING clause at all. (Per the SQL spec, it still forces the query to be grouped.) The planner deals with this by having a boolean hasHavingQual that records whether havingQual was originally nonempty; places that just want to check whether HAVING was specified are supposed to consult that. I found three places that got that wrong. Fortunately, these could only affect cost estimates not correctness. It'd be hard even to demonstrate the errors; for example, the one in allpaths.c would only matter in a query that has HAVING TRUE but no GROUP BY and no aggregates, which would require a completely variable-free SELECT list, making the case probably of only academic interest. Hence, while these are worth fixing before someone copies the incorrect coding somewhere more critical, they don't seem worth back-patching. I didn't bother trying to devise regression tests, either. Discussion: https://postgr.es/m/2503888.1666042643@sss.pgh.pa.us
* Remove no-longer-needed compatibility hackAlvaro Herrera2022-10-18
| | | | | | Our Perl version requirement was raised to 5.14 by commit 4c1532763a00 Discussion: https://postgr.es/m/20221017081649.fjcd2kjqif77uyf2@alvherre.pgsql
* Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATIONAlvaro Herrera2022-10-18
| | | | | | | | | | | | | | | The original hint says to use SET PUBLICATION when really ADD/DROP PUBLICATION is called for, so this is arguably a bug fix. Also, a very similar message elsewhere was using an inconsistent SQLSTATE. While at it, unwrap some strings. Backpatch to 15. Author: Hou zj <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/OS0PR01MB57160AD0E7386547BA978EB394299@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Remove compatibility declarations for InitMaterializedSRF()Michael Paquier2022-10-18
| | | | | | | | | | These routines have been renamed in a19e5ce. There is no need to keep the compatibility declarations on HEAD, as once an extension moves to the new routine name when compiling with v16~ the code would work the same way when recompiled on v15. No backpatch to v15 for this one, because ABI compatibility has to be maintained there. Discussion: https://postgr.es/m/20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de
* Rename SetSingleFuncCall() to InitMaterializedSRF()Michael Paquier2022-10-18
| | | | | | | | | | | | | | | | | | Per discussion, the existing routine name able to initialize a SRF function with materialize mode is unpopular, so rename it. Equally, the flags of this function are renamed, as of: - SRF_SINGLE_USE_EXPECTED -> MAT_SRF_USE_EXPECTED_DESC - SRF_SINGLE_BLESS -> MAT_SRF_BLESS The previous function and flags introduced in 9e98583 are kept around for compatibility purposes, so as any extension code already compiled with v15 continues to work as-is. The declarations introduced here for compatibility will be removed from HEAD in a follow-up commit. The new names have been suggested by Andres Freund and Melanie Plageman. Discussion: https://postgr.es/m/20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de Backpatch-through: 15
* Record dependencies of a cast on other casts that it requires.Tom Lane2022-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | When creating a cast that uses a conversion function, we've historically allowed the input and result types to be binary-compatible with the function's input and result types, rather than necessarily being identical. This means that the new cast is logically dependent on the binary-compatible cast or casts that it references: if those are defined by pg_cast entries, and you try to restore the new cast without having defined them, it'll fail. Hence, we should make pg_depend entries to record these dependencies so that pg_dump knows that there is an ordering requirement. This is not the only place where we allow such shortcuts; aggregate functions for example are similarly lax, and in principle should gain similar dependencies. However, for now it seems sufficient to fix the cast-versus-cast case, as pg_dump's other ordering heuristics should keep it out of trouble for other object types. Per report from David Turoň; thanks also to Robert Haas for preliminary investigation. I considered back-patching, but seeing that this issue has existed for many years without previous reports, it's not clear it's worth the trouble. Moreover, back-patching wouldn't be enough to ensure that the new pg_depend entries exist in existing databases anyway. Discussion: https://postgr.es/m/OF0A160F3E.578B15D1-ONC12588DA.003E4857-C12588DA.0045A428@notes.linuxbox.cz
* Reject non-ON-SELECT rules that are named "_RETURN".Tom Lane2022-10-17
| | | | | | | | | | | | DefineQueryRewrite() has long required that ON SELECT rules be named "_RETURN". But we overlooked the converse case: we should forbid non-ON-SELECT rules that are named "_RETURN". In particular this prevents using CREATE OR REPLACE RULE to overwrite a view's _RETURN rule with some other kind of rule, thereby breaking the view. Per bug #17646 from Kui Liu. Back-patch to all supported branches. Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org
* Guard against table-AM-less relations in planner.Tom Lane2022-10-17
| | | | | | | | | | | | | | | The executor will dump core if it's asked to execute a seqscan on a relation having no table AM, such as a view. While that shouldn't really happen, it's possible to get there via catalog corruption, such as a missing ON SELECT rule. It seems worth installing a defense against that. There are multiple plausible places for such a defense, but I picked the planner's get_relation_info(). Per discussion of bug #17646 from Kui Liu. Back-patch to v12 where the tableam APIs were introduced; in older versions you won't get a SIGSEGV, so it seems less pressing. Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org
* Add checks for regexes with user name map in test for peer authenticationMichael Paquier2022-10-17
| | | | | | | | | | There is already some coverage for that in the kerberos test suite, though it requires PG_TEST_EXTRA to be set as per its insecure nature. This provides coverage in a default setup, as long as peer is supported on the platform where its test is run. Author: Bertrand Drouvot Discussion: https://postgr.es/m/7f87ca27-e184-29da-15d6-8be4325ad02e@gmail.com
* Fix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value.Tom Lane2022-10-16
| | | | | | | | | | | | | | | | | | | | | | If the non-recursive term of a SEARCH BREADTH FIRST recursive query has only constants in its target list, the planner will fold the starting RowExpr added by rewrite into a simple Const of type RECORD. The executor doesn't have any problem with that --- but EXPLAIN VERBOSE will encounter the Const as the ultimate source of truth about what the field names of the SET column are, and it didn't know what to do with that. Fortunately, we can pull the identifying typmod out of the Const, in much the same way that record_out would. For reasons that remain a bit obscure to me, this only fails with SEARCH BREADTH FIRST, not SEARCH DEPTH FIRST or CYCLE. But I added regression test cases for both of those options too, just to make sure we don't break it in future. Per bug #17644 from Matthijs van der Vleuten. Back-patch to v14 where these constructs were added. Discussion: https://postgr.es/m/17644-3bd1f3036d6d7a16@postgresql.org
* Rename parser token REF to REF_P to avoid a symbol conflict.Tom Lane2022-10-16
| | | | | | | | | | | | | | | | | | | | | | | In the latest version of Apple's macOS SDK, <sys/socket.h> fails to compile if "REF" is #define'd as something. Apple may or may not agree that this is a bug, and even if they do accept the bug report I filed, they probably won't fix it very quickly. In the meantime, our back branches will all fail to compile gram.y. v15 and HEAD currently escape the problem thanks to the refactoring done in 98e93a1fc, but that's purely accidental. Moreover, since that patch removed a widely-visible inclusion of <netdb.h>, back-patching it seems too likely to break third-party code. Instead, change the token's code name to REF_P, following our usual convention for naming parser tokens that are likely to have symbol conflicts. The effects of that should be localized to the grammar and immediately surrounding files, so it seems like a safer answer. Per project policy that we want to keep recently-out-of-support branches buildable on modern systems, back-patch all the way to 9.2. Discussion: https://postgr.es/m/1803927.1665938411@sss.pgh.pa.us
* Use libc's snprintf, not sprintf, for special cases in snprintf.c.Tom Lane2022-10-16
| | | | | | | | | | | | | | | | | | | | | | | | | | snprintf.c has always fallen back on libc's *printf implementation when printing pointers (%p) and floats. When this code originated, we were still supporting some platforms that lacked native snprintf, so we used sprintf for that. That's not actually unsafe in our usage, but nonetheless builds on macOS are starting to complain about sprintf being unconditionally deprecated; and I wouldn't be surprised if other platforms follow suit. There seems little reason to believe that any platform supporting C99 wouldn't have standards-compliant snprintf, so let's just use that instead to suppress such warnings. Back-patch to v12, which is where we started to require C99. It's also where we started to use our snprintf.c everywhere, so this wouldn't be enough to suppress the warning in older branches anyway --- that is, in older branches these aren't necessarily all our usages of libc's sprintf. It is enough in v12+ because any deprecation annotation attached to libc's sprintf won't apply to pg_sprintf. (Whether all our usages of pg_sprintf are adequately safe is not a matter I intend to address here, but perhaps it could do with some review.) Per report from Andres Freund and local testing. Discussion: https://postgr.es/m/20221015211955.q4cwbsfkyk3c4ty3@awork3.anarazel.de
* Disallow MERGE cleanly for foreign partitionsAlvaro Herrera2022-10-15
| | | | | | | | | | | | While directly targetting a foreign table with MERGE was already expressly forbidden, we failed to catch the case of a partitioned table that has a foreign table as a partition; and the result if you try is an incomprehensible error. Fix that by adding a specific check. Backpatch to 15. Reported-by: Tatsuhiro Nakamori <bt22nakamorit@oss.nttdata.com> Discussion: https://postgr.es/m/bt22nakamorit@oss.nttdata.com
* Fix some comments in proc.hMichael Paquier2022-10-15
| | | | | | | | There was a typo and two places where delayChkpt was still mentioned, but it is called delayChkptFlags these days. Author: David Christensen Discussion: https://postgr.es/m/CAOxo6XLB=ab_Y9jRw4iKyMZDns0wo=EGSRvijhhaL67RzqbtMg@mail.gmail.com
* pgstat: Track time of the last scan of a relationAndres Freund2022-10-14
| | | | | | | | | | | | | | | | | | | | | It can be useful to know when a relation has last been used, e.g., when evaluating whether an index is still required. It was already possible to infer the time of the last usage by tracking, e.g., pg_stat_all_indexes.idx_scan over time. But far from everybody does so. To make it easier to detect the last time a relation has been scanned, track that time in each relation's pgstat entry. To minimize overhead a) the timestamp is updated only when the backend pending stats entry is flushed to shared stats b) the last transaction's stop timestamp is used as the timestamp. Bumps catalog and stats format versions. Author: Dave Page <dpage@pgadmin.org> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Bruce Momjian <bruce@momjian.us> Reviewed-by: Vik Fearing <vik@postgresfriends.org> Discussion: https://postgr.es/m/CA+OCxozrVHNFVEPkweUHMZje+t1tfY816d9MZYc6eZwOOusOaQ@mail.gmail.com
* Have GetCurrentTransactionStopTimestamp() set xactStopTimestamp if unsetAndres Freund2022-10-14
| | | | | | | | | | | | | | | | Previously GetCurrentTransactionStopTimestamp() computed a new timestamp whenever xactStopTimestamp was unset and xactStopTimestamp was only set when a commit or abort record was written. An upcoming patch will add additional calls to GetCurrentTransactionStopTimestamp() from pgstats. To avoid computing timestamps multiple times, set xactStopTimestamp in GetCurrentTransactionStopTimestamp() if not already set. Author: Dave Page <dpage@pgadmin.org> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Vik Fearing <vik@postgresfriends.org> Discussion: https://postgr.es/m/20220906155325.an3xesq5o3fq36gt%40awork3.anarazel.de
* libpq: Reset singlerow flag correctly in pipeline modeAlvaro Herrera2022-10-14
| | | | | | | | | | | | | | | When a query whose results were requested in single-row mode is the last in the queue by the time those results are being read, the single-row flag was not being reset, because we were returning early from pqPipelineProcessQueue. Move that stanza up so that the flag is always reset at the end of sending that query's results. Add a test for the situation. Backpatch to 14. Author: Denis Laxalde <denis.laxalde@dalibo.com> Discussion: https://postgr.es/m/01af18c5-dacc-a8c8-07ee-aecc7650c3e8@dalibo.com
* Add auxiliary lists to GUC data structures for better performance.Tom Lane2022-10-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | The previous patch made addition of new GUCs cheap, but other GUC operations aren't improved and indeed get a bit slower, because hash_seq_search() is slower than just scanning a pointer array. However, most performance-critical GUC operations only need to touch a relatively small fraction of the GUCs; especially so for AtEOXact_GUC(). We can improve matters at the cost of a bit more space by adding dlist or slist links to the GUC data structures. This patch invents lists that track (1) all GUCs with non-default "source"; (2) all GUCs with nonempty state stack (implying they've been changed in the current transaction); (3) all GUCs due for reporting to the client. All of guc.c's performance-critical cases can make use of one or another of these lists to avoid searching the whole hash table. In particular, the stack list means that transaction end doesn't take time proportional to the number of GUCs, but only to the number changed in the current transaction. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
* Replace the sorted array of GUC variables with a hash table.Tom Lane2022-10-14
| | | | | | | | | | | | | | | | This gets rid of bsearch() in favor of hashed lookup. The main advantage is that it becomes far cheaper to add new GUCs, since we needn't re-sort the pointer array. Adding N new GUCs had been O(N^2 log N), but now it's closer to O(N). We need to sort only in SHOW ALL and equivalent functions, which are hopefully not performance-critical to anybody. Also, merge GetNumConfigOptions() into get_guc_variables(), because in a world where the set of GUCs isn't fairly static you really want to consider those two results as tied together not independent. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
* Store GUC data in a memory context, instead of using malloc().Tom Lane2022-10-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The only real argument for using malloc directly was that we needed the ability to not throw error on OOM; but mcxt.c grew that feature awhile ago. Keeping the data in a memory context improves accountability and debuggability --- for example, without this it's almost impossible to detect memory leaks in the GUC code with anything less costly than valgrind. Moreover, the next patch in this series will add a hash table for GUC lookup, and it'd be pretty silly to be using palloc-dependent hash facilities alongside malloc'd storage of the underlying data. This is a bit invasive though, in particular causing an API break for GUC check hooks that want to modify the GUC's value or use an "extra" data structure. They must now use guc_malloc() and guc_free() instead of malloc() and free(). Failure to change affected code will result in assertion failures or worse; but thanks to recent effort in the mcxt infrastructure, it shouldn't be too hard to diagnose such oversights (at least in assert-enabled builds). One note is that this changes ParseLongOption() to return short-lived palloc'd not malloc'd data. There wasn't any caller for which the previous definition was better. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
* Make some minor improvements in memory-context infrastructure.Tom Lane2022-10-14
| | | | | | | | | | | | | | | We lack a version of repalloc() that supports MCXT_ALLOC_NO_OOM semantics, so invent repalloc_extended() with the usual set of flags. repalloc_huge() becomes a legacy wrapper for that. Also, fix dynahash.c so that it can support HASH_ENTER_NULL requests when using the default palloc-based allocator. The only reason it didn't do that already was the lack of the MCXT_ALLOC_NO_OOM option when that code was written, ages ago. While here, simplify a few overcomplicated tests in mcxt.c. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
* Standardize format for printing PIDsPeter Eisentraut2022-10-14
| | | | | | | | | | | | | | Most code prints PIDs as %d, but some code tried to print them as long or unsigned long. While this is in theory allowed, the fact that PIDs fit into int is deeply baked into all PostgreSQL code, so these random deviations don't accomplish anything except confusion. Note that we still need casts from pid_t to int, because on 64-bit MinGW, pid_t is long long int. (But per above, actually supporting that range in PostgreSQL code would be major surgery and probably not useful.) Discussion: https://www.postgresql.org/message-id/289c2e45-c7d9-5ce4-7eff-a9e2a33e1580@enterprisedb.com
* Fix incorrect comment regarding command completion tagsDavid Rowley2022-10-14
| | | | | | | | | | | | The comment talked about some Asserts which did not exist and also a variable name which seems to have long since disappeared. Rewrite the comment in a way that will hopefully stand the test of time and inform people why we always write "INSERT 0 <nrows>" instead of "INSERT <nrows>" in the command completion tag for INSERT. Reviewed-by: Mark Dilger Discussion: https://postgr.es/m/CAApHDvpiUg09AvvGAVopNAKemA9z-kCmt7Fi6HKauc32bKzx4w@mail.gmail.com
* Put tests of md5() function into separate test filePeter Eisentraut2022-10-13
| | | | | | | | | | In FIPS mode, these calls will fail. By having them in a separate file, it would make it easier to have an alternative output file or selectively disable these tests. This isn't done here; this is just some preparation. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/647f6cc1-473d-f788-ade0-c09201e5ab6a@enterprisedb.com
* Allow batch insertion during COPY into a foreign table.Etsuro Fujita2022-10-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 3d956d956 allowed the COPY, but it's done by inserting individual rows to the foreign table, so it can be inefficient due to the overhead caused by each round-trip to the foreign server. To improve performance of the COPY in such a case, this patch allows batch insertion, by extending the multi-insert machinery in CopyFrom() to the foreign-table case so that we insert multiple rows to the foreign table at once using the FDW callback routine added by commit b663a4136. This patch also allows this for postgres_fdw. It is enabled by the "batch_size" option added by commit b663a4136, which is disabled by default. When doing batch insertion, we update progress of the COPY command after performing the FDW callback routine, to count rows not suppressed by the FDW as well as a BEFORE ROW INSERT trigger. For consistency, this patch changes the timing of updating it for plain tables: previously, we updated it immediately after adding each row to the multi-insert buffer, but we do so only after writing the rows stored in the buffer out to the table using table_multi_insert(), which I think would be consistent even with non-batching mode, because in that mode we update it after writing each row out to the table using table_tuple_insert(). Andrey Lepikhov, heavily revised by me, with review from Ian Barwick, Andrey Lepikhov, and Zhihong Yu. Discussion: https://postgr.es/m/bc489202-9855-7550-d64c-ad2d83c24867%40postgrespro.ru
* Improve the WARNING message for CREATE SUBSCRIPTION.Amit Kapila2022-10-13
| | | | | | Author: Peter Smith Reviewed-By: Alvaro Herrera, Tom Lane, Amit Kapila Discussion: https://postgr.es/m/CAHut+PvqdqOanheWSHDyhQiF+Z-7w=-+k4U+bwbT=b6YQ_hrXQ@mail.gmail.com
* Fix ordering issue with WAL operations in GIN fast insert pathMichael Paquier2022-10-13
| | | | | | | | | | | | | | | | | | Contrary to what is documented in src/backend/access/transam/README, ginHeapTupleFastInsert() had a few ordering issues with the way it does its WAL operations when inserting items in its fast path. First, when using a separate list, XLogBeginInsert() was being always called before START_CRIT_SECTION(), and in this case a second thing was wrong when merging lists, as an exclusive lock was taken on the tail page *before* calling XLogBeginInsert(). Finally, when inserting items into a tail page, the order of XLogBeginInsert() and START_CRIT_SECTION() was reversed. This commit addresses all these issues by moving the calls of XLogBeginInsert() after all the pages logged are locked and pinned, within a critical section. Author: Matthias van de Meent, Zhang Mingli Discussion: https://postgr.es/m/CAEze2WhL8uLMqynnnCu1LAPwxD5RKEo0nHV+eXGg_N6ELU88HQ@mail.gmail.com
* doc: Fix description of replication command CREATE_REPLICATION_SLOTMichael Paquier2022-10-13
| | | | | | | | | | | The output plugin name is a mandatory option when creating a logical slot, but the grammar documented was not described as such. While on it, fix two comments in repl_gram.y to show that TEMPORARY is an optional grammar choice. Author: Ayaki Tachikake Discussion: https://postgr.es/m/OSAPR01MB2852607B2329FFA27834105AF1229@OSAPR01MB2852.jpnprd01.prod.outlook.com Backpatch-through: 15
* Fix outdated code referenceAlvaro Herrera2022-10-12
| | | | | | | | ExecCreatePartitionPruneState was renamed by commit 297daa9d4353, but this test file didn't get the memo. Repair. Author: Amit Langote Discussion: https://postgr.es/m/CA+HiwqFLw=oLX0tP9kcKBmoOExNjDaoAe99dRcxo-GdB9abP9A@mail.gmail.com
* Reduce xlog.h inclusion footprintAlvaro Herrera2022-10-12
| | | | | | | | | | This file needs xlogreader.h only for the XLogReaderState typedef; but we can dodge that by forward-declaring it. Many files use xlog.h for reasons other than reading WAL, and it's not good to force all those files to include xlogreader.h, so take it out. Surprisingly, there is no fallout in core code from making this change. Perhaps external code will have to start including xlogreader.h.
* Reduce basebackup_sink.h inclusion footprintAlvaro Herrera2022-10-12
| | | | This file doesn't need xlog_internal.h, only xlogdefs.h.
* Add meson.build to version_stamp.plPeter Eisentraut2022-10-12
| | | | | | Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/7567dd2d-5e28-c135-79ff-270d7ed83490%40enterprisedb.com
* Remove Abs()Peter Eisentraut2022-10-12
| | | | | | | | All callers have been replaced by standard C library functions. 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 shadow variable in postgres.cMichael Paquier2022-10-12
| | | | | | | | -Wshadow=compatible-local is added by default since 0fe954c, and this warning was detected under -DWRITE_READ_PARSE_PLAN_TREES. Reviewed-by: David Rowley Discussion: https://postgr.es/m/Y0Ya5SH0QiaO9kKG@paquier.xyz
* Simplify some maths in xlogreader.cMichael Paquier2022-10-12
| | | | | | | | | | | | An LSN was calculated from a segment number, a segment size and a position offset, matching exactly the LSN given by the caller of XLogReaderValidatePageHeader(). This change removes the extra LSN calculation, relying only on the LSN given by the function caller instead. Author: Bharath Rupireddy Reviewed-by: Richard Guo, Álvaro Herrera, Kyotaro Horiguchi Discussion: https://postgr.es/m/CALj2ACXuh4Ms9j9sxMYdtHEe=5sFcyrs-GAHyADu_A_G71kZTg@mail.gmail.com
* Fix compilation warning in test_copy_callbacksMichael Paquier2022-10-12
| | | | | | | | | | A passed-in parameter value was incorrect, for a warning coming from MSVC. Oversight in 9fcdf2c. Reported-by: Andres Freund Discussion: https://postgr.es/m/20221011224221.dvg5q7e7vhjdtcvv@awork3.anarazel.de
* Harden pmsignal.c against clobbered shared memory.Tom Lane2022-10-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | The postmaster is not supposed to do anything that depends fundamentally on shared memory contents, because that creates the risk that a backend crash that trashes shared memory will take the postmaster down with it, preventing automatic recovery. In commit 969d7cd43 I lost sight of this principle and coded AssignPostmasterChildSlot() in such a way that it could fail or even crash if the shared PMSignalState structure became corrupted. Remarkably, we've not seen field reports of such crashes; but I managed to induce one while testing the recent changes around palloc chunk headers. To fix, make a semi-duplicative state array inside the postmaster so that we need consult only local state while choosing a "child slot" for a new backend. Ensure that other postmaster-executed routines in pmsignal.c don't have critical dependencies on the shared state, either. Corruption of PMSignalState might now lead ReleasePostmasterChildSlot() to conclude that backend X failed, when actually backend Y was the one that trashed things. But that doesn't matter, because we'll force a cluster-wide reset regardless. Back-patch to all supported branches, since this is an old bug. Discussion: https://postgr.es/m/3436789.1665187055@sss.pgh.pa.us
* Yet further fixes for multi-row VALUES lists for updatable views.Tom Lane2022-10-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | DEFAULT markers appearing in an INSERT on an updatable view could be mis-processed if they were in a multi-row VALUES clause. This would lead to strange errors such as "cache lookup failed for type NNNN", or in older branches even to crashes. The cause is that commit 41531e42d tried to re-use rewriteValuesRTE() to remove any SetToDefault nodes (that hadn't previously been replaced by the view's own default values) appearing in "product" queries, that is DO ALSO queries. That's fundamentally wrong because the DO ALSO queries might not even be INSERTs; and even if they are, their targetlists don't necessarily match the view's column list, so that almost all the logic in rewriteValuesRTE() is inapplicable. What we want is a narrow focus on replacing any such nodes with NULL constants. (That is, in this context we are interpreting the defaults as being strictly those of the view itself; and we already replaced any that aren't NULL.) We could add still more !force_nulls tests to further lobotomize rewriteValuesRTE(); but it seems cleaner to split out this case to a new function, restoring rewriteValuesRTE() to the charter it had before. Per bug #17633 from jiye_sw. Patch by me, but thanks to Richard Guo and Japin Li for initial investigation. Back-patch to all supported branches, as the previous fix was. Discussion: https://postgr.es/m/17633-98cc85e1fa91e905@postgresql.org
* C comment: explain procArray->pgprocnos[]Bruce Momjian2022-10-11
| | | | | | | | | | Reported-by: Aleksander Alekseev Discussion: https://postgr.es/m/CAJ7c6TOs9Dh3KNR2kiQJ3Ow0=TBucL_57DAbm--2p8w5x_8YXQ@mail.gmail.com Author: Aleksander Alekseev Backpatch-through: master
* Add a common function to generate the origin name.Amit Kapila2022-10-11
| | | | | | | | | | | | | Make a common replication origin name formatting function to replace multiple snprintf() expressions. This also includes logic previously done by ReplicationOriginNameForTablesync(). This makes the code to generate the origin name consistent among apply worker and tablesync worker. Author: Peter Smith Reviewed-By: Aleksander Alekseev Discussion: https://postgr.es/m/CAHut%2BPsa8hhfSE6ozUK-ih7GkQziAVAf4f3bqiXEj2nQiu-43g%40mail.gmail.com
* Add TAP tests for role membership in pg_hba.confMichael Paquier2022-10-11
| | | | | | | | | | | | | | | This commit expands the coverage of pg_hba.conf with checks specific to role memberships (one "root" role combined with a member and a non-member). Coverage is added for the database keywords "samegroup" and "samerole", where the specified role has to be be a member of the role with the same name as the requested database, and '+' on the user entry, where members are allowed. These tests are plugged in the authentication test 001_password.pl as of extra connection attempts combined with resets of pg_hba.conf, making them rather cheap. Author: Nathan Bossart Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/20221009211348.GB900071@nathanxps13
* Add support for COPY TO callback functionsMichael Paquier2022-10-11
| | | | | | | | | | | | | | | | | This is useful as a way for extensions to process COPY TO rows in the way they see fit (say auditing, analytics, backend, etc.) without the need to invoke an external process running as the OS user running the backend through PROGRAM that requires superuser rights. COPY FROM already provides a similar callback for logical replication. For COPY TO, the callback is triggered when we are ready to send a row in CopySendEndOfRow(), which is the same code path as when sending a row to a frontend or a pipe/file. A small test module, test_copy_callbacks, is added to provide some coverage for this facility. Author: Bilva Sanaba, Nathan Bossart Discussion: https://postgr.es/m/253C21D1-FCEB-41D9-A2AF-E6517015B7D7@amazon.com
* Harden memory context allocators against bogus chunk pointers.Tom Lane2022-10-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before commit c6e0fe1f2, functions such as AllocSetFree could pretty safely presume that they were given a valid chunk pointer for their own type of context, because the indirect call through a memory context object and method struct would be very unlikely to work otherwise. But now, if pfree() is mistakenly invoked on a pointer to garbage, we have three chances in eight of ending up at one of these functions. That means we need to take extra measures to verify that we are looking at what we're supposed to be looking at, especially in debug builds. Hence, add code to verify that the chunk's back-link to a block header leads to a memory context object that satisfies the right sort of IsA() check. This is still a bit weaker than what we did before, but for the moment assume that an IsA() check is sufficient. As a compromise between speed and safety, implement these checks as Asserts when dealing with small chunks but plain test-and-elogs when dealing with large (external) chunks. The latter case should not be too performance-critical, but the former case probably is. In slab.c, all chunks are small; but nonetheless use a plain test in SlabRealloc, because that is certainly not performance-critical, indeed we should be suspicious that it's being called in error. In aset.c, additionally add some assertions that the "value" field of the chunk header is within the small range allowed for freelist indexes. Without that, we might find ourselves trying to wipe most of memory when CLOBBER_FREED_MEMORY is enabled, or scribbling on a "freelist header" that's far away from the context object. Eventually, field experience might show us that it's smarter for these tests to be active always, but for now we'll try to get away with just having them as assertions. While at it, also be more uniform about asserting that context objects passed as parameters are of the type we expect. Some places missed that altogether, and slab.c was for no very good reason doing it differently from the other allocators. Discussion: https://postgr.es/m/3578387.1665244345@sss.pgh.pa.us