aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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
* Simplify our Assert infrastructure a little.Tom Lane2022-10-10
| | | | | | | | | | | | | Remove the Trap and TrapMacro macros, which were nearly unused and confusingly had the opposite condition polarity from the otherwise-functionally-equivalent Assert macros. Having done that, it's very hard to justify carrying the errorType argument of ExceptionalCondition, so drop that too, and just let it assume everything's an Assert. This saves about 64K of code space as of current HEAD. Discussion: https://postgr.es/m/3928703.1665345117@sss.pgh.pa.us
* Remove unnecessary semicolons after goto labelsJohn Naylor2022-10-10
| | | | | | | | | | | According to the C standard, a label must followed by a statement. If there was ever a time we needed an empty statement here, it was a long time ago. Japin Li Reviewed by Julien Rouhaud Discussion: https://www.postgresql.org/message-id/MEYP282MB16690F40189A4F060B41D56DB65E9%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* 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
* autoconf: Rely on ar supporting index creationAndres Freund2022-10-07
| | | | | | | | | | | | | | | | | | | | This way we don't need RANLIB anymore, making it a bit simpler for the meson build to generate Makefile.global for PGXS compatibility. FreeBSD, NetBSD, OpenBSD, the only platforms where we didn't use AROPT=crs, all have supported the 's' option for a long time. On macOS we ran ranlib after installing a static library. This was added a long time ago, in 58ad65ec2def. I cannot reproduce an issue in more recent macOS versions. This is removed now. Based on discussion with Tom, I left the 'touch' at the end of static libraries generation, added in 826eff57c4c, in place. While it looks like current versions of Apple's ar/ranlib don't need it, it was needed not too long ago. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de
* 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
* Convert macros to static inline functions (rel.h)Peter Eisentraut2022-10-07
| | | | | Reviewed-by: Amul Sul <sulamul@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com
* 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
* Fix final warnings produced by -Wshadow=compatible-localDavid Rowley2022-10-07
| | | | | | | | I thought I had these in d8df67bb1, but per report from Andres Freund, I missed some. Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20221005214052.c4tkudawyp5wxt3c@awork3.anarazel.de
* windows: Adjust FD_SETSIZE via commandline defineAndres Freund2022-10-06
| | | | | | | | | | | | | | | | | | | | | | | When using precompiled headers, we cannot pre-define macros for the system headers from within .c files, as headers are already processed before the #define in the C file is reached. But we can pre-define using -DFD_SETSIZE, as long as that's also used when building the precompiled header. A few files #define FD_SETSIZE 1024 on windows, as the default is only 64. I am hesitant to change FD_SETSIZE globally on windows, due to src/backend/port/win32/socket.c using it to size on-stack arrays. Instead add -DFD_SETSIZE=1024 when building the specific targets needing it. We likely should move away from using select() in those places, but that's a larger change. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20221005190829.lda7ttalh4mzrvf4@awork3.anarazel.de Discussion: https://postgr.es/m/CA+hUKG+50eOUbN++ocDc0Qnp9Pvmou23DSXu=ZA6fepOcftKqA@mail.gmail.com Discussion: https://postgr.es/m/20190826054000.GE7005%40paquier.xyz
* 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
* Refactor TAP test authentication/001_password.plMichael Paquier2022-10-06
| | | | | | | | | | | The test is changed to test for connection strings rather than specific roles, and the reset logic of pg_hba.conf is extended so as the database and user name entries can be directly specified. This is aimed at being used as a base for more test scenarios of pg_hba.conf and authentication paths. Author: Bertrand Drouvot, Michael Paquier Discussion: https://postgr.es/m/Yz0xO0emJ+mxtj2a@paquier.xyz
* Fix final compiler warning produced by -Wshadow=compatible-localDavid Rowley2022-10-06
| | | | | | | | We're now able to compile the entire tree with -Wshadow=compatible-local without any compiler warnings. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvqWGMdB_pATeUqE=JCtNqNxObPOJ00jFEa2_sZ20j_Wvg@mail.gmail.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
* tests: Restrict pg_locks queries in advisory_locks.sql to current databaseAndres Freund2022-10-05
| | | | | | | | Otherwise testing an existing installation can fail, if there are other locks, e.g. from one of the isolation tests. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20221003234111.4ob7yph6r4g4ywhu@awork3.anarazel.de
* tests: Rename conflicting role namesAndres Freund2022-10-05
| | | | | | | These cause problems when running installcheck-world USE_MODULE_DB=1 with -j. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20221003234111.4ob7yph6r4g4ywhu@awork3.anarazel.de
* 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>
* meson: ecpg: Split definition of static and shared librariesAndres Freund2022-10-05
| | | | | | | | | | | | | | Required for correct resource file generation, as the resource files should only be added to the shared library. This also fixes a bunch of issues in the .pc files. Previously I tried to avoid building sources twice, once for the static and once for the shared libraries. We could still do so, but it's not clear that it's worth the complication. Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Discussion: https://postgr.es/m/20220927011951.j3h4o7n6bhf7dwau@awork3.anarazel.de
* meson: libpq: Revise static / shared library setupAndres Freund2022-10-05
| | | | | | | | | | | | | | | Improvements: - we don't need -DFRONTEND for libpq anymore since 1d77afefbd1 - the .pc file contents for a static libpq were wrong (referencing {pgport, common}_shlib) - incidentally fixes meson not supporting link_whole on AIX yet - added explanatory comments Previously I tried to avoid building libpq's sources twice, once for the static and once for the shared library. We could still do so, but it's not clear that it's worth the complication. 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
* Remove definition of JUMBLE_SIZE from queryjumble.hMichael Paquier2022-10-05
| | | | | | | | | The same exists in queryjumble.c, and it is used only locally in this file so let's remove the definition in the header. Author: Tatsu Nakamori Reviewed-by: Tom Lane, Julien Rouhaud Discussion: https://postgr.es/m/bb4ebd0412da9b1ac87a5eb2a3646bf1@oss.nttdata.com
* Use macros from xlog_internal.h for WAL segment logic in pg_resetwalMichael Paquier2022-10-05
| | | | | | | | | | | | | | | | | | | | | When scanning for the end of WAL, pg_resetwal has been maintaining its own internal logic to calculate segment numbers and to parse a WAL segment name for its timeline and segment number. The code claimed for example that XLogFromFileName() cannot be used because it lacks the possibility of specifying a WAL segment size, which is not the case since fc49e24, that has made the WAL segment size configurable at initialization time, extending this routine to do so. Similarly, this switches one segment number calculation to use XLByteToSeg() rather than the same logic present in xlog_internal.h. While on it, switch to TimeLineID in pg_resetwal.c for TLI numbers parsed from segment names, to be more consistent with XLogFromFileName(). The maths are exactly the same, but the code gets simplified. Author: Bharath Rupireddy Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/CALj2ACX+E_jnwqH_jmjhNG8BczJTNRTOLpw8K1CB1OcB48MJ8w@mail.gmail.com
* Add a few new patterns to the tab completion of psqlMichael Paquier2022-10-05
| | | | | | | | | | | | This improves the tab completion of psql on a few points: - Provide a list of subscriptions on \dRs. - Provide a list of publications on \dRp. - Add CURRENT_ROLE, CURRENT_USER, SESSION_USER when OWNER TO is provided at the end of a query (as defined by RoleSpec in gram.y). Author: Vignesh C Reviewed-by: Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/CALDaNm3toRBt6c6saY3174f7CsGztXRvVvfWbikkJEXY7x5WAA@mail.gmail.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
* Add filtering capability for cross-version pg_upgrade testsMichael Paquier2022-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | This commit expands the TAP tests of pg_upgrade when running these with different major versions for the "old" cluster (to-be-upgraded) and the "new" cluster (upgraded-to), by backporting some of the buildfarm facilities directory into the script: - Remove comments from the dump files, avoiding version-dependent information. - Remove empty lines from the dump files. - Use --extra-float-digits=0 in the pg_dump command, when using an "old" cluster with version equal to or lower than v11. - Use --wal-segsize and --allow-group-access in initdb only when the "old" cluster is equal to or higher than v11. This allows the tests to pass down to v14 with the main regression test suite, while v9.5~13 still generate some diffs, but these are minimal compared to what happened before this commit. Much more could be done, especially around dump differences with function and procedures (these can also be avoided with direct manipulation of the dumps loaded, for example, in a way similar to the buildfarm), but at least the basics are in place now. Reviewed-by: Justin Pryzby, Anton A. Melnikov Discussion: https://postgr.es/m/Yox1ME99GhAemMq1@paquier.xyz
* 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.
* Fix psql's behavior with \g for a multiple-command string.Tom Lane2022-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The pre-v15 behavior was to discard all but the last result, but with the new behavior of printing all results by default, we will send each such result to the \g file. However, we're still opening and closing the \g file for each result, so you lose all but the last result anyway. Move the output-file state up to ExecQueryAndProcessResults so that we open/close the \g file only once per command string. To support this without changing other behavior, we must adjust PrintQueryResult to have separate FILE * arguments for query and status output (since status output has never gone to the \g file). That in turn makes it a good idea to push the responsibility for fflush'ing output down to PrintQueryTuples and PrintQueryStatus. Also fix an infinite loop if COPY IN/OUT is attempted in \watch. We used to reject that, but that error exit path got broken somewhere along the line in v15. There seems no real reason to reject it anyway as the code now stands, so just remove the error exit and make sure that COPY OUT data goes to the right place. Also remove PrintQueryResult's unused is_watch parameter, and make some other cosmetic cleanups (adjust obsolete comments, break some overly-long lines). Daniel Vérité and Tom Lane Discussion: https://postgr.es/m/4333844c-2244-4d6e-a49a-1d483fbe304f@manitou-mail.org
* 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
* Add authentication TAP test for peer authenticationMichael Paquier2022-10-03
| | | | | | | | | | | This commit introduces an authentication test for the peer method, as of a set of scenarios with and without a user name map. The script is automatically skipped if peer is not supported in the environment where this test is run, checking this behavior by attempting a connection first on a cluster up and running. Author: Bertrand Drouvot Discussion: https://postgr.es/m/aa60994b-1c66-ca7a-dab9-9a200dbac3d2@amazon.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