aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Make getObjectDescription robust against dangling amproc type links.Tom Lane2024-12-07
| | | | | | | | | | | | | | | | | | | Yoran Heling reported a case where a data type could be dropped while references to its OID remain behind in pg_amproc. This causes getObjectDescription to fail, which blocks dropping the operator family (since our DROP code likes to construct descriptions of everything it's dropping). The proper fix for this requires adding more pg_depend entries. But to allow DROP to go through with already-corrupt catalogs, tweak getObjectDescription to print "???" for the type instead of failing when it processes such an entry. I changed the logic for pg_amop similarly, for consistency, although it is not known that the problem can manifest in pg_amop. Per report from Yoran Heling. Back-patch to all supported branches (although the problem may be unreachable in v13). Discussion: https://postgr.es/m/Z1MVCOh1hprjK5Sf@gmai021
* Fix is_digit labeling of to_timestamp's FFn format codes.Tom Lane2024-12-07
| | | | | | | | | | | | | | | | These format codes produce or consume strings of digits, so they should be labeled with is_digit = true, but they were not. This has effect in only one place, where is_next_separator() is checked to see if the preceding format code should slurp up all the available digits. Thus, with a format such as '...SSFF3' with remaining input '12345', the 'SS' code would consume all five digits (and then complain about seconds being out of range) when it should eat only two digits. Per report from Nick Davies. This bug goes back to d589f9446 where the FFn codes were introduced, so back-patch to v13. Discussion: https://postgr.es/m/AM8PR08MB6356AC979252CFEA78B56678B6312@AM8PR08MB6356.eurprd08.prod.outlook.com
* Comment fix: "buffer context lock" to "buffer content lock".Jeff Davis2024-12-06
| | | | The term "buffer context lock" is outdated as of commit 5d5087363d.
* Remove useless casts to (const void *)Peter Eisentraut2024-12-06
| | | | | Similar to commit 7f798aca1d5, but I didn't think to look for "const" as well.
* Fix printf format string warning on MinGW.Thomas Munro2024-12-06
| | | | | | | | | | | | Commit 517bf2d91 changed a printf format string to placate MinGW, which at the time warned about "%lld". Current MinGW is now warning about the replacement "%I64d". Reverting the change clears the warning on the MinGW CI task, and hopefully it will clear it on build farm animal fairywren too. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com> Discussion: https://postgr.es/m/TYAPR01MB5866A71B744BE01B3BF71791F5AEA%40TYAPR01MB5866.jpnprd01.prod.outlook.com
* Remove pg_regex_collationPeter Eisentraut2024-12-05
| | | | | | | | We can also use the existing pg_regex_locale as the cache key, which is the only use of this variable. Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://www.postgresql.org/message-id/flat/b1b92ae1-2e06-4619-a87a-4b4858e547ec%40eisentraut.org
* Fix header inclusion order in c.h.Thomas Munro2024-12-05
| | | | | | | | | | | | | | | | | Commit 962da900a added #include <stdint.h> to postgres_ext.h, which broke c.h's header ordering rule. The system headers on some systems would then lock down off_t's size in private macros, before they'd had a chance to see our definition of _FILE_OFFSET_BITS (and presumably other things). This was picked up by perl's ABI compatibility checks on some 32 bit systems in the build farm. Move #include "postgres_ext.h" down below the system header section, and make the comments clearer (thanks to Tom for the new wording). Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/2397643.1733347237%40sss.pgh.pa.us
* Provide a better error message for misplaced dispatch options.Nathan Bossart2024-12-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before this patch, misplacing a special must-be-first option for dispatching to a subprogram (e.g., postgres -D . --single) would fail with an error like FATAL: --single requires a value This patch adjusts this error to more accurately complain that the special option wasn't listed first. The aforementioned error message now looks like FATAL: --single must be first argument The dispatch option parsing code has been refactored for use wherever ParseLongOption() is called. Beyond the obvious advantage of avoiding code duplication, this should prevent similar problems when new dispatch options are added. Note that we assume that none of the dispatch option names match another valid command-line argument, such as the name of a configuration parameter. Ideally, we'd remove this must-be-first requirement for these options, but after some investigation, we decided that wasn't worth the added complexity and behavior changes. Author: Nathan Bossart, Greg Sabino Mullane Reviewed-by: Greg Sabino Mullane, Peter Eisentraut, Álvaro Herrera, Tom Lane Discussion: https://postgr.es/m/CAKAnmmJkZtZAiSryho%3DgYpbvC7H-HNjEDAh16F3SoC9LPu8rqQ%40mail.gmail.com
* Fix dead codePeter Eisentraut2024-12-04
| | | | | | from commit 85b7efa1cdd per Coverity report
* Fix use-after-free in parallel_vacuum_reset_dead_itemsJohn Naylor2024-12-04
| | | | | | | | | | | | | | | | | | | | | | parallel_vacuum_reset_dead_items used a local variable to hold a pointer from the passed vacrel, purely as a shorthand. This pointer was later freed and a new allocation was made and stored to the struct. Then the local pointer was mistakenly referenced again. This apparently happened not to break anything since the freed chunk would have been put on the context's freelist, so it was accidentally the same pointer anyway, in which case the DSA handle was correctly updated. The minimal fix is to change two places so they access dead_items through the vacrel. This coding style is a maintenance hazard, so while at it get rid of most other similar usages, which were inconsistently used anyway. Analysis and patch by Vallimaharajan G, with further defensive coding by me Backpath to v17, when TidStore came in Discussion: https://postgr.es/m/1936493cc38.68cb2ef27266.7456585136086197135@zohocorp.com
* Simplify IsIndexUsableForReplicaIdentityFull()Peter Eisentraut2024-12-04
| | | | | | | | | | Take Relation as argument instead of IndexInfo. Building the IndexInfo is an unnecessary intermediate step here. A future patch wants to get some information that is in the relcache but not in IndexInfo, so this will also help there. Discussion: https://www.postgresql.org/message-id/333d3886-b737-45c3-93f4-594c96bb405d@eisentraut.org
* Ensure stored generated columns must be published when required.Amit Kapila2024-12-04
| | | | | | | | | | | | | | | | | | | | | | | Ensure stored generated columns that are part of REPLICA IDENTITY must be published explicitly for UPDATE and DELETE operations to be published. We can publish generated columns by listing them in the column list or by enabling the publish_generated_columns option. This commit changes the behavior of the test added in commit adedf54e65 by giving an ERROR for the UPDATE operation in such cases. There is no way to trigger the bug reported in commit adedf54e65 but we didn't remove the corresponding code change because it is still relevant when replicating changes from a publisher with version less than 18. We decided not to backpatch this behavior change to avoid the risk of breaking existing output plugins that may be sending generated columns by default although we are not aware of any such plugin. Also, we didn't see any reports related to this on STABLE branches which is another reason not to backpatch this change. Author: Shlok Kyal, Hou Zhijie Reviewed-by: Vignesh C, Amit Kapila Discussion: https://postgr.es/m/CANhcyEVw4V2Awe2AB6i0E5AJLNdASShGfdBLbUd1XtWDboymCA@mail.gmail.com
* Use <stdint.h> and <inttypes.h> for c.h integers.Thomas Munro2024-12-04
| | | | | | | | | | | | | | | | | | Redefine our exact width types with standard C99 types and macros, including int64_t, INT64_MAX, INT64_C(), PRId64 etc. We were already using <stdint.h> types in a few places. One complication is that Windows' <inttypes.h> uses format strings like "%I64d", "%I32", "%I" for PRI*64, PRI*32, PTR*PTR, instead of mapping to other standardized format strings like "%lld" etc as seen on other known systems. Teach our snprintf.c to understand them. This removes a lot of configure clutter, and should also allow 64-bit numbers and other standard types to be used in localized messages without casting. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/ME3P282MB3166F9D1F71F787929C0C7E7B6312%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
* Move check for ucol_strcollUTF8 to pg_locale_icu.cJeff Davis2024-12-03
| | | | | | | The result of the check is only used by pg_locale_icu.c. Author: Andreas Karlsson Discussion: https://postgr.es/m/4548a168-62cd-457b-8d06-9ba7b985c477@proxel.se
* Fix synchronized_standby_slots GUC check hookÁlvaro Herrera2024-12-03
| | | | | | | | | | | | | | | | | The validate_sync_standby_slots subroutine requires an LWLock, so it cannot run in processes without PGPROC; skip it there to avoid a crash. This replaces the current test for ReplicationSlotCtl being not null, which appears to be a solution for the same problem but less general. I also rewrote a related comment that mentioned ReplicationSlotCtl in StandbySlotsHaveCaughtup. This code came in with commit bf279ddd1c28; backpatch to 17. Reported-by: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/202411281216.sutbxtr6idnn@alvherre.pgsql
* Drop "Lock" suffix from LWLock wait event namesÁlvaro Herrera2024-12-03
| | | | | | | | | | | | | Commit da952b415f44 unintentially reverted the SQL-visible part of commit 14a910109126, which breaks queries joining pg_wait_events with pg_stat_acivity. Remove the suffix again. Backpatch to 17. Reported-by: Christophe Courtois <christophe.courtois@dalibo.com> Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/18728-450924477056a339%40postgresql.org Discussion: https://postgr.es/m/Z01w1+LihtRiS0Te@ip-10-97-1-34.eu-west-3.compute.internal
* Update obsolete commentÁlvaro Herrera2024-12-03
| | | | | Commit 3aa0395d4ed3 made worrying about BKI_ROWTYPE_OID matching no longer necessary, but this comment didn't get the memo.
* Fix handling of CREATE DOMAIN with GENERATED constraint syntaxPeter Eisentraut2024-12-03
| | | | | | | | | | | | | | | | | | | | Stuff like CREATE DOMAIN foo AS int CONSTRAINT cc GENERATED ALWAYS AS (2) STORED is not supported for domains, but the parser allows it, because it's the same syntax as for table constraints. But CreateDomain() did not explicitly handle all ConstrType values, so the above would get an internal error like ERROR: unrecognized constraint subtype: 4 Fix that by providing a user-facing error message for all ConstrType values. Also, remove the switch default case, so future additions to ConstrType are caught. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/CACJufxF8fmM=Dbm4pDFuV_nKGz2-No0k4YifhrF3-rjXTWJM3w@mail.gmail.com
* Fix temporary memory leak in system table index scansPeter Eisentraut2024-12-03
| | | | | | | | | | | | | | | | Commit 811af9786b introduced palloc() calls into systable_beginscan() and systable_beginscan_ordered(). But there was no pfree(), as is the usual style. It turns out that an ANALYZE of a partitioned table can invoke many thousand system table index scans, and this memory is not cleaned up until the end of the command, so this can temporarily leak quite a bit of memory. Maybe there are improvements to be made at a higher level about this, but for now, insert a couple of corresponding pfree() calls to fix this particular issue. Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://www.postgresql.org/message-id/Z0XTfIq5xUtbkiIh@pryzbyj2023
* Perform provider-specific initialization in new functions.Jeff Davis2024-12-02
| | | | | Reviewed-by: Andreas Karlsson Discussion: https://postgr.es/m/4548a168-62cd-457b-8d06-9ba7b985c477@proxel.se
* Fix unintentional behavior change in commit e9931bfb75.Jeff Davis2024-12-02
| | | | | | | | | Prior to that commit, there was special case to use ASCII case mapping behavior for the libc provider with a single-byte encoding when that's the default collation. Commit e9931bfb75 mistakenly eliminated that special case; this commit restores it. Discussion: https://postgr.es/m/01a104f0d2179d756261e90d96fd65c36ad6fcf0.camel@j-davis.com
* Revert "Introduce CompactAttribute array in TupleDesc"David Rowley2024-12-03
| | | | | | | | | | This reverts commit d28dff3f6cd6a7562fb2c211ac0fb74a33ffd032. Quite a large number of buildfarm members didn't like this commit and it's not yet clear why. Reverting this before too many animals turn red. Discussion: https://postgr.es/m/CAApHDvr9i6T5=iAwQCxFDgMsthr_obVxgwBaEJkC8KUH6yM3Hw@mail.gmail.com
* Introduce CompactAttribute array in TupleDescDavid Rowley2024-12-03
| | | | | | | | | | | | | | | | | | | | | | | | | | The new compact_attrs array stores a few select fields from FormData_pg_attribute in a more compact way, using only 16 bytes per column instead of the 104 bytes that FormData_pg_attribute uses. Using CompactAttribute allows performance-critical operations such as tuple deformation to be performed without looking at the FormData_pg_attribute element in TupleDesc which means fewer cacheline accesses. With this change, NAMEDATALEN could be increased with a much smaller negative impact on performance. For some workloads, tuple deformation can be the most CPU intensive part of processing the query. Some testing with 16 columns on a table where the first column is variable length showed around a 10% increase in transactions per second for an OLAP type query performing aggregation on the 16th column. However, in certain cases, the increases were much higher, up to ~25% on one AMD Zen4 machine. This also makes pg_attribute.attcacheoff redundant. A follow-on commit will remove it, thus shrinking the FormData_pg_attribute struct by 4 bytes. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvrBztXP3yx=NKNmo3xwFAFhEdyPnvrDg3=M0RhDs+4vYw@mail.gmail.com Reviewed-by: Andres Freund, Victor Yegorov
* Rework some code handling pg_subscription data in psql and pg_dumpMichael Paquier2024-12-03
| | | | | | | | | | | | | | | | | | | | | | | | | This commit fixes some inconsistencies found in the frontend code when dealing with subscription catalog data. The following changes are done: - pg_subscription.h gains a EXPOSE_TO_CLIENT_CODE, so as more content defined in pg_subscription.h becomes available in pg_subscription_d.h for the frontend. - In psql's describe.c, substream can be switched to use CppAsString2() with its three LOGICALREP_STREAM_* values, with pg_subscription_d.h included. - pg_dump.c included pg_subscription.h, which is a header that should only be used in the backend code. The code is updated to use pg_subscription_d.h instead. - pg_dump stored all the data from pg_subscription in SubscriptionInfo with only strings, and a good chunk of them are boolean and char values. Using strings is not necessary, complicates the code (see for example two_phase_disabled[] removed here), and is inconsistent with the way other catalogs' data is handled. The fields of SubscriptionInfo are reordered to match with the order in its catalog, while on it. Reviewed-by: Hayato Kuroda Discussion: https://postgr.es/m/Z0lB2kp0ksHgmVuk@paquier.xyz
* RelationTruncate() must set DELAY_CHKPT_START.Thomas Munro2024-12-03
| | | | | | | | | | | | | | | | | | | | | | Previously, it set only DELAY_CHKPT_COMPLETE. That was important, because it meant that if the XLOG_SMGR_TRUNCATE record preceded a XLOG_CHECKPOINT_ONLINE record in the WAL, then the truncation would also happen on disk before the XLOG_CHECKPOINT_ONLINE record was written. However, it didn't guarantee that the sync request for the truncation was processed before the XLOG_CHECKPOINT_ONLINE record was written. By setting DELAY_CHKPT_START, we guarantee that if an XLOG_SMGR_TRUNCATE record is written to WAL before the redo pointer of a concurrent checkpoint, the sync request queued by that operation must be processed by that checkpoint, rather than being left for the following one. This is a refinement of commit 412ad7a5563. Back-patch to all supported releases, like that commit. Author: Robert Haas <robertmhaas@gmail.com> Reported-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2B-2rjGZC2kwqr2NMLBcEBp4uf59QT1advbWYF_uc%2B0Aw%40mail.gmail.com
* Deprecate MD5 passwords.Nathan Bossart2024-12-02
| | | | | | | | | | | | | | | | | | | MD5 has been considered to be unsuitable for use as a cryptographic hash algorithm for some time. Furthermore, MD5 password hashes in PostgreSQL are vulnerable to pass-the-hash attacks, i.e., knowing the username and hashed password is sufficient to authenticate. The SCRAM-SHA-256 method added in v10 is not subject to these problems and is considered to be superior to MD5. This commit marks MD5 password support in PostgreSQL as deprecated and to be removed in a future release. The documentation now contains several deprecation notices, and CREATE ROLE and ALTER ROLE now emit deprecation warnings when setting MD5 passwords. The warnings can be disabled by setting the md5_password_warnings parameter to "off". Reviewed-by: Greg Sabino Mullane, Jim Nasby Discussion: https://postgr.es/m/ZwbfpJJol7lDWajL%40nathan
* Add a planner support function for numeric generate_series().Dean Rasheed2024-12-02
| | | | | | | | | | | This allows the planner to estimate the number of rows returned by generate_series(numeric, numeric[, numeric]), when the input values can be estimated at plan time. Song Jinzhou, reviewed by Dean Rasheed and David Rowley. Discussion: https://postgr.es/m/tencent_F43E7F4DD50EF5986D1051DE8DE547910206%40qq.com Discussion: https://postgr.es/m/tencent_1F6D5B9A1545E02FD7D0EE508DFD056DE50A%40qq.com
* Fix #include order in timestamp.c.Dean Rasheed2024-12-02
| | | | Oversight in 036bdcec9f.
* Fix error code for referential action RESTRICTPeter Eisentraut2024-12-02
| | | | | | | | | | | | According to the SQL standard, if the referential action RESTRICT is triggered, it has its own error code. We previously didn't use that, we just used the error code for foreign key violation. But RESTRICT is not necessarily an actual foreign key violation. The foreign key might still be satisfied in theory afterwards, but the RESTRICT setting prevents the action even then. So it's a separate kind of error condition. Discussion: https://www.postgresql.org/message-id/ea5b2777-266a-46fa-852f-6fca6ec480ad@eisentraut.org
* Fix broken list-munging in ecpg's remove_variables().Tom Lane2024-12-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The loops over cursor argument variables neglected to ever advance "prevvar". The code would accidentally do the right thing anyway when removing the first or second list entry, but if it had to remove the third or later entry then it would also remove all entries between there and the first entry. AFAICS this would only matter for cursors that reference out-of-scope variables, which is a weird Informix compatibility hack; between that and the lack of impact for short lists, it's not so surprising that nobody has complained. Nonetheless it's a pretty obvious bug. It would have been more obvious if these loops used a more standard coding style for chasing the linked lists --- this business with the "prev" pointer sometimes pointing at the current list entry is confusing and overcomplicated. So rather than just add a minimal band-aid, I chose to rewrite the loops in the same style we use elsewhere, where the "prev" pointer is NULL until we are dealing with a non-first entry and we save the "next" pointer at the top of the loop. (Two of the four loops touched here are not actually buggy, but it seems better to make them all look alike.) Coverity discovered this problem, but not until 2b41de4a5 added code to free no-longer-needed arguments structs. With that, the incorrect link updates are possibly touching freed memory, and it complained about that. Nonetheless the list corruption hazard is ancient, so back-patch to all supported branches.
* Avoid mislabeling of lateral references, redux.Tom Lane2024-11-30
| | | | | | | | | | | | | | | | | | As I'd feared, commit 5c9d8636d was still a few bricks shy of a load. We can't just leave pulled-up lateral-reference Vars with no new nullingrels: we have to carefully compute what subset of the to-be-replaced Var's nullingrels apply to them, else we still get "wrong varnullingrels" errors. This is a bit tedious, but it looks like we can use the nullingrel data this patch computes for other purposes, enabling better optimization. We don't want to inject unnecessary plan changes into stable branches though, so leave that idea for a later HEAD-only patch. Patch by me, but thanks to Richard Guo for devising a test case that broke 5c9d8636d, and for preliminary investigation about how to fix it. As before, back-patch to v16. Discussion: https://postgr.es/m/E1tGn4j-0003zi-MP@gemulon.postgresql.org
* Small indenting fixes in jsonpath_scan.lPeter Eisentraut2024-11-29
| | | | | | Some lines were indented by an inconsistent number of spaces. While we're here, also fix some code that used the newline after left parenthesis style, which is obsolete.
* Add tests for foreign keys with case-insensitive collationsPeter Eisentraut2024-11-29
| | | | | | | | | | | | | Some of the behaviors of the different referential actions, such as the difference between NO ACTION and RESTRICT are best illustrated using a case-insensitive collation. So add some tests for that. (What is actually being tested here is the behavior with values that are "distinct" (binary different) but compare as equal. Another way to do that would be with positive and negative zeroes with float types. But this way seems nicer and more flexible.) Discussion: https://www.postgresql.org/message-id/ea5b2777-266a-46fa-852f-6fca6ec480ad@eisentraut.org
* Skip not SOAP-supported indexes while transforming an OR clause into SAOPAlexander Korotkov2024-11-29
| | | | | | | | | | | | There is no point in transforming OR-clauses into SAOP's if the target index doesn't support SAOP scans anyway. This commit adds corresponding checks to match_orclause_to_indexcol() and group_similar_or_args(). The first check fixes the actual bug, while the second just saves some cycles. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/8174de69-9e1a-0827-0e81-ef97f56a5939%40gmail.com Author: Alena Rybakina Reviewed-by: Ranier Vilela, Alexander Korotkov, Andrei Lepikhov
* Fix typo in header comment for set_operation_ordered_results_usefulDavid Rowley2024-11-29
| | | | | Reported-by: Richard Guo Discussion: https://postgr.es/m/CAMbWs492vMy3XNjDZRtqtHfFTK6HVeDwhrEQH7eXGgF_h5Jnzw@mail.gmail.com
* psql: Sprinkle more CppAsString2() in describe.cMichael Paquier2024-11-29
| | | | | | | | | | | | | | | | | | | Like 91f5a4a000ea for pg_amcheck, this makes the code more self-documented as there is less need to look in the headers what a hardcoded value means. This touches queries related to procedures, AMs, functions, databases, relations, constraints, collations, types and extended stats, pulling into psql their *_d.h headers. The queries are written the same way as originally. There are still a couple of hardcoded values. These cannot be included yet as they are not exposed in headers that are safe to use in frontend code. Note that describe.c was including pg_am.h that should be used only in backend code. This is updated to use pg_am_d.h. Reviewed-by: Daniel Gustafsson, Corey Huinker Discussion: https://postgr.es/m/Zxb2hpca-pZc6zKe@paquier.xyz
* Avoid mislabeling of lateral references when pulling up a subquery.Tom Lane2024-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we are pulling up a subquery that's under an outer join, and the subquery's target list contains a strict expression that uses both a subquery variable and a lateral-reference variable, it's okay to pull up the expression without wrapping it in a PlaceHolderVar. That's safe because if the subquery variable is forced to NULL by the outer join, the expression result will come out as NULL too, so we don't have to force that outcome by evaluating the expression below the outer join. It'd be correct to wrap in a PHV, but that can lead to very significantly worse plans, since we'd then have to use a nestloop plan to pass down the lateral reference to where the expression will be evaluated. However, when we do that, we should not mark the lateral reference variable as being nulled by the outer join, because it isn't after we pull up the expression in this way. So the marking logic added by cb8e50a4a was incorrect in this detail, leading to "wrong varnullingrels" errors from the consistency-checking logic in setrefs.c. It seems to be sufficient to just not mark lateral references at all in this case. (I have a nagging feeling that more complexity may be needed in cases where there are several levels of outer join, but some attempts to break it with that didn't succeed.) Per report from Bertrand Mamasam. Back-patch to v16, as the previous patch was. Discussion: https://postgr.es/m/CACZ67_UA_EVrqiFXJu9XK50baEpH=ofEPJswa2kFxg6xuSw-ww@mail.gmail.com
* Fix wording in commentDaniel Gustafsson2024-11-28
| | | | | | Author: Peter Smith <smithpb2250@gmail.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Discussion: https://postgr.es/m/CAHut+PvE+2T2etdTaHi3n+xbCG_UYrshQuCbaAdJCFPpQGLwgQ@mail.gmail.com
* psql: Add tab completion for COPY (MERGE ...Peter Eisentraut2024-11-28
| | | | | | | The underlying feature for this was added in PostgreSQL 17. Author: Jian He <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/CACJufxEmNjxvf1deR1zBrJbjAMeCooooLRzZ+yaaBuqDKh_6-Q@mail.gmail.com
* Remove useless casts to (void *)Peter Eisentraut2024-11-28
| | | | | | | | Many of them just seem to have been copied around for no real reason. Their presence causes (small) risks of hiding actual type mismatches or silently discarding qualifiers Discussion: https://www.postgresql.org/message-id/flat/461ea37c-8b58-43b4-9736-52884e862820@eisentraut.org
* Require sizeof(bool) == 1.Thomas Munro2024-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The C standard says that sizeof(bool) is implementation-defined, but we know of no current systems where it is not 1. The last known systems seem to have been Apple macOS/PowerPC 10.5 and Microsoft Visual C++ 4, both long defunct. PostgreSQL has always required sizeof(bool) == 1 for the definition of bool that it used, but previously it would define its own type if the system-provided bool had a different size. That was liable to cause memory layout problems when interacting with system and third-party libraries on (by now hypothetical) computers with wider _Bool, and now C23 has introduced a new problem by making bool a built-in datatype (like C++), so the fallback code doesn't even compile. We could probably work around that, but then we'd be writing new untested code for a computer that doesn't exist. Instead, delete the unreachable and C23-uncompilable fallback code, and let existing static assertions fail if the system-provided bool is too wide. If we ever get a problem report from a real system, then it will be time to figure out what to do about it in a way that also works on modern compilers. Note on C++: Previously we avoided including <stdbool.h> or trying to define a new bool type in headers that might be included by C++ code. These days we might as well just include <stdbool.h> unconditionally: it should be visible to C++11 but do nothing, just as in C23. We already include <stdint.h> without C++ guards in c.h, and that falls under the same C99-compatibility section of the C++11 standard as <stdbool.h>, so let's remove the guards here too. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3198438.1731895163%40sss.pgh.pa.us
* Use __attribute__((target(...))) for SSE4.2 CRC-32C support.Nathan Bossart2024-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Presently, we check for compiler support for the required intrinsics both with and without the -msse4.2 compiler flag, and then depending on the results of those checks, we pick which files to compile with which flags. This is tedious and complicated, and it results in unsustainable coding patterns such as separate files for each portion of code that may need to be built with different compiler flags. This commit makes use of the newly-added support for __attribute__((target(...))) in the SSE4.2 CRC-32C code. This simplifies both the configure-time checks and the build scripts, and it allows us to place the functions that use the intrinsics in files that we otherwise do not want to build with special CPU instructions (although this commit refrains from doing so). This is also preparatory work for a proposed follow-up commit that will further optimize the CRC-32C code with AVX-512 instructions. While at it, this commit modifies meson's checks for SSE4.2 CRC support to be the same as autoconf's. meson was choosing whether to use a runtime check based purely on whether -msse4.2 is required, while autoconf has long checked for the __SSE4_2__ preprocessor symbol to decide. meson's previous approach seems to work just fine, but this change avoids needing to build multiple test programs and to keep track of whether to actually use pg_attribute_target(). Ideally we'd use __attribute__((target(...))) for ARMv8 CRC support, too, but there's little point in doing so because until clang 16, using the ARM intrinsics still requires special compiler flags. Perhaps we can re-evaluate this decision after some time has passed. Author: Raghuveer Devulapalli Discussion: https://postgr.es/m/PH8PR11MB8286BE735A463468415D46B5FB5C2%40PH8PR11MB8286.namprd11.prod.outlook.com
* Make GUC_check_errdetail messages full sentencesÁlvaro Herrera2024-11-27
| | | | | | | They were all missing punctuation, one was missing initial capital. Per our message style guidelines. No backpatch, to avoid breaking existing translations.
* Remove redundant relam initializationÁlvaro Herrera2024-11-27
| | | | | | | | This struct member is initialized again a few lines below in the same function. This is cosmetic, so no backpatch. Reported-by: Jingtang Zhang <mrdrivingduck@gmail.com> Discussion: https://postgr.es/m/AFF74506-B925-46BB-B875-CF5A946170EB@gmail.com
* ecpg: clean up some other assorted memory leaks.Tom Lane2024-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | | Avoid leaking the prior value when updating the "connection" state variable. Ditto for ECPGstruct_sizeof. (It seems like this one ought to be statement-local, but testing says it isn't, and I didn't feel like diving deeper.) The actual_type[] entries are statement-local, though, so no need to mm_strdup() strings stored in them. Likewise, sqlda variables are statement-local, so we can loc_alloc them. Also clean up sloppiness around management of the argsinsert and argsresult lists. progname changes are strictly to prevent valgrind from complaining about leaked allocations. With this, valgrind reports zero leakage in the ecpg preprocessor for all of our ecpg regression test cases. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
* ecpg: put all string-valued tokens returned by pgc.l in local storage.Tom Lane2024-11-27
| | | | | | | | | This didn't work earlier in the patch series (I think some of the strings were ending up in data-type-related structures), but apparently we're now clean enough for it. This considerably reduces process-lifespan memory leakage. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
* ecpg: fix some memory leakage of data-type-related structures.Tom Lane2024-11-27
| | | | | | | | | | | | | | | | ECPGfree_type() and related functions were quite incomplete about removing subsidiary data structures. Possibly this is because ecpg wasn't careful to make sure said data structures always had their own storage. Previous patches in this series cleaned up a lot of that, and I had to add a couple more mm_strdup's here. Also, ecpg.trailer tended to overwrite struct_member_list[struct_level] without bothering to free up its previous contents, thus potentially leaking a lot of struct-member-related storage. Add ECPGfree_struct_member() calls at appropriate points. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
* jsonapi: add lexer option to keep token ownershipAndrew Dunstan2024-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit 0785d1b8b adds support for libpq as a JSON client, but allocations for string tokens can still be leaked during parsing failures. This is tricky to fix for the object_field semantic callbacks: the field name must remain valid until the end of the object, but if a parsing error is encountered partway through, object_field_end() won't be invoked and the client won't get a chance to free the field name. This patch adds a flag to switch the ownership of parsed tokens to the lexer. When this is enabled, the client must make a copy of any tokens it wants to persist past the callback lifetime, but the lexer will handle necessary cleanup on failure. Backend uses of the JSON parser don't need to use this flag, since the parser's allocations will occur in a short lived memory context. A -o option has been added to test_json_parser_incremental to exercise the new setJsonLexContextOwnsTokens() API, and the test_json_parser TAP tests make use of it. (The test program now cleans up allocated memory, so that tests can be usefully run under leak sanitizers.) Author: Jacob Champion Discussion: https://postgr.es/m/CAOYmi+kb38EciwyBQOf9peApKGwraHqA7pgzBkvoUnw5BRfS1g@mail.gmail.com
* ci: Fix cached MacPorts installation managementAndres Freund2024-11-27
| | | | | | | | | | | | | | | | | | | 1. The error reporting of "port setrequested list-of-packages..." changed, hiding errors we were relying on to know if all packages in our list were already installed. Use a loop instead. 2. The cached MacPorts installation was shared between PostgreSQL major branches, though each branch wanted different packages. Add the list of packages to cache key, so that different branches, when tested in one github account/repo such as postgres/postgres, stop fighting with each other, adding and removing packages. Back-patch to 15 where CI began. Author: Thomas Munro <thomas.munro@gmail.com> Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Suggested-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/au2uqfuy2nf43nwy2txmc5t2emhwij7kzupygto3d2ffgtrdgr%40ckvrlwyflnh2
* Look up backend type in pg_signal_backend() more cheaply.Nathan Bossart2024-11-27
| | | | | | | | | | | | | | | | | | Commit ccd38024bc, which introduced the pg_signal_autovacuum_worker role, added a call to pgstat_get_beentry_by_proc_number() for the purpose of determining whether the process is an autovacuum worker. This function calls pgstat_read_current_status(), which can be fairly expensive and may return cached, out-of-date information. Since we just need to look up the target backend's BackendType, and we already know its ProcNumber, we can instead inspect the BackendStatusArray directly, which is much less expensive and possibly more up-to-date. There are some caveats with this approach (which are documented in the code), but it's still substantially better than before. Reported-by: Andres Freund Reviewed-by: Andres Freund Discussion: https://postgr.es/m/ujenaa2uabzfkwxwmfifawzdozh3ljr7geozlhftsuosgm7n7q%40g3utqqyyosb6