aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* pg_clean_ascii(): escape bytes rather than lose themPeter Eisentraut2022-09-13
| | | | | | | | | Rather than replace each unprintable byte with a '?' character, replace it with a hex escape instead. The API now allocates a copy rather than modifying the input in place. Author: Jacob Champion <jchampion@timescale.com> Discussion: https://www.postgresql.org/message-id/CAAWbhmgsvHrH9wLU2kYc3pOi1KSenHSLAHBbCVmmddW6-mc_=w@mail.gmail.com
* Improve wal_decode_buffer_size description some moreAlvaro Herrera2022-09-13
| | | | | | Per Thomas Munro Discussion: https://postgr.es/m/CA+hUKGJ9wP9kpvgoxHvqA=4g1d9-y_w3LhhdhFVU=mFiqjwHww@mail.gmail.com
* Remove useless pstrdups in untransformRelOptionsAlvaro Herrera2022-09-13
| | | | | | | | | | | | | | | The two strings are already a single palloc'd chunk, not freed; there's no reason to allocate separate copies that have the same lifetime. This code is only called in short-lived memory contexts (except in some cases in TopTransactionContext, which is still short-lived enough not to really matter), and typically only for short arrays, so the memory or computation saved is likely negligible. However, let's fix it to avoid leaving a bad example of code to copy. This is the only place I could find where we're doing this with makeDefElem(). Reported-by: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/20220909142050.3vv2hjekppk265dd@alvherre.pgsql
* Rename macro related to pg_backup_stop()Michael Paquier2022-09-13
| | | | | | | | | This should have been part of 39969e2 that has renamed pg_stop_backup() to pg_backup_stop(), and this one is the last reference to pg_stop/start_backup() I could find in the tree. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACXjvC28ppeDTCrfaSyHga0ggP5nRLJbsjx=7N-74UT4QA@mail.gmail.com
* Don't reference out-of-bounds array elements in brin_minmax_multi.cDavid Rowley2022-09-13
| | | | | | | | | | | | | | | | | | The primary fix here is to fix has_matching_range() so it does not reference ranges->values[-1] when nranges == 0. Similar problems existed in AssertCheckRanges() too. It does not look like any of these problems could lead to a crash as the array in question is at the end of the Ranges struct, and values[-1] is memory that belongs to other fields in the struct. However, let's get rid of these rather unsafe coding practices. In passing, I (David) adjusted some comments to try to make it more clear what some of the fields are for in the Ranges struct. I had to study the code to find out what nsorted was for as I couldn't tell from the comments. Author: Ranier Vilela Discussion: https://postgr.es/m/CAEudQAqJQzPitufX-jR=YUbJafpCDAKUnwgdbX_MzSc93wuvdw@mail.gmail.com Backpatch-through: 14, where multi-range brin was added.
* Revert "Convert *GetDatum() and DatumGet*() macros to inline functions"Peter Eisentraut2022-09-12
| | | | | | This reverts commit 595836e99bf1ee6d43405b885fb69bb8c6d3ee23. It has problems when USE_FLOAT8_BYVAL is off.
* Convert *GetDatum() and DatumGet*() macros to inline functionsPeter Eisentraut2022-09-12
| | | | | | | | | 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. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Discussion: https://www.postgresql.org/message-id/flat/8528fb7e-0aa2-6b54-85fb-0c0886dbd6ed%40enterprisedb.com
* Fix NaN comparison in circle_same testDaniel Gustafsson2022-09-12
| | | | | | | | | | | | | | | | | | | | | | Commit c4c340088 changed geometric operators to use float4 and float8 functions, and handle NaN's in a better way. The circle sameness test had a typo in the code which resulted in all comparisons with the left circle having a NaN radius considered same. postgres=# select '<(0,0),NaN>'::circle ~= '<(0,0),1>'::circle; ?column? ---------- t (1 row) This fixes the sameness test to consider the radius of both the left and right circle. Backpatch to v12 where this was introduced. Author: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/CAEudQAo8dK=yctg2ZzjJuzV4zgOPBxRU5+Kb+yatFiddtQk6Rw@mail.gmail.com Backpatch-through: v12
* Make the tablesync worker's replication origin drop logic robust.Amit Kapila2022-09-12
| | | | | | | | | | | | | | | | | | | | | | | In commit f6c5edb8ab, we started to drop the replication origin slots before tablesync worker exits to avoid consuming more slots than required. We were dropping the replication origin in the same transaction where we were marking the tablesync state as SYNCDONE. Now, if there is any error after we have dropped the origin but before we commit the containing transaction, the in-memory state of replication progress won't be rolled back. Due to this, after the restart, tablesync worker can start streaming from the wrong location and can apply the already processed transaction. To fix this, we need to opportunistically drop the origin after marking the tablesync state as SYNCDONE. Even, if the tablesync worker fails to remove the replication origin before exit, the apply worker ensures to clean it up afterward. Reported by Tom Lane as per buildfarm. Diagnosed-by: Masahiko Sawada Author: Hou Zhijie Reviewed-By: Masahiko Sawada, Amit Kapila Discussion: https://postgr.es/m/20220714115155.GA5439@depesz.com Discussion: https://postgr.es/m/CAD21AoAw0Oofi4kiDpJBOwpYyBBBkJj=sLUOn4Gd2GjUAKG-fw@mail.gmail.com
* Assorted examples of expanded type-safer palloc/pg_malloc APIPeter Eisentraut2022-09-12
| | | | | | | | | This adds some uses of the new palloc/pg_malloc variants here and there as a demonstration and test. This is kept separate from the actual API patch, since the latter might be backpatched at some point. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a907@enterprisedb.com
* Make eval statement naturally proof against perltidyJohn Naylor2022-09-12
| | | | | | | | | This is a bit less verbose than adding an exception. Rewrite the other eval statement in Catalog.pm as well, just for the sake of consistency. Idea from Andrew Dunstan Discussion: https://www.postgresql.org/message-id/CAD5tBc%2BzRhuWn_S4ayH2sWKe60FQu1guTTokDFS3YMOeSrsozA%40mail.gmail.com
* Replace loading of ldap_start_tls_sA() by direct function callMichael Paquier2022-09-12
| | | | | | | | | | | | | | | This change impacts the backend-side code in charge of starting a LDAP TLS session. It is a bit sad that it is not possible to unify the WIN32 and non-WIN32 code paths, but the different number of arguments for both discard this possibility. This is similar to 47bd0b3, where this replaces the last function loading that seems worth it, any others being either environment or version-dependent. Reported-by: Thomas Munro Reviewed-by: Thomas Munro Discussion: https://postgr.es/m/Yx0rxpNgDh8tN4XA@paquier.xyz
* Free correctly LDAPMessage returned by ldap_search_s() in auth.cMichael Paquier2022-09-10
| | | | | | | | | | | | | | | | The LDAP wiki states that the search message should be freed regardless of the return value of ldap_search_s(), but we failed to do so in one backend code path when searching LDAP with a filter. This is not critical in an authentication code path failing in the backend as this causes such the process to exit promptly, but let's be clean and free the search message appropriately, as documented by upstream. All the other code paths failing a LDAP operation do that already, and somebody looking at this code in the future may miss what LDAP expects with the search message. Author: Zhihong Yu Discussion: https://postgr.es/m/CALNJ-vTf5Y+8RtzZ4GjOGE9qWVHZ8awfhnFYc_qGm8fMLUNRAg@mail.gmail.com
* Fix GetForeignKey*Triggers for self-referential FKsAlvaro Herrera2022-09-09
| | | | | | | | | | | | | | | | | | | | Because of inadequate filtering, the check triggers were confusing the search for action triggers in GetForeignKeyActionTriggers and vice-versa in GetForeignKeyCheckTriggers; this confusion results in seemingly random assertion failures, and can have real impact in non-asserting builds depending on catalog order. Change these functions so that they correctly ignore triggers that are not relevant to each side. To reduce the odds of further problems, do not break out of the searching loop in assertion builds. This break is likely to hide bugs; without it, we would have detected this bug immediately. This problem was introduced by f4566345cf40, so backpatch to 15 where that commit first appeared. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/20220908172029.sejft2ppckbo6oh5@awork3.anarazel.de Discussion: https://postgr.es/m/4104619.1662663056@sss.pgh.pa.us
* Bump minimum version of Flex to 2.5.35John Naylor2022-09-09
| | | | | | | | Since the retirement of some older buildfarm members, the oldest Flex that gets regular testing is 2.5.35. Reviewed by Andres Freund Discussion: https://www.postgresql.org/message-id/1097762.1662145681@sss.pgh.pa.us
* Bump minimum version of Bison to 2.3John Naylor2022-09-09
| | | | | | | | | | | | Since the retirement of some older buildfarm members, the oldest Bison that gets regular testing is 2.3. MacOS ships that version, and will continue doing so for the forseeable future because of Apple's policy regarding GPLv3. While Mac users could use a package manager to install a newer version, there is no compelling reason to force them do so at this time. Reviewed by Andres Freund Discussion: https://www.postgresql.org/message-id/1097762.1662145681@sss.pgh.pa.us
* Add jsonpath_gram.h to list of distprep targetsJohn Naylor2022-09-09
| | | | Oversight in dac048f71e
* Replace load of functions by direct calls for some WIN32Michael Paquier2022-09-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit changes the following code paths to do direct system calls to some WIN32 functions rather than loading them from an external library, shaving some code in the process: - Creation of restricted tokens in pg_ctl.c, introduced by a25cd81. - QuerySecurityContextToken() in auth.c for SSPI authentication in the backend, introduced in d602592. - CreateRestrictedToken() in src/common/. This change is similar to the case of pg_ctl.c. Most of these functions were loaded rather than directly called because, as mentioned in the code comments, MinGW headers were not declaring them. I have double-checked the recent MinGW code, and all the functions changed here are declared in its headers, so this change should be safe. Note that I do not have a MinGW environment at hand so I have not tested it directly, but that MSVC was fine with the change. The buildfarm will tell soon enough if this change is appropriate or not for a much broader set of environments. A few code paths still use GetProcAddress() to load some functions: - LDAP authentication for ldap_start_tls_sA(), where I am not confident that this change would work. - win32env.c and win32ntdll.c where we have a per-MSVC version dependency for the name of the library loaded. - crashdump.c for MiniDumpWriteDump() and EnumDirTree(), where direct calls were not able to work after testing. Reported-by: Thomas Munro Reviewed-by: Justin Prysby Discussion: https://postgr.es/m/CA+hUKG+BMdcaCe=P-EjMoLTCr3zrrzqbcVE=8h5LyNsSVHKXZA@mail.gmail.com
* Add more error context to RestoreBlockImage() and consume itMichael Paquier2022-09-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | On failure in restoring a block image, no details were provided, while it is possible to see failure with an inconsistent record state, a failure in processing decompression or a failure in decompression because a build does not support this option. RestoreBlockImage() is used in two code paths in the backend code, during recovery and when checking a page consistency after applying masking, and both places are changed to consume the error message produced by the internal routine when it returns a false status. All the error messages are reported under ERRCODE_INTERNAL_ERROR, that gets used also when attempting to access a page compressed by a method not supported by the build attempting the decompression. This is something that can happen in core when doing physical replication with primary and standby using inconsistent build options, for example. This routine is available since 2c03216d and it has never provided any context about the error happening when it failed. This change is justified even more after 57aa5b2, that introduced compression of FPWs in WAL. Reported-by: Justin Prysby Author: Michael Paquier Discussion: https://postgr.es/m/20220905002320.GD31833@telsasoft.com Backpatch-through: 15
* Instrument freezing in autovacuum log reports.Peter Geoghegan2022-09-08
| | | | | | | | | | | Add a new line to log reports from autovacuum (as well as VACUUM VERBOSE output) that shows information about freezing. Emphasis is placed on the total number of heap pages that had one or more tuples frozen by VACUUM. The total number of tuples frozen is also shown. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Jeff Janes <jeff.janes@gmail.com> Discussion: https://postgr.es/m/CAH2-WznTY6D0zyE8VLrC6Gd4kh_HGAXxnTPtcOQOOsxzLx9zog@mail.gmail.com
* Temporarily make MemoryContextContains return falseDavid Rowley2022-09-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 5265e91fd changed MemoryContextContains to update it so that it works correctly with the new MemoryChunk code added in c6e0fe1f2. However, 5265e91fd was done with the assumption that MemoryContextContains would only ever be given pointers to memory that had been returned by one of our MemoryContext allocators. It seems that's not true and many of our 32-bit buildfarm animals are clearly showing that. There are some code paths that call MemoryContextContains with a pointer pointing part way into an allocated chunk. The example of this found by the 32-bit buildfarm animals is the int2int4_sum() function. This function returns transdata->sum, which is not a pointer to memory that was allocated directly. This return value is then subsequently passed to MemoryContextContains which causes it to crash due to it thinking the memory directly prior to that pointer is a MemoryChunk. What's actually in that memory is the field in the struct that comes prior to the "sum" field. This problem didn't occur in 64-bit world because BIGINT is a byval type and the code which was calling MemoryContextContains with the bad pointer only does so with non-byval types. Here, instead of reverting 5265e91fd and making MemoryContextContains completely broken again, let's just make it always return false for now. Effectively prior to 5265e91fd it was doing that anyway, this at least makes that more explicit. The only repercussions of this with the current MemoryContextContains calls are that we perform a datumCopy() when we might not need to. This should make the 32-bit buildfarm animals happy again and give us more time to consider a long-term fix. Discussion: https://postgr.es/m/20220907130552.sfjri7jublfxyyi4%40jrouhaud
* Choose FK name correctly during partition attachmentAlvaro Herrera2022-09-08
| | | | | | | | | | | | | | | During ALTER TABLE ATTACH PARTITION, if the name of a parent's foreign key constraint is already used on the partition, the code tries to choose another one before the FK attributes list has been populated, so the resulting constraint name was "<relname>__fkey" instead of "<relname>_<attrs>_fkey". Repair, and add a test case. Backpatch to 12. In 11, the code to attach a partition was not smart enough to cope with conflicting constraint names, so the problem doesn't exist there. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20220901184156.738ebee5@karst
* Fix recovery_prefetch with low maintenance_io_concurrency.Thomas Munro2022-09-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | We should process completed IOs *before* trying to start more, so that it is always possible to decode one more record when the decoded record queue is empty, even if maintenance_io_concurrency is set so low that a single earlier WAL record might have saturated the IO queue. That bug was hidden because the effect of maintenance_io_concurrency was arbitrarily clamped to be at least 2. Fix the ordering, and also remove that clamp. We need a special case for 0, which is now treated the same as recovery_prefetch=off, but otherwise the number is used directly. This allows for testing with 1, which would have made the problem obvious in simple test scenarios. Also add an explicit error message for missing contrecords. It was a bit strange that we didn't report an error already, and became a latent bug with prefetching, since the internal state that tracks aborted contrecords would not survive retrying, as revealed by 026_overwrite_contrecord.pl with this adjustment. Reporting an error prevents that. Back-patch to 15. Reported-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20220831140128.GS31833%40telsasoft.com
* Fix perltidy breaking perlcriticAlvaro Herrera2022-09-08
| | | | | | | | perltidying a "##no critic" line moves the marker to where it becomes useless. Put the line back to how it was, and protect it from further malfeasance. Per buildfarm member crake.
* Run perltidy over Catalog.pmJohn Naylor2022-09-08
| | | | | | | | | Commit 69eb643b2 deliberately left indentation unchanged to make the changes more legible. Rather than waiting until next year's perltidy run, do it now to avoid confusion Per suggestion from Álvaro Herrera Discussion: https://www.postgresql.org/message-id/20220907083558.vfvb5hcauaictgum%40alvherre.pgsql
* Parse catalog .dat files as a whole when compiling the backendJohn Naylor2022-09-08
| | | | | | | | | | | | Previously Catalog.pm eval'd each individual hash reference so that comments and whitespace can be preserved when running reformat-dat-files. This is unnecessary when building, and we can save ~15% off the run time of genbki.pl by simply slurping and eval'-ing the whole file at once. This saves a bit of time, especially in highly parallel builds, since most build targets depend on this script's outputs. Report and review by Andres Freund Discussion: https://www.postgresql.org/message-id/CAFBsxsGW%3DWRbnxXrc8UqqR479XuxtukSFWV-hnmtgsbuNAUO6w%40mail.gmail.com
* Raise a warning if there is a possibility of data from multiple origins.Amit Kapila2022-09-08
| | | | | | | | | | | | | | | This commit raises a warning message for a combination of options ('copy_data = true' and 'origin = none') during CREATE/ALTER subscription operations if the publication tables were also replicated from other publishers. During replication, we can skip the data from other origins as we have that information in WAL but that is not possible during initial sync so we raise a warning if there is such a possibility. Author: Vignesh C Reviewed-By: Peter Smith, Amit Kapila, Jonathan Katz, Shi yu, Wang wei Discussion: https://www.postgresql.org/message-id/CALDaNm0gwjY_4HFxvvty01BOT01q_fJLKQ3pWP9=9orqubhjcQ@mail.gmail.com
* Message style fixesAlvaro Herrera2022-09-07
|
* Make MemoryContextContains work correctly againDavid Rowley2022-09-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | c6e0fe1f2 recently changed the way we store headers for allocated chunks of memory. Prior to that commit, we stored a pointer to the owning MemoryContext directly prior to the pointer to the allocated memory. That's no longer true and c6e0fe1f2 neglected to update MemoryContextContains() so that it correctly obtains the owning context with the new method. A side effect of this change and c6e0fe1f2, in general, is that it's even less safe than it was previously to pass MemoryContextContains() an arbitrary pointer which was not allocated by one of our MemoryContexts. Previously some comments in MemoryContextContains() seemed to indicate that the worst that could happen by passing an arbitrary pointer would be a false positive return value. It seems to me that this was a rather wishful outlook as we subsequently proceeded to subtract sizeof(void *) from the given pointer and then dereferenced that memory. So it seems quite likely that we could have segfaulted instead of returning a false positive. However, it's not impossible that the memory sizeof(void *) bytes before the pointer could have been owned by the process, but it's far less likely to work now as obtaining a pointer to the owning MemoryContext is less direct than before c6e0fe1f2 and will access memory that's possibly much further away to obtain the owning MemoryContext. Because of this, I took the liberty of updating the comment to warn against any future usages of the function and checked the existing core usages to ensure that we only ever pass in a pointer to memory allocated by a MemoryContext. Extension authors updating their code for PG16 who are using MemoryContextContains should check to ensure that only NULL pointers and pointers to chunks allocated with a MemoryContext will ever be passed to MemoryContextContains. Reported-by: Andres Freund Discussion: https://postgr.es/m/20220905230949.kb3x2fkpfwtngz43@awork3.anarazel.de
* Make more effort to put a sentinel at the end of allocated memoryDavid Rowley2022-09-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Traditionally, in MEMORY_CONTEXT_CHECKING builds, we only ever marked a sentinel byte just beyond the requested size if there happened to be enough space on the chunk to do so. For Slab and Generation context types, we only rounded the size of the chunk up to the next maxalign boundary, so it was often not that likely that those would ever have space for the sentinel given that the majority of allocation requests are going to be for sizes which are maxaligned. For AllocSet, it was a little different as smaller allocations are rounded up to the next power-of-2 value rather than the next maxalign boundary, so we're a bit more likely to have space for the sentinel byte, especially when we get away from tiny sized allocations such as 8 or 16 bytes. Here we make more of an effort to allow space so that there is enough room for the sentinel byte in more cases. This makes it more likely that we'll detect when buggy code accidentally writes beyond the end of any of its memory allocations. Each of the 3 MemoryContext types has been changed as follows: The Slab allocator will now always set a sentinel byte. Both the current usages of this MemoryContext type happen to use chunk sizes which were on the maxalign boundary, so these never used sentinel bytes previously. For the Generation allocator, we now always ensure there's enough space in the allocation for a sentinel byte. For AllocSet, this commit makes an adjustment for allocation sizes which are greater than allocChunkLimit. We now ensure there is always space for a sentinel byte. We don't alter the sentinel behavior for request sizes <= allocChunkLimit. Making way for the sentinel byte for power-of-2 request sizes would require doubling up to the next power of 2. Some analysis done on the request sizes made during installcheck shows that a fairly large portion of allocation requests are for power-of-2 sizes. The amount of additional memory for the sentinel there seems prohibitive, so we do nothing for those here. Author: David Rowley Discussion: https://postgr.es/m/3478405.1661824539@sss.pgh.pa.us
* Fix new pg_publication_tables query.Tom Lane2022-09-06
| | | | | | | | | | | | | | | | The addition of published column names forgot to filter on attisdropped, leading to cases where you could see "........pg.dropped.1........" or the like as a reportedly-published column. While we're here, rewrite the new subquery to get a more efficient plan for it. Hou Zhijie, per report from Jaime Casanova. Back-patch to v15 where the bug was introduced. (Sadly, this means we need a post-beta4 catversion bump before beta4 has even hit the streets. I see no good alternative though.) Discussion: https://postgr.es/m/Yxa1SU4nH2HfN3/i@ahch-to
* Fix failure to maintainer-clean jsonpath_gram.hJohn Naylor2022-09-06
| | | | Oversight in dac048f71e
* Fix typo in 16d69ec29David Rowley2022-09-06
| | | | | | | As noted by Justin Pryzby, just I forgot to commit locally before creating a patch file. Discussion: https://postgr.es/m/20220901053146.GI31833@telsasoft.com
* Remove buggy and dead code from CreateTriggerFiringOnDavid Rowley2022-09-06
| | | | | | | | | | | | | | | | | | | | | | | | Here we remove some dead code from CreateTriggerFiringOn() which was attempting to find the relevant child partition index corresponding to the given indexOid. As it turned out, thanks to -Wshadow=compatible-local, this code was buggy as the code which was finding the child indexes assigned those to a shadowed variable that directly went out of scope. The code which thought it was looking at the List of child indexes was always referencing an empty List. On further investigation, this code is dead. We never call CreateTriggerFiringOn() passing a valid indexOid in a way that the function would actually ever execute the code in question. So, for lack of a way to test if a fix actually works, let's just remove the dead code instead. As a reminder, if there is ever a need to resurrect this code, an Assert() has been added to remind future feature developers that they might need to write some code to find the corresponding child index. Reported-by: Justin Pryzby Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/20220819211824.GX26426@telsasoft.com
* Fix an assortment of improper usages of string functionsDavid Rowley2022-09-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | In a similar effort to f736e188c and 110d81728, fixup various usages of string functions where a more appropriate function is available and more fit for purpose. These changes include: 1. Use cstring_to_text_with_len() instead of cstring_to_text() when working with a StringInfoData and the length can easily be obtained. 2. Use appendStringInfoString() instead of appendStringInfo() when no formatting is required. 3. Use pstrdup(...) instead of psprintf("%s", ...) 4. Use pstrdup(...) instead of psprintf(...) (with no formatting) 5. Use appendPQExpBufferChar() instead of appendPQExpBufferStr() when the length of the string being appended is 1. 6. appendStringInfoChar() instead of appendStringInfo() when no formatting is required and string is 1 char long. 7. Use appendPQExpBufferStr(b, .) instead of appendPQExpBuffer(b, "%s", .) 8. Don't use pstrdup when it's fine to just point to the string constant. I (David) did find other cases of #8 but opted to use #4 instead as I wasn't certain enough that applying #8 was ok (e.g in hba.c) Author: Ranier Vilela, David Rowley Discussion: https://postgr.es/m/CAApHDvo2j2+RJBGhNtUz6BxabWWh2Jx16wMUMWKUjv70Ver1vg@mail.gmail.com
* Fix incorrect uses of Datum conversion macrosPeter Eisentraut2022-09-05
| | | | | | | | | | | Since these macros just cast whatever you give them to the designated output type, and many normal uses also cast the output type further, a number of incorrect uses go undiscovered. The fixes in this patch have been discovered by changing these macros to inline functions, which is the subject of a future patch. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Discussion: https://www.postgresql.org/message-id/flat/8528fb7e-0aa2-6b54-85fb-0c0886dbd6ed%40enterprisedb.com
* Build all Flex files standaloneJohn Naylor2022-09-04
| | | | | | | | | | | | | The proposed Meson build system will need a way to ignore certain generated files in order to coexist with the autoconf build system, and C files generated by Flex which are #include'd into .y files make this more difficult. In similar vein to 72b1e3a21, arrange for all Flex C files to compile to their own .o targets. Reviewed by Andres Freund Discussion: https://www.postgresql.org/message-id/20220810171935.7k5zgnjwqzalzmtm%40awork3.anarazel.de Discussion: https://www.postgresql.org/message-id/CAFBsxsF8Gc2StS3haXofshHCzqNMRXiSxvQEYGwnFsTmsdwNeg@mail.gmail.com
* Move private declarations shared between guc.c and guc-file.l to new headerJohn Naylor2022-09-04
| | | | | | | | Further preparatory refactoring for compiling guc-file.c standalone. Reviewed by Andres Freund Discussion: https://www.postgresql.org/message-id/20220810171935.7k5zgnjwqzalzmtm%40awork3.anarazel.de Discussion: https://www.postgresql.org/message-id/CAFBsxsF8Gc2StS3haXofshHCzqNMRXiSxvQEYGwnFsTmsdwNeg@mail.gmail.com
* Preparatory refactoring for compiling guc-file.c standaloneJohn Naylor2022-09-04
| | | | | | | | | Mostly this involves moving ProcessConfigFileInternal() to guc.c and fixing the shared API to match. Reviewed by Andres Freund Discussion: https://www.postgresql.org/message-id/20220810171935.7k5zgnjwqzalzmtm%40awork3.anarazel.de Discussion: https://www.postgresql.org/message-id/CAFBsxsF8Gc2StS3haXofshHCzqNMRXiSxvQEYGwnFsTmsdwNeg@mail.gmail.com
* Fix cache invalidation bug in recovery_prefetch.Thomas Munro2022-09-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | XLogPageRead() can retry internally after a pread() system call has succeeded, in the case of short reads, and page validation failures while in standby mode (see commit 0668719801). Due to an oversight in commit 3f1ce973, these cases could leave stale data in the internal cache of xlogreader.c without marking it invalid. The main defense against stale cached data on failure to read a page was in the error handling path of the calling function ReadPageInternal(), but that wasn't quite enough for errors handled internally by XLogPageRead()'s retry loop if we then exited with XLREAD_WOULDBLOCK. 1. ReadPageInternal() now marks the cache invalid before calling the page_read callback, by setting state->readLen to 0. It'll be set to a non-zero value only after a successful read. It'll stay valid as long as the caller requests data in the cached range. 2. XLogPageRead() no long performs internal retries while reading ahead. While such retries should work, the general philosophy is that we should give up prefetching if anything unusual happens so we can handle it when recovery catches up, to reduce the complexity of the system. Let's do that here too. 3. While here, a new function XLogReaderResetError() improves the separation between xlogrecovery.c and xlogreader.c, where the former previously clobbered the latter's internal error buffer directly. The new function makes this more explicit, and also clears a related flag, without which a standby would needlessly retry in the outer function. Thanks to Noah Misch for tracking down the conditions required for a rare build farm failure in src/bin/pg_ctl/t/003_promote.pl, and providing a reproducer. Back-patch to 15. Reported-by: Noah Misch <noah@leadboat.com> Discussion: https://postgr.es/m/20220807003627.GA4168930%40rfd.leadboat.com
* Fix planner to consider matches to boolean columns in extension indexes.Tom Lane2022-09-02
| | | | | | | | | | | | | | | | | | | | | | | | | | The planner has to special-case indexes on boolean columns, because what we need for an indexscan on such a column is a qual of the shape of "boolvar = pseudoconstant". For plain bool constants, previous simplification will have reduced this to "boolvar" or "NOT boolvar", and we have to reverse that if we want to make an indexqual. There is existing code to do so, but it only fires when the index's opfamily is BOOL_BTREE_FAM_OID or BOOL_HASH_FAM_OID. Thus extension AMs, or extension opclasses such as contrib/btree_gin, are out in the cold. The reason for hard-wiring the set of relevant opfamilies was mostly to avoid a catalog lookup in a hot code path. We can improve matters while not taking much of a performance hit by relying on the hard-wired set when the opfamily OID is visibly built-in, and only checking the catalogs when dealing with an extension opfamily. While here, rename IsBooleanOpfamily to IsBuiltinBooleanOpfamily to remind future users of that macro of its limitations. At some point we might want to make indxpath.c's improved version of the test globally accessible, but it's not presently needed elsewhere. Zongliang Quan and Tom Lane Discussion: https://postgr.es/m/f293b91d-1d46-d386-b6bb-4b06ff5c667b@yeah.net
* Expand the use of get_dirent_type(), shaving a few calls to stat()/lstat()Michael Paquier2022-09-02
| | | | | | | | | | | | | | | | | | | Several backend-side loops scanning one or more directories with ReadDir() (WAL segment recycle/removal in xlog.c, backend-side directory copy, temporary file removal, configuration file parsing, some logical decoding logic and some pgtz stuff) already know the type of the entry being scanned thanks to the dirent structure associated to the entry, on platforms where we know about DT_REG, DT_DIR and DT_LNK to make the difference between a regular file, a directory and a symbolic link. Relying on the direct structure of an entry saves a few system calls to stat() and lstat() in the loops updated here, shaving some code while on it. The logic of the code remains the same, calling stat() or lstat() depending on if it is necessary to look through symlinks. Authors: Nathan Bossart, Bharath Rupireddy Reviewed-by: Andres Freund, Thomas Munro, Michael Paquier Discussion: https://postgr.es/m/CALj2ACV8n-J-f=yiLUOx2=HrQGPSOZM3nWzyQQvLPcccPXxEdg@mail.gmail.com
* Revert SQL/JSON featuresAndrew Dunstan2022-09-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The reverts the following and makes some associated cleanups: commit f79b803dc: Common SQL/JSON clauses commit f4fb45d15: SQL/JSON constructors commit 5f0adec25: Make STRING an unreserved_keyword. commit 33a377608: IS JSON predicate commit 1a36bc9db: SQL/JSON query functions commit 606948b05: SQL JSON functions commit 49082c2cc: RETURNING clause for JSON() and JSON_SCALAR() commit 4e34747c8: JSON_TABLE commit fadb48b00: PLAN clauses for JSON_TABLE commit 2ef6f11b0: Reduce running time of jsonb_sqljson test commit 14d3f24fa: Further improve jsonb_sqljson parallel test commit a6baa4bad: Documentation for SQL/JSON features commit b46bcf7a4: Improve readability of SQL/JSON documentation. commit 112fdb352: Fix finalization for json_objectagg and friends commit fcdb35c32: Fix transformJsonBehavior commit 4cd8717af: Improve a couple of sql/json error messages commit f7a605f63: Small cleanups in SQL/JSON code commit 9c3d25e17: Fix JSON_OBJECTAGG uniquefying bug commit a79153b7a: Claim SQL standard compliance for SQL/JSON features commit a1e7616d6: Rework SQL/JSON documentation commit 8d9f9634e: Fix errors in copyfuncs/equalfuncs support for JSON node types. commit 3c633f32b: Only allow returning string types or bytea from json_serialize commit 67b26703b: expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size. The release notes are also adjusted. Backpatch to release 15. Discussion: https://postgr.es/m/40d2c882-bcac-19a9-754d-4299e1d87ac7@postgresql.org
* Be smarter about freeing tuples during tuplesortsDavid Rowley2022-09-01
| | | | | | | | | | | | | | | | | | | | | | During dumptuples() the call to writetuple() would pfree any non-null tuple. This was quite wasteful as this happens just before we perform a reset of the context which stores all of those tuples. It seems to make sense to do a bit of a code refactor to make this work, so here we just get rid of the writetuple function and adjust the WRITETUP macro to call the state's writetup function. The WRITETUP usage in mergeonerun() always has state->slabAllocatorUsed == true, so writetuple() would never free the tuple or do any memory accounting. The only call path that needs memory accounting done is in dumptuples(), so let's just do it manually there. In passing, let's get rid of the state->memtupcount-- code that counts the memtupcount down to 0 one tuple at a time inside the loop. That seems to be a rather inefficient way to set memtupcount to 0, so let's just zero it after the loop instead. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvqZXoDCyrfCzZJR0-xH+7_q+GgitcQiYXUjRani7h4j8Q@mail.gmail.com
* Prevent long-term memory leakage in autovacuum launcher.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | get_database_list() failed to restore the caller's memory context, instead leaving current context set to TopMemoryContext which is how CommitTransactionCommand() leaves it. The callers both think they are using short-lived contexts, for the express purpose of not having to worry about cleaning up individual allocations. The net effect therefore is that supposedly short-lived allocations could accumulate indefinitely in the launcher's TopMemoryContext. Although this has been broken for a long time, it seems we didn't have any obvious memory leak here until v15's rearrangement of the stats logic. I (tgl) am not entirely convinced that there's no other leak at all, though, and we're surely at risk of adding one in future back-patched fixes. So back-patch to all supported branches, even though this may be only a latent bug in pre-v15. Reid Thompson Discussion: https://postgr.es/m/972a4e12b68b0f96db514777a150ceef7dcd2e0f.camel@crunchydata.com
* Derive freeze cutoff from nextXID, not OldestXmin.Peter Geoghegan2022-08-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before now, the cutoffs that VACUUM used to determine which XIDs/MXIDs to freeze were determined at the start of each VACUUM by taking related cutoffs that represent which XIDs/MXIDs VACUUM should treat as still running, and subtracting an XID/MXID age based value controlled by GUCs like vacuum_freeze_min_age. The FreezeLimit cutoff (XID freeze cutoff) was derived by subtracting an XID age value from OldestXmin, while the MultiXactCutoff cutoff (MXID freeze cutoff) was derived by subtracting an MXID age value from OldestMxact. This approach didn't match the approach used nearby to determine whether this VACUUM operation should be an aggressive VACUUM or not. VACUUM now uses the standard approach instead: it subtracts the same age-based values from next XID/next MXID (rather than subtracting from OldestXmin/OldestMxact). This approach is simpler and more uniform. Most of the time it will have only a negligible impact on how and when VACUUM freezes. It will occasionally make VACUUM more robust in the event of problems caused by long running transaction. These are cases where OldestXmin and OldestMxact are held back by so much that they attain an age that is a significant fraction of the value of age-based settings like vacuum_freeze_min_age. There is no principled reason why freezing should be affected in any way by the presence of a long-running transaction -- at least not before the point that the OldestXmin and OldestMxact limits used by each VACUUM operation attain an age that makes it unsafe to freeze some of the XIDs/MXIDs whose age exceeds the value of the relevant age-based settings. The new approach should at least make freezing degrade more gracefully than before, even in the most extreme cases. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Nathan Bossart <nathandbossart@gmail.com> Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAH2-WzkOv5CEeyOO=c91XnT5WBR_0gii0Wn5UbZhJ=4TTykDYg@mail.gmail.com
* In the Snowball dictionary, don't try to stem excessively-long words.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | If the input word exceeds 1000 bytes, don't pass it to the stemmer; just return it as-is after case folding. Such an input is surely not a word in any human language, so whatever the stemmer might do to it would be pretty dubious in the first place. Adding this restriction protects us against a known recursion-to-stack-overflow problem in the Turkish stemmer, and it seems like good insurance against any other safety or performance issues that may exist in the Snowball stemmers. (I note, for example, that they contain no CHECK_FOR_INTERRUPTS calls, so we really don't want them running for a long time.) The threshold of 1000 bytes is arbitrary. An alternative definition could have been to treat such words as stopwords, but that seems like a bigger break from the old behavior. Per report from Egor Chindyaskin and Alexander Lakhin. Thanks to Olly Betts for the recommendation to fix it this way. Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* Fix a bug in roles_is_member_of.Robert Haas2022-08-31
| | | | | | | | | | | | | | Commit e3ce2de09d814f8770b2e3b3c152b7671bcdb83f rearranged this function to be able to identify which inherited role had admin option on the target role, but it got the order of operations wrong, causing the function to return wrong answers in the presence of non-inherited grants. Fix that, and add a test case that verifies the correct behavior. Patch by me, reviewed by Nathan Bossart Discussion: http://postgr.es/m/CA+TgmoYamnu-xt-u7CqjYWnRiJ6BQaSpYOHXP=r4QGTfd1N_EA@mail.gmail.com
* Drop replication origin slots before tablesync worker exits.Amit Kapila2022-08-30
| | | | | | | | | | | | | | | | | | | Currently, the replication origin tracking of the tablesync worker is dropped by the apply worker. So, there will be a small lag between the tablesync worker exit and its origin tracking got removed. In the meantime, new tablesync workers can be launched and will try to set up a new origin tracking. This can lead the system to reach max configured limit (max_replication_slots) even if the user has configured the max limit considering the number of tablesync workers required in the system. We decided not to back-patch as this can occur in very narrow circumstances and users have to option to increase the configured limit by increasing max_replication_slots. Reported-by: Hubert Depesz Lubaczewski Author: Ajin Cherian Reviwed-by: Masahiko Sawada, Peter Smith, Hou Zhijie, Amit Kapila Discussion: https://postgr.es/m/20220714115155.GA5439@depesz.com
* Adjust comments that called MultiXactIds "XMIDs".Peter Geoghegan2022-08-29
| | | | Oversights in commits 0b018fab and f3c15cbe.
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-04 20:27:19 +0000