aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Eliminate fixed token-length limit in hba.c.Tom Lane2023-07-27
| | | | | | | | | | | | | | | | | | | Historically, hba.c limited tokens in the authentication configuration files (pg_hba.conf and pg_ident.conf) to less than 256 bytes. We have seen a few reports of this limit causing problems; notably, for moderately-complex LDAP configurations. Let's get rid of the fixed limit by using a StringInfo instead of a fixed-size buffer. This actually takes less code than before, since we can get rid of a nontrivial error recovery stanza. It's doubtless a hair slower, but parsing the content of the HBA files should in no way be performance-critical. Although this is a pretty straightforward patch, it doesn't seem worth the risk to back-patch given the small number of complaints to date. In released branches, we'll just raise MAX_TOKEN to ameliorate the problem. Discussion: https://postgr.es/m/1588937.1690221208@sss.pgh.pa.us
* worker_spi: Switch to TAP testsMichael Paquier2023-07-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit moves worker_spi to use TAP tests. sql/worker_spi.sql is gone, replaced by an equivalent set of queries in a TAP script, without worker_spi loaded in shared_preload_libraries: - One query to launch a worker dynamically, relying now on "postgres" as the default database to connect to. - Two wait queries with poll_query_until(), one to wait for the worker schema to be initialized and a second to wait for a tuple processed by the worker. - Server reload to accelerate the main loop of the spawned worker. More coverage is added for workers registered when the library is loaded with shared_preload_libraries, while on it, checking that these are connecting to the database set in the GUC worker_spi.database. A local run of these test is showing that TAP is slightly faster than the original, while providing more coverage (3.7s vs 4.4s). There was also some discussions about keeping the SQL tests, but this would require initializing twice a cluster, increasing the runtime of the tests up to 5.6s here. These tests will be expanded more in an upcoming patch aimed at adding support for custom wait events for the Extension class, still under discussion, to check the new in-core APIs with and without a library set in shared_preload_libraries. Bharath has written the part where shared_preload_libraries is used, while I have migrated the existing SQL tests to TAP. Author: Bharath Rupireddy, Michael Paquier Reviewed-by: Masahiro Ikeda Discussion: https://postgr.es/m/CALj2ACWR9ncAiDF73unqdJF1dmsA2R0efGXX2624X+YVxcAVWg@mail.gmail.com
* Fix performance problem with new COPY DEFAULT codeDavid Rowley2023-07-27
| | | | | | | | | | | | | | | | 9f8377f7a added code to allow COPY FROM insert a column's default value when the input matches the DEFAULT string specified in the COPY command. Here we fix some inefficient code which needlessly palloc0'd an array to store if we should use the default value or input value for the given column. This array was being palloc0'd and pfree'd once per row. It's much more efficient to allocate this once and just reset the values once per row. Reported-by: Masahiko Sawada Author: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoDvDmUQeJtZrau1ovnT_smN940%3DKp6mszNGK3bq9yRN6g%40mail.gmail.com Backpatch-through: 16, where 9f8377f7a was introduced.
* Add sanity asserts for index OID and attnums during cache initMichael Paquier2023-07-27
| | | | | | | | | | | | | | | | There was already a check on the relation OID, but not its index OID and the attributes that can be used during the syscache lookups. The two assertions added by this commit are cheap, and actually useful for developers to fasten the detection of incorrect data in a new entry added in the syscache list, as these assertions are triggered during the initial cache loading (initdb, or just backend startup), not requiring a syscache that uses the new entry. While on it, the relation OID check is switched to use OidIsValid(). Author: Aleksander Alekseev Reviewed-by: Dagfinn Ilmari Mannsåker, Zhang Mingli, Michael Paquier Discussion: https://postgr.es/m/CAJ7c6TOjUTJ0jxvWY6oJeP2-840OF8ch7qscZQsuVuotXTOS_g@mail.gmail.com
* Show savepoint names as constants in pg_stat_statementsMichael Paquier2023-07-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | In pg_stat_statements, savepoint names now show up as constants with a parameter symbol, using as base query string the one added as a new entry to the PGSS hash table, leading to: RELEASE $1 ROLLBACK TO $1 SAVEPOINT $1 Applying constants to these query parts is a huge advantage for workloads that generate randomly savepoint points, like ORMs (Django is at the origin of this patch). The ODBC driver is a second layer that likes a lot savepoints, though it does not use a random naming pattern. A "location" field is added to TransactionStmt, now set only for savepoints. The savepoint name is ignored by the query jumbling. The location can be extended to other query patterns, if required, like 2PC commands. Some tests are added to pg_stat_statements for all the query patterns supported by the parser. ROLLBACK, ROLLBACK TO SAVEPOINT and ROLLBACK TRANSACTION TO SAVEPOINT have the same Node representation, so all these are equivalents. The same happens for RELEASE and RELEASE SAVEPOINT. Author: Greg Sabino Mullane Discussion: https://postgr.es/m/CAKAnmm+2s9PA4OaumwMJReWHk8qvJ_-g1WqxDRDAN1BSUfxyTw@mail.gmail.com
* Adjust extra lines generated by psql to be valid SQL comments.Nathan Bossart2023-07-26
| | | | | | | | | | | | | | | psql's --echo-hidden, --log-file, and --single-step options generate extra lines to clearly separate queries from other output. Presently, these extra lines are not valid SQL comments, which makes them a hazard for anyone trying to copy/paste the decorated queries into a client or query editor. This commit replaces the starting and ending asterisks in these extra lines with forward slashes so that they are valid SQL comments that can be copy/pasted without incident. Author: Kirk Wolak Reviewed-by: Pavel Stehule, Laurenz Albe, Tom Lane, Alvaro Herrera, Andrey Borodin Discussion: https://postgr.es/m/CACLU5mTFJRJYtbvmZ26txGgmXWQo0hkGhH2o3hEquUPmSbGtBw%40mail.gmail.com
* Add more SQL/JSON constructor functionsAmit Langote2023-07-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This Patch introduces three SQL standard JSON functions: JSON() JSON_SCALAR() JSON_SERIALIZE() JSON() produces json values from text, bytea, json or jsonb values, and has facilitites for handling duplicate keys. JSON_SCALAR() produces a json value from any scalar sql value, including json and jsonb. JSON_SERIALIZE() produces text or bytea from input which containis or represents json or jsonb; For the most part these functions don't add any significant new capabilities, but they will be of use to users wanting standard compliant JSON handling. Catversion bumped as this changes ruleutils.c. Author: Nikita Glukhov <n.gluhov@postgrespro.ru> Author: Teodor Sigaev <teodor@sigaev.ru> Author: Oleg Bartunov <obartunov@gmail.com> Author: Alexander Korotkov <aekorotkov@gmail.com> Author: Andrew Dunstan <andrew@dunslane.net> Author: Amit Langote <amitlangote09@gmail.com> Reviewers have included (in no particular order) Andres Freund, Alexander Korotkov, Pavel Stehule, Andrew Alsup, Erik Rijkers, Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson, Justin Pryzby, Álvaro Herrera, Peter Eisentraut Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Rename a nonterminal used in SQL/JSON grammarAmit Langote2023-07-26
| | | | | | | | | | This renames json_output_clause_opt to json_returning_clause_opt, because the new name makes more sense given that the governing keyword is RETURNING. Per suggestion from Álvaro Herrera. Discussion: https://postgr.es/m/20230707122820.wy5zlmhn4tdzbojl%40alvherre.pgsql
* Some refactoring to export json(b) conversion functionsAmit Langote2023-07-26
| | | | | | | | | | | This is to export datum_to_json(), datum_to_jsonb(), and jsonb_from_cstring(), though the last one is exported as jsonb_from_text(). A subsequent commit to add new SQL/JSON constructor functions will need them for calling from the executor. Discussion: https://postgr.es/m/20230720160252.ldk7jy6jqclxfxkq%40alvherre.pgsql
* Fix crash with RemoveFromWaitQueue() when detecting a deadlock.Masahiko Sawada2023-07-26
| | | | | | | | | | | | | | | | | | Commit 5764f611e used dclist_delete_from() to remove the proc from the wait queue. However, since it doesn't clear dist_node's next/prev to NULL, it could call RemoveFromWaitQueue() twice: when the process detects a deadlock and then when cleaning up locks on aborting the transaction. The waiting lock information is cleared in the first call, so it led to a crash in the second call. Backpatch to v16, where the change was introduced. Bug: #18031 Reported-by: Justin Pryzby, Alexander Lakhin Reviewed-by: Andres Freund Discussion: https://postgr.es/m/ZKy4AdrLEfbqrxGJ%40telsasoft.com Discussion: https://postgr.es/m/18031-ebe2d08cb405f6cc@postgresql.org Backpatch-through: 16
* worker_spi: Use term "dynamic" for bgworkers launched with worker_spi_launch()Michael Paquier2023-07-26
| | | | | | | | | | | | This gives a way to make a difference between workers registered when the library is loaded with shared_preload_libraries and when these are launched dynamically, in ps output or pg_stat_activity. Extracted from a larger patch by the same author. Author: Bharath Rupireddy Reviewed-by: Masahiro Ikeda Discussion: https://postgr.es/m/CALj2ACWR9ncAiDF73unqdJF1dmsA2R0efGXX2624X+YVxcAVWg@mail.gmail.com
* Document more assumptions of LWLock variable changes with WAL insertsMichael Paquier2023-07-26
| | | | | | | | | | | | | | | | | | | | | | | | | This commit adds a few comments about what LWLockWaitForVar() relies on when a backend waits for a variable update on its LWLocks for WAL insertions up to an expected LSN. First, LWLockWaitForVar() does not include a memory barrier, relying on a spinlock taken at the beginning of WaitXLogInsertionsToFinish(). This was hidden behind two layers of routines in lwlock.c. This assumption is now documented at the top of LWLockWaitForVar(), and detailed at bit more within LWLockConflictsWithVar(). Second, document why WaitXLogInsertionsToFinish() does not include memory barriers, relying on a spinlock at its top, which is, per Andres' input, fine for two different reasons, both depending on the fact that the caller of WaitXLogInsertionsToFinish() is waiting for a LSN up to a certain value. This area's documentation and assumptions could be improved more in the future, but at least that's a beginning. Author: Bharath Rupireddy, Andres Freund Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CALj2ACVF+6jLvqKe6xhDzCCkr=rfd6upaGc3477Pji1Ke9G7Bg@mail.gmail.com
* Fix code indentation vioaltion introduced in commit d38ad8e31d.Amit Kapila2023-07-25
| | | | | | Per buildfarm member koel Discussion: https://postgr.es/m/ZL9bsGhthne6FaVV@paquier.xyz
* Remove unnecessary checks for indexes for REPLICA IDENTITY FULL tables.Masahiko Sawada2023-07-25
| | | | | | | | | | | | | | | | | | | | | | Previously, when selecting an usable index for update/delete for the REPLICA IDENTITY FULL table, in IsIndexOnlyExpression(), we used to check if all index fields are not expressions. However, it was not necessary, because it is enough to check if only the leftmost index field is not an expression (and references the remote table column) and this check has already been done by RemoteRelContainsLeftMostColumnOnIdx(). This commit removes IsIndexOnlyExpression() and RemoteRelContainsLeftMostColumnOnIdx() and all checks for usable indexes for REPLICA IDENTITY FULL tables are now performed by IsIndexUsableForReplicaIdentityFull(). Backpatch this to remain the code consistent. Reported-by: Peter Smith Reviewed-by: Amit Kapila, Önder Kalacı Discussion: https://postgr.es/m/CAHut%2BPsGRE5WSsY0jcLHJEoA17MrbP9yy8FxdjC_ZOAACxbt%2BQ%40mail.gmail.com Backpatch-through: 16
* Optimize WAL insertion lock acquisition and release with some atomicsMichael Paquier2023-07-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The WAL insertion lock variable insertingAt is currently being read and written with the help of the LWLock wait list lock to avoid any read of torn values. This wait list lock can become a point of contention on a highly concurrent write workloads. This commit switches insertingAt to a 64b atomic variable that provides torn-free reads/writes. On platforms without 64b atomic support, the fallback implementation uses spinlocks to provide the same guarantees for the values read. LWLockWaitForVar(), through LWLockConflictsWithVar(), reads the new value to check if it still needs to wait with a u64 atomic operation. LWLockUpdateVar() updates the variable before waking up the waiters with an exchange_u64 (full memory barrier). LWLockReleaseClearVar() now uses also an exchange_u64 to reset the variable. Before this commit, all these steps relied on LWLockWaitListLock() and LWLockWaitListUnlock(). This reduces contention on LWLock wait list lock and improves performance of highly-concurrent write workloads. Here are some numbers using pg_logical_emit_message() (HEAD at d6677b93) with various arbitrary record lengths and clients up to 1k on a rather-large machine (64 vCPUs, 512GB of RAM, 16 cores per sockets, 2 sockets), in terms of TPS numbers coming from pgbench: message_size_b | 16 | 64 | 256 | 1024 --------------------+--------+--------+--------+------- patch_4_clients | 83830 | 82929 | 80478 | 73131 patch_16_clients | 267655 | 264973 | 250566 | 213985 patch_64_clients | 380423 | 378318 | 356907 | 294248 patch_256_clients | 360915 | 354436 | 326209 | 263664 patch_512_clients | 332654 | 321199 | 287521 | 240128 patch_1024_clients | 288263 | 276614 | 258220 | 217063 patch_2048_clients | 252280 | 243558 | 230062 | 192429 patch_4096_clients | 212566 | 213654 | 205951 | 166955 head_4_clients | 83686 | 83766 | 81233 | 73749 head_16_clients | 266503 | 265546 | 249261 | 213645 head_64_clients | 366122 | 363462 | 341078 | 261707 head_256_clients | 132600 | 132573 | 134392 | 165799 head_512_clients | 118937 | 114332 | 116860 | 150672 head_1024_clients | 133546 | 115256 | 125236 | 151390 head_2048_clients | 137877 | 117802 | 120909 | 138165 head_4096_clients | 113440 | 115611 | 120635 | 114361 Bharath has been measuring similar improvements, where the limit of the WAL insertion lock begins to be felt when more than 256 concurrent clients are involved in this specific workload. An extra patch has been discussed to introduce a fast-exit path in LWLockUpdateVar() when there are no waiters, still this does not influence the write-heavy workload cases discussed as there are always waiters. This will be considered separately. Author: Bharath Rupireddy Reviewed-by: Nathan Bossart, Andres Freund, Michael Paquier Discussion: https://postgr.es/m/CALj2ACVF+6jLvqKe6xhDzCCkr=rfd6upaGc3477Pji1Ke9G7Bg@mail.gmail.com
* Fix the display of UNKNOWN message type in apply worker.Amit Kapila2023-07-25
| | | | | | | | | | | | | | We include the message type while displaying an error context in the apply worker. Now, while retrieving the message type string if the message type is unknown we throw an error that will hide the original error. So, instead, we need to simply return the string indicating an unknown message type. Reported-by: Ashutosh Bapat Author: Euler Taveira, Amit Kapila Reviewed-by: Ashutosh Bapat Backpatch-through: 15 Discussion: https://postgr.es/m/CAExHW5suAEDW-mBZt_qu4RVxWZ1vL54-L+ci2zreYWebpzxYsA@mail.gmail.com
* Fix off-by-one in LimitAdditionalPins()Andres Freund2023-07-24
| | | | | | | | | | | | | | | Due to the bug LimitAdditionalPins() could return 0, violating LimitAdditionalPins()'s API ("One additional pin is always allowed"). This could be hit when setting shared_buffers very low and using a fair amount of concurrency. This bug was introduced in 31966b151e6a. Author: "Anton A. Melnikov" <aamelnikov@inbox.ru> Reported-by: "Anton A. Melnikov" <aamelnikov@inbox.ru> Reported-by: Victoria Shepard Discussion: https://postgr.es/m/ae46f2fb-5586-3de0-b54b-1bb0f6410ebd@inbox.ru Backpatch: 16-
* Compare only major versions in AdjustUpgrade.pmAlvaro Herrera2023-07-24
| | | | | | | | | | | | | | | | | | | Because PostgreSQL::Version is very nuanced about development version numbers, the comparison to 16beta2 makes it think that that release is older than 16, therefore applying a database tweak that doesn't work there (the comparison is only supposed to match when run on version 15). As suggested by Andrew Dunstan, fix by having AdjustUpgrade.pm public methods create a separate PostgreSQL::Version object to use for these comparisons, that only carries the major version number. While at it, have the same methods ensure that the objects given are of the expected type. Backpatch to 16. This module goes all the way back to 9.2, but there's probably no need for this fix except where betas still live. Co-authored-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20230719110504.zbu74o54bqqlsufb@alvherre.pgsql
* pgbench: Use COPY for client-side data generationMichael Paquier2023-07-24
| | | | | | | | | | | | | | | | | | | | | | This commit switches the client-side data generation from INSERT queries to COPY for the two tables pgbench_branches and pgbench_tellers. pgbench_accounts was already using COPY. COPY is a better interface for bulk loading or high latency connections (this point can be countered with the option for server-side data generation, still client-side is the default), and measurements have proved that using it for these two other tables can lead to improvements during initialization. I did not notice slowdowns at large scale numbers on a local setup, either, most of the work happening for the accounts table. Previously COPY was only used for the pgbench_accounts table because the amount of data was much larger than the two other tables. The code is refactored so as all three tables use the same code path to execute the COPY queries, with a callback to build data rows. Author: Tristan Partin Discussion: https://postgr.es/m/CSTU5P82ONZ1.19XFUGHMXHBRY@c3po
* pgbench: Add TAP tests to check consistency of data generatedMichael Paquier2023-07-23
| | | | | | | | | | | | | | | | | | The tables created by pgbench rely on a few assumptions for TPC-B, where the "filler" attribute is used to comply with this benchmark's rules as well as pgbencn historical behavior. The data generated for each table uses this filler in a different way: - pgbench_accounts uses it as a blank-padded empty string. - pgbench_tellers and pgbench_branches use it as a NULL value. There were no checks done about the consistency of the data initialized, and this has showed up while discussing a patch that changes the logic in charge of the client-side data generation (pgbench documents all that already in its comments). This commit adds some checks on the data generated for both the server-side and client-side logic. Reviewed-by: Tristan Partin Discussion: https://postgr.es/m/ZLik4oKnqRmVCM3t@paquier.xyz
* Avoid compiler warning in non-assert builds.Tom Lane2023-07-22
| | | | | | | | | | | After 3c90dcd03, try_partitionwise_join's child_joinrelids variable is read only in an Assert, provoking a compiler warning in non-assert builds. Rearrange code to avoid the warning and eliminate unnecessary work in the non-assert case. Per CI testing (via Jeff Davis and Bharath Rupireddy) Discussion: https://postgr.es/m/ef0de9713e605451f1b60b30648c5ee900b2394c.camel@j-davis.com
* ICU: remove negative test that fails to fail.Jeff Davis2023-07-21
| | | | | | | | On OpenBSD, setlocale() does not fail on an ICU-specific locale. Remove the test. Reported-by: Andres Freund Discussion: https://postgr.es/m/20230702165615.k6waysygrefdeiiw@awork3.anarazel.de
* Fix calculation of relid sets for partitionwise child joins.Tom Lane2023-07-21
| | | | | | | | | | | | | | | | | | | Applying add_outer_joins_to_relids() to a child join doesn't actually work, even if we've built a SpecialJoinInfo specialized to the child, because that function will also compare the join's relids to elements of the main join_info_list, which only deal in regular relids not child relids. This mistake escaped detection by the existing partitionwise join tests because they didn't test any cases where add_outer_joins_to_relids() needs to add additional OJ relids (that is, any cases where join reordering per identity 3 is possible). Instead, let's apply adjust_child_relids() to the relids of the parent join. This requires minor code reordering to collect the relevant AppendRelInfo structures first, but that's work we'd do shortly anyway. Report and fix by Richard Guo; cosmetic changes by me Discussion: https://postgr.es/m/CAMbWs49NCNbyubZWgci3o=_OTY=snCfAPtMnM-32f3mm-K-Ckw@mail.gmail.com
* Code review for commit b6e1157e7dAmit Langote2023-07-21
| | | | | | | | | | | | b6e1157e7d made some changes to enforce that JsonValueExpr.formatted_expr is always set and is the expression that gives a JsonValueExpr its runtime value, but that's not really apparent from the comments about and the code manipulating formatted_expr. This commit fixes that. Per suggestion from Álvaro Herrera. Discussion: https://postgr.es/m/20230718155313.3wqg6encgt32adqb%40alvherre.pgsql
* Fix worker_spi when launching workers without shared_preload_librariesMichael Paquier2023-07-21
| | | | | | | | | | | | | | | | | | | | | Currently, the database name to connect is initialized only when the module is loaded with shared_preload_libraries, causing any call of worker_spi_launch() to fail if the library is not loaded for a dynamic bgworker launch. Rather than making the GUC defining the database to connect to a PGC_POSTMASTER, this commit switches worker_spi.database to PGC_SIGHUP, loaded even if the module's library is loaded dynamically for a worker. We have been discussing about the integration of more advanced tests in this module, with and without shared_preload_libraries set, so this eases a bit the work planned in this area. No backpatch is done as, while this is a bug, it changes the definition of worker_spi.database. Author: Masahiro Ikeda Reviewed-by: Bharath Rupireddy Discussion: https://postgr.es/m/d30d3ea7d21cb7c9e1e3cc47e301f1b6@oss.nttdata.com
* Guard against null plan pointer in CachedPlanIsSimplyValid().Tom Lane2023-07-20
| | | | | | | | | | | | | If both the passed-in plan pointer and plansource->gplan are NULL, CachedPlanIsSimplyValid would think that the plan pointer is possibly-valid and try to dereference it. For the one extant call site in plpgsql, this situation doesn't normally happen which is why we've not noticed. However, it appears to be possible if the previous use of the cached plan failed, as per report from Justin Pryzby. Add an extra check to prevent crashing. Back-patch to v13 where this code was added. Discussion: https://postgr.es/m/ZLlV+STFz1l/WhAQ@telsasoft.com
* Revert "Add notBefore and notAfter to SSL cert info display"Daniel Gustafsson2023-07-20
| | | | | | | Due to an oversight in reviewing, this used functionality not compatible with old versions of OpenSSL. This reverts commit 75ec5e7bec700577d39d653c316e3ae6c505842c.
* Add notBefore and notAfter to SSL cert info displayDaniel Gustafsson2023-07-20
| | | | | | | | | This adds the X509 attributes notBefore and notAfter to sslinfo as well as pg_stat_ssl to allow verifying and identifying the validity period of the current client certificate. Author: Cary Huang <cary.huang@highgo.ca> Discussion: https://postgr.es/m/182b8565486.10af1a86f158715.2387262617218380588@highgo.ca
* Set fixed dates for test certificates validityDaniel Gustafsson2023-07-20
| | | | | | | | | | | | | Rather than specifying a validity of 10 000 days into the future during test certificate generation, this hardcodes the notBefore and notAfter attributes to known values. This will allow writing tests on the validity of the certificates without knowing when a specific certificate was regenerated. This is done as a prerequisite for an upcoming patch which adds notBefore and notAfter to pg_stat_ssl and sslinfo. Discussion: https://postgr.es/m/EE288A58-947E-479A-9D99-C46C273D7A23@yesql.se
* pg_upgrade: include additional detail in cluster checkDaniel Gustafsson2023-07-20
| | | | | | | | | When the cluster failed the pg_controldata check for clean shut down we only reported that it did so, not why. The state of the cluster can be important information when diagnosing the failed upgrade attempt, so instead include it in the error message. Discussion: https://postgr.es/m/E0D5EA16-A085-4753-8DDC-C7055048B439@yesql.se
* Unify JSON categorize type API and export for external useAmit Langote2023-07-20
| | | | | | | | | | | | | | | | | | | | | | This essentially removes the JsonbTypeCategory enum and jsonb_categorize_type() and integrates any jsonb-specific logic that was in jsonb_categorize_type() into json_categorize_type(), now moved to jsonfuncs.c. The remaining JsonTypeCategory enum and json_categorize_type() cover the needs of the callers in both json.c and jsonb.c. json_categorize_type() has grown a new parameter named is_jsonb for callers to engage the jsonb-specific behavior of json_categorize_type(). One notable change in the now exported API of json_categorize_type() is that it now always returns *outfuncoid even though a caller may have no need currently to see one. This is in preparation of later commits to implement additional SQL/JSON functions. Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Add missing ObjectIdGetDatum() in syscache lookup calls for OidsMichael Paquier2023-07-20
| | | | | | | | | | Based on how postgres.h foes the Oid <-> Datum conversion, there is no existing bugs but let's be consistent. 17 spots have been noticed as incorrectly passing down Oids rather than Datums. Aleksander got one, Zhang two and I the rest. Author: Michael Paquier, Aleksander Alekseev, Zhang Mingli Discussion: https://postgr.es/m/ZLUhqsqQN1MOaxdw@paquier.xyz
* Fix pg_recvlogical upon signal terminationMichael Paquier2023-07-20
| | | | | | | | | | | | | | | | | | | | | | | | When pg_recvlogical needs to abort on a signal like SIGINT/SIGTERM, it is expected to exit cleanly as the code documents. However, the code forgot to clean up the state of the connection before leaving. This would cause the tool to emit messages like "unexpected termination of replication stream" error, which is meant for really unexpected termination or a crash. The code is refactored to apply the same termination abort operations for signals, end LSN and keepalive cases, registering a "reason" for the termination with a message printed under --verbose adapted to the reason used. This is arguably a bug, but this has been this way since the tool exists and the signal termination can now become slower depending on the change being decoded when the signal is received. Reported-by: Andres Freund Author: Bharath Rupireddy Reviewed-by: Andres Freund, Kyotaro Horiguchi, Cary Huang, Michael Paquier Discussion: https://postgr.es/m/20221019213953.htdtzikf4f45ywil@awork3.anarazel.de
* Support parenthesized syntax for CLUSTER without a table name.Nathan Bossart2023-07-19
| | | | | | | | | | | | | b5913f6120 added a parenthesized syntax for CLUSTER, but it requires specifying a table name. This is unlike commands such as VACUUM and ANALYZE, which do not require specifying a table in the parenthesized syntax. This change resolves this inconsistency. This is preparatory work for a follow-up commit that will move the unparenthesized syntax to the "Compatibility" section of the CLUSTER documentation. Reviewed-by: Melanie Plageman, Michael Paquier Discussion: https://postgr.es/m/CAAKRu_bc5uHieG1976kGqJKxyWtyQt9yvktjsVX%2Bi7NOigDjOA%40mail.gmail.com
* Rearrange CLUSTER rules in gram.y.Nathan Bossart2023-07-19
| | | | | | | | | | | This change moves the unparenthesized syntax for CLUSTER to the end of the ClusterStmt rules in preparation for a follow-up commit that will move this syntax to the "Compatibility" section of the CLUSTER documentation. The documentation for the CLUSTER syntaxes has also been consolidated. Suggested-by: Melanie Plageman Discussion https://postgr.es/m/CAAKRu_bc5uHieG1976kGqJKxyWtyQt9yvktjsVX%2Bi7NOigDjOA%40mail.gmail.com
* Add psql \drg command to display role grants.Tom Lane2023-07-19
| | | | | | | | | | | | | | | | | | | | | | | With the addition of INHERIT and SET options for role grants, the historical display of role memberships in \du/\dg is woefully inadequate. Besides those options, there are pre-existing shortcomings that you can't see the ADMIN option nor the grantor. To fix this, remove the "Member of" column from \du/\dg altogether (making that output usefully narrower), and invent a new meta-command "\drg" that is specifically for displaying role memberships. It shows one row for each role granted to the selected role(s), with the grant options and grantor. We would not normally back-patch such a feature addition post feature freeze, but in this case the change is mainly driven by v16 changes in the server, so it seems appropriate to include it in v16. Pavel Luzanov, with bikeshedding and review from a lot of people, but particularly David Johnston Discussion: https://postgr.es/m/b9be2d0e-a9bc-0a30-492f-a4f68e4f7740@postgrespro.ru
* pg_archivecleanup: Add --clean-backup-historyMichael Paquier2023-07-19
| | | | | | | | | | | | | | | | | | | By default, pg_archivecleanup does not remove backup history files. These are just few bytes useful for debugging purposes, still keeping them around can bloat an archive path history files mixed with the WAL segments if the path has a long history. This patch adds a new option to control if backup history files are removed, depending on the oldest segment name to keep around. While on it, the TAP tests are refactored so as these are now able to handle lists of files. Each file has a flag to track if it should still exist or not depending on the oldest segment defined with the command run. Author: Atsushi Torikoshi Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/d660ef741ce3d82f3b4283f1cafd576c@oss.nttdata.com
* pg_archivecleanup: Refactor loop doing old segment removalsMichael Paquier2023-07-19
| | | | | | | | | | | This commit refactors a bit the main loop of pg_archivecleanup that handles the removal of old segments, reducing by one its level of indentation. This will help an incoming patch that adds a new option related to the segment filtering logic. Author: Atsushi Torikoshi Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/d660ef741ce3d82f3b4283f1cafd576c@oss.nttdata.com
* Fix indentation in twophase.cMichael Paquier2023-07-18
| | | | | | | | | This has been missed in cb0cca1, noticed before buildfarm member koel has been able to complain while poking at a different patch. Like the other commit, backpatch all the way down to limit the odds of merge conflicts. Backpatch-through: 11
* Fix recovery of 2PC transaction during crash recoveryMichael Paquier2023-07-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A crash in the middle of a checkpoint with some two-phase state data already flushed to disk by this checkpoint could cause a follow-up crash recovery to recover twice the same transaction, once from what has been found in pg_twophase/ at the beginning of recovery and a second time when replaying its corresponding record. This would lead to FATAL failures in the startup process during recovery, where the same transaction would have a state recovered twice instead of once: LOG: recovering prepared transaction 731 from shared memory LOG: recovering prepared transaction 731 from shared memory FATAL: lock ExclusiveLock on object 731/0/0 is already held This issue is fixed by skipping the addition of any 2PC state coming from a record whose equivalent 2PC state file has already been loaded in TwoPhaseState at the beginning of recovery by restoreTwoPhaseData(), which is OK as long as the system has not reached a consistent state. The timing to get a messed up recovery processing is very racy, and would very unlikely happen. The thread that has reported the issue has demonstrated the bug using injection points to force a PANIC in the middle of a checkpoint. Issue introduced in 728bd99, so backpatch all the way down. Reported-by: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com> Author: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com> Author: Michael Paquier Discussion: https://postgr.es/m/109e6994-b971-48cb-84f6-829646f18b4c.mengjuan.cmj@alibaba-inc.com Backpatch-through: 11
* Include <limits.h> in fe-auth.c, to get CHAR_BIT reliably.Tom Lane2023-07-17
| | | | | | | | | | | | | fe-auth.c references CHAR_BIT since commit 3a465cc67, but it did not #include <limits.h>, which per POSIX is where that symbol is defined. This escaped notice so far because (a) on most platforms, <sys/param.h> pulls in <limits.h>, (b) even if yours doesn't, OpenSSL pulls it in, so compiling with --with-openssl masks the omission. Per bug #18026 from Marcel Hofstetter. Back-patch to v16. Discussion: https://postgr.es/m/18026-d5bb69f79cd16203@postgresql.org
* Remove db_user_namespace.Nathan Bossart2023-07-17
| | | | | | | | | | | This feature was intended to be a temporary measure to support per-database user names. A better one hasn't materialized in the ~21 years since it was added, and nobody claims to be using it, so let's just remove it. Reviewed-by: Michael Paquier, Magnus Hagander Discussion: https://postgr.es/m/20230630200509.GA2830328%40nathanxps13 Discussion: https://postgr.es/m/20230630215608.GD2941194%40nathanxps13
* Shrink memory contexts struct sizesDavid Rowley2023-07-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | Here we reduce the block size fields in AllocSetContext, GenerationContext and SlabContext from Size down to uint32. Ever since c6e0fe1f2, blocks for non-dedicated palloc chunks can no longer be larger than 1GB, so there's no need to store the various block size fields as 64-bit values. 32 bits are enough to store 2^30. Here we also further reduce the memory context struct sizes by getting rid of the 'keeper' field which stores a pointer to the context's keeper block. All the context types which have this field always allocate the keeper block in the same allocation as the memory context itself, so the keeper block always comes right at the end of the context struct. Add some macros to calculate that address rather than storing it in the context. Overall, in AllocSetContext and GenerationContext, this saves 20 bytes on 64-bit builds which for ALLOCSET_SMALL_SIZES can sometimes mean the difference between having to allocate a 2nd block and storing all the required allocations on the keeper block alone. Such contexts are used in relcache to store cache entries for indexes, of which there can be a large number in a single backend. Author: Melih Mutlu Reviewed-by: David Rowley Discussion: https://postgr.es/m/CAGPVpCSOW3uJ1QJmsMR9_oE3X7fG_z4q0AoU4R_w+2RzvroPFg@mail.gmail.com
* Simplify option handling in pg_ctl.Nathan Bossart2023-07-14
| | | | | | | | | Now that the in-tree getopt_long() moves non-options to the end of argv (see commit 411b720343), we can remove pg_ctl's workaround for getopt_long() implementations that don't reorder argv. Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20230713034903.GA991765%40nathanxps13
* Allow plan nodes with initPlans to be considered parallel-safe.Tom Lane2023-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the plan itself is parallel-safe, and the initPlans are too, there's no reason anymore to prevent the plan from being marked parallel-safe. That restriction (dating to commit ab77a5a45) was really a special case of the fact that we couldn't transmit subplans to parallel workers at all. We fixed that in commit 5e6d8d2bb and follow-ons, but this case never got addressed. We still forbid attaching initPlans to a Gather node that's inserted pursuant to debug_parallel_query = regress. That's because, when we hide the Gather from EXPLAIN output, we'd hide the initPlans too, causing cosmetic regression diffs. It seems inadvisable to kluge EXPLAIN to the extent required to make the output look the same, so just don't do it in that case. Along the way, this also takes care of some sloppiness about updating path costs to match when we move initplans from one place to another during createplan.c and setrefs.c. Since all the planning decisions are already made by that point, this is just cosmetic; but it seems good to keep EXPLAIN output consistent with where the initplans are. The diff in query_planner() might be worth remarking on. I found that one because after fixing things to allow parallel-safe initplans, one partition_prune test case changed plans (as shown in the patch) --- but only when debug_parallel_query was active. The reason proved to be that we only bothered to mark Result nodes as potentially parallel-safe when debug_parallel_query is on. This neglects the fact that parallel-safety may be of interest for a sub-query even though the Result itself doesn't parallelize. Discussion: https://postgr.es/m/1129530.1681317832@sss.pgh.pa.us
* Account for optimized MinMax aggregates during SS_finalize_plan.Tom Lane2023-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We are capable of optimizing MIN() and MAX() aggregates on indexed columns into subqueries that exploit the index, rather than the normal thing of scanning the whole table. When we do this, we replace the Aggref node(s) with Params referencing subquery outputs. Such Params really ought to be included in the per-plan-node extParam/allParam sets computed by SS_finalize_plan. However, we've never done so up to now because of an ancient implementation choice to perform that substitution during set_plan_references, which runs after SS_finalize_plan, so that SS_finalize_plan never sees these Params. This seems like clearly a bug, yet there have been no field reports of problems that could trace to it. This may be because the types of Plan nodes that could contain Aggrefs do not have any of the rescan optimizations that are controlled by extParam/allParam. Nonetheless it seems certain to bite us someday, so let's fix it in a self-contained patch that can be back-patched if we find a case in which there's a live bug pre-v17. The cleanest fix would be to perform a separate tree walk to do these substitutions before SS_finalize_plan runs. That seems unattractive, first because a whole-tree mutation pass is expensive, and second because we lack infrastructure for visiting expression subtrees in a Plan tree, so that we'd need a new function knowing as much as SS_finalize_plan knows about that. I also considered swapping the order of SS_finalize_plan and set_plan_references, but that fell foul of various assumptions that seem tricky to fix. So the approach adopted here is to teach SS_finalize_plan itself to check for such Aggrefs. I refactored things a bit in setrefs.c to avoid having three copies of the code that does that. Given the lack of any currently-known bug, no test case here. Discussion: https://postgr.es/m/2391880.1689025003@sss.pgh.pa.us
* Improve error message for MaxAllocSize overrun in accumArrayResult.Tom Lane2023-07-14
| | | | | | | | | | | | | | | Before, if you went past about 64M array elements in array_agg() and allied functions, you got a generic "invalid memory alloc request size" error. This patch replaces that with "array size exceeds the maximum allowed", which seems more user-friendly since it points you to needing to reduce the size of your array result. (This is the same error text you'd get from construct_md_array in the event of overrunning the maximum physical size for the finished array.) Per question from Shaozhong Shi. Since this hasn't come up often, I don't feel a need to back-patch. Discussion: https://postgr.es/m/CA+i5JwYtVS9z2E71PcNKAVPbOn4R2wuj-LqbJsYr_XOz73q7dQ@mail.gmail.com
* Add missing initializations of p_perminfoAmit Langote2023-07-14
| | | | | | | | | | | | In a61b1f74823, we failed to update transformFromClauseItem() and buildNSItemFromLists() to set ParseNamespaceItem.p_perminfo causing it to point to garbage. Pointed out by Tom Lane. Reported-by: Farias de Oliveira <matheusfarias519@gmail.com> Discussion: https://postgr.es/m/3173476.1689286373%40sss.pgh.pa.us Backpatch-through: 16
* Fix privilege check for SET SESSION AUTHORIZATION.Nathan Bossart2023-07-13
| | | | | | | | | | | | | | | | | | | Presently, the privilege check for SET SESSION AUTHORIZATION checks whether the original authenticated role was a superuser at connection start time. Even if the role loses the superuser attribute, its existing sessions are permitted to change session authorization to any role. This commit modifies this privilege check to verify the original authenticated role currently has superuser. In the event that the authenticated role loses superuser within a session authorization change, the authorization change will remain in effect, which means the user can still take advantage of the target role's privileges. However, [RE]SET SESSION AUTHORIZATION will only permit switching to the original authenticated role. Author: Joseph Koshakow Discussion: https://postgr.es/m/CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK%2BfVBhRzaBWYYLVQL-g%40mail.gmail.com
* Move privilege check for SET SESSION AUTHORIZATION.Nathan Bossart2023-07-13
| | | | | | | | | | | | | | | | Presently, the privilege check for SET SESSION AUTHORIZATION is performed in session_authorization's assign_hook. A relevant comment states, "It's OK because the check does not require catalog access and can't fail during an end-of-transaction GUC reversion..." However, we plan to add a catalog lookup to this privilege check in a follow-up commit. This commit moves this privilege check to the check_hook for session_authorization. Like check_role(), we do not throw a hard error for insufficient privileges when the source is PGC_S_TEST. Author: Joseph Koshakow Discussion: https://postgr.es/m/CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK%2BfVBhRzaBWYYLVQL-g%40mail.gmail.com
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-12 15:29:02 +0000