aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Fixup for pg_set_relation_stats().Jeff Davis2024-10-13
| | | | | Reported-by: Noriyoshi Shinoda Discussion: https://postgr.es/m/DM4PR84MB17345E2DFF28A5557B7CBC3CEE7A2@DM4PR84MB1734.NAMPRD84.PROD.OUTLOOK.COM
* Use MAX_PARALLEL_WORKER_LIMIT for max_parallel_maintenance_workersMichael Paquier2024-10-13
| | | | | | | | | | | | | max_parallel_maintenance_workers has been introduced in 9da0cc35284b, and used a hardcoded limit of 1024 rather than this variable. max_parallel_workers and max_parallel_workers_per_gather already used MAX_PARALLEL_WORKER_LIMIT (1024) as their upper-bound since 6599c9ac3340. Author: Matthias van de Meent Reviewed-by: Zhang Mingli Discussion: https://postgr.es/m/CAEze2WiCiJD+8Wig_wGPyn4vgdPjbnYXy2Rw+9KYi6izTMuP=w@mail.gmail.com
* Correctly identify which EC members are computable at a plan node.Tom Lane2024-10-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | find_computable_ec_member() had the wrong mental model of what its primary caller prepare_sort_from_pathkeys() would do with the selected EquivalenceClass member expression. We will not compute the EC expression in a plan node atop the one returning the passed-in targetlist; rather, the EC expression will be computed as an additional column of that targetlist. So any Var or quasi-Var used in the given tlist is also available to the EC expression. In simple cases this makes no difference because the given tlist is just a list of Vars or quasi-Vars --- but if we are considering an appendrel member produced by flattening a UNION ALL, the tlist may contain expressions, resulting in failure to match and a "could not find pathkey item to sort" error. To fix, we can flatten both the tlist and the EC members with pull_var_clause(), and then just check for subset-ness, so that the code is actually shorter than before. While this bug is quite old, the present patch only works back to v13. We could possibly make it work in v12 by back-patching parts of 375398244. On the whole though I don't like the risk/reward ratio of that idea. v12's final release is next month, meaning there would be no chance to correct matters if the patch causes a regression. Since this failure has escaped notice for 14 years, it's likely nobody will hit it in the field with v12. Per bug #18652 from Alexander Lakhin. Andrei Lepikhov and Tom Lane Discussion: https://postgr.es/m/18652-deaa782ebcca85d1@postgresql.org
* Fix missed case for builtin collation provider.Jeff Davis2024-10-11
| | | | | | | | | | | | A missed check for the builtin collation provider could result in falling through to call isalpha(). This does not appear to have practical consequences because it only happens for characters in the ASCII range. Regardless, the builtin provider should not be calling libc functions, so backpatch. Discussion: https://postgr.es/m/1bd5a0a5192f82c22ee7527e825b18ab0028b2c7.camel@j-davis.com Backpatch-through: 17
* Create functions pg_set_relation_stats, pg_clear_relation_stats.Jeff Davis2024-10-11
| | | | | | | | | | | These functions are used to tweak statistics on any relation, provided that the user has MAINTAIN privilege on the relation, or is the database owner. Bump catalog version. Author: Corey Huinker Discussion: https://postgr.es/m/CADkLM=eErgzn7ECDpwFcptJKOk9SxZEk5Pot4d94eVTZsvj3gw@mail.gmail.com
* Avoid mixing custom and OpenSSL BIO functionsDaniel Gustafsson2024-10-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | PostgreSQL has for a long time mixed two BIO implementations, which can lead to subtle bugs and inconsistencies. This cleans up our BIO by just just setting up the methods we need. This patch does not introduce any functionality changes. The following methods are no longer defined due to not being needed: - gets: Not used by libssl - puts: Not used by libssl - create: Sets up state not used by libpq - destroy: Not used since libpq use BIO_NOCLOSE, if it was used it close the socket from underneath libpq - callback_ctrl: Not implemented by sockets The following methods are defined for our BIO: - read: Used for reading arbitrary length data from the BIO. No change in functionality from the previous implementation. - write: Used for writing arbitrary length data to the BIO. No change in functionality from the previous implementation. - ctrl: Used for processing ctrl messages in the BIO (similar to ioctl). The only ctrl message which matters is BIO_CTRL_FLUSH used for writing out buffered data (or signal EOF and that no more data will be written). BIO_CTRL_FLUSH is mandatory to implement and is implemented as a no-op since there is no intermediate buffer to flush. BIO_CTRL_EOF is the out-of-band method for signalling EOF to read_ex based BIO's. Our BIO is not read_ex based but someone could accidentally call BIO_CTRL_EOF on us so implement mainly for completeness sake. As the implementation is no longer related to BIO_s_socket or calling SSL_set_fd, methods have been renamed to reference the PGconn and Port types instead. This also reverts back to using BIO_set_data, with our fallback, as a small optimization as BIO_set_app_data require the ex_data mechanism in OpenSSL. Author: David Benjamin <davidben@google.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAF8qwaCZ97AZWXtg_y359SpOHe+HdJ+p0poLCpJYSUxL-8Eo8A@mail.gmail.com
* Add pg_ls_summariesdir().Nathan Bossart2024-10-11
| | | | | | | | | | | | | | | | This function returns the name, size, and last modification time of each regular file in pg_wal/summaries. This allows administrators to grant privileges to view the contents of this directory without granting privileges on pg_ls_dir(), which allows listing the contents of many other directories. This commit also gives the pg_monitor predefined role EXECUTE privileges on the new pg_ls_summariesdir() function. Bumps catversion. Author: Yushi Ogiwara Reviewed-by: Michael Paquier, Fujii Masao Discussion: https://postgr.es/m/a0a3af15a9b9daa107739eb45aa9a9bc%40oss.nttdata.com
* Use deconstruct_array_builtin instead of deconstruct_arrayÁlvaro Herrera2024-10-11
| | | | | | | | | Commit 062a84442424 introduced use of deconstruct_array when deconstruct_array_builtin can be used instead. Do that to save some code. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/Zwi5g2GzlUX1NqxR@ip-10-97-1-34.eu-west-3.compute.internal
* Adjust EXPLAIN's output for disabled nodesDavid Rowley2024-10-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | c01743aa4 added EXPLAIN output to display the plan node's disabled_node count whenever that count is above 0. Seemingly, there weren't many people who liked that output as each parent of a disabled node would also have a "Disabled Nodes" output due to the way disabled_nodes is accumulated towards the root plan node. It was often hard and sometimes impossible to figure out which nodes were disabled from looking at EXPLAIN. You might think it would be possible to manually add up the numbers from the "Disabled Nodes" output of a given node's children to figure out if that node has a higher disabled_nodes count than its children, but that wouldn't have worked for Append and Merge Append nodes if some disabled child nodes were run-time pruned during init plan. Those children are not displayed in EXPLAIN. Here we attempt to improve this output by only showing "Disabled: true" against only the nodes which are explicitly disabled themselves. That seems to be the output that's desired by the most people who voiced their opinion. This is done by summing up the disabled_nodes of the given node's children and checking if that number is less than the disabled_nodes of the current node. This commit also fixes a bug in make_sort() which was neglecting to set the Sort's disabled_nodes field. This should have copied what was done in cost_sort(), but it hadn't been updated. With the new output, the choice to not maintain that field properly was clearly wrong as the disabled-ness of the node was attributed to the Sort's parent instead. Reviewed-by: Laurenz Albe, Alena Rybakina Discussion: https://postgr.es/m/9e4ad616bebb103ec2084bf6f724cfc739e7fabb.camel@cybertec.at
* Unbreak overflow test for attinhcount/coninhcountÁlvaro Herrera2024-10-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 90189eefc1e1 narrowed pg_attribute.attinhcount and pg_constraint.coninhcount from 32 to 16 bits, but kept other related structs with 32-bit wide fields: ColumnDef and CookedConstraint contain an int 'inhcount' field which is itself checked for overflow on increments, but there's no check that the values aren't above INT16_MAX before assigning to the catalog columns. This means that a creative user can get a inconsistent table definition and override some protections. Fix it by changing those other structs to also use int16. Also, modernize style by using pg_add_s16_overflow for overflow testing instead of checking for negative values. We also have Constraint.inhcount, which is here removed completely. This was added by commit b0e96f311985 and not removed by its revert at 6f8bb7c1e961. It is not needed by the upcoming not-null constraints patch. This is mostly academic, so we agreed not to backpatch to avoid ABI problems. Bump catversion because of the changes to parse nodes. Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Co-authored-by: 何建 (jian he) <jian.universality@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/202410081611.up4iyofb5ie7@alvherre.pgsql
* Avoid crash in estimate_array_length with null root pointer.Tom Lane2024-10-09
| | | | | | | | | | | | | | | | | | | Commit 9391f7152 added a "PlannerInfo *root" parameter to estimate_array_length, but failed to consider the possibility that NULL would be passed for that, leading to a null pointer dereference. We could rectify the particular case shown in the bug report by fixing simplify_function/inline_function to pass through the root pointer. However, as long as eval_const_expressions is documented to accept NULL for root, similar hazards would remain. For now, let's just do the narrow fix of hardening estimate_array_length to not crash. Its behavior with NULL root will be the same as it was before 9391f7152, so this is not too awful. Per report from Fredrik Widlert (via Paul Ramsey). Back-patch to v17 where 9391f7152 came in. Discussion: https://postgr.es/m/518339E7-173E-45EC-A0FF-9A4A62AA4F40@cleverelephant.ca
* Apply GUC name from central table in more places of guc.cMichael Paquier2024-10-09
| | | | | | | | | | | | | | | | | | | The name extracted from the record of the GUC tables is applied to more internal places of guc.c. This change has the advantage to simplify parse_and_validate_value(), where the "name" was only used in elog messages, while it was required to match with the name from the GUC record. pg_parameter_aclcheck() now passes the name of the GUC from its record in two places rather than the caller's argument. The value given to this function goes through convert_GUC_name_for_parameter_acl() that does a simple ASCII downcasing. Few GUCs mix character casing in core; one test is added for one of these code paths with "IntervalStyle". Author: Peter Smith, Michael Paquier Discussion: https://postgr.es/m/ZwNh4vkc2NHJHnND@paquier.xyz
* Allow pushdown of HAVING clauses with grouping setsRichard Guo2024-10-09
| | | | | | | | | | | | | | | | | | | In some cases, we may want to transfer a HAVING clause into WHERE in hopes of eliminating tuples before aggregation instead of after. Previously, we couldn't do this if there were any nonempty grouping sets, because we didn't have a way to tell if the HAVING clause referenced any columns that were nullable by the grouping sets, and moving such a clause into WHERE could potentially change the results. Now, with expressions marked nullable by grouping sets with the RT index of the RTE_GROUP RTE, it is much easier to identify those clauses that reference any nullable-by-grouping-sets columns: we just need to check if the RT index of the RTE_GROUP RTE is present in the clause. For other HAVING clauses, they can be safely pushed down. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs4-NpzPgtKU=hgnvyn+J-GanxQCjrUi7piNzZ=upiCV=2Q@mail.gmail.com
* Consider explicit incremental sort for mergejoinsRichard Guo2024-10-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For a mergejoin, if the given outer path or inner path is not already well enough ordered, we need to do an explicit sort. Currently, we only consider explicit full sort and do not account for incremental sort. In this patch, for the outer path of a mergejoin, we choose to use explicit incremental sort if it is enabled and there are presorted keys. For the inner path, though, we cannot use incremental sort because it does not support mark/restore at present. The rationale is based on the assumption that incremental sort is always faster than full sort when there are presorted keys, a premise that has been applied in various parts of the code. In addition, the current cost model tends to favor incremental sort as being cheaper than full sort in the presence of presorted keys, making it reasonable not to consider full sort in such cases. It could be argued that what if a mergejoin with an incremental sort as the outer path is selected as the inner path of another mergejoin. However, this should not be a problem, because mergejoin itself does not support mark/restore either, and we will add a Material node on top of it anyway in this case (see final_cost_mergejoin). There is one ensuing plan change in the regression tests, and we have to modify that test case to ensure that it continues to test what it is intended to. No backpatch as this could result in plan changes. Author: Richard Guo Reviewed-by: David Rowley, Tomas Vondra Discussion: https://postgr.es/m/CAMbWs49x425QrX7h=Ux05WEnt8GS757H-jOP3_xsX5t1FoUsZw@mail.gmail.com
* Introduce two fields in EState to track parallel worker activityMichael Paquier2024-10-09
| | | | | | | | | | | | | | | These fields can be set by executor nodes to record how many parallel workers were planned to be launched and how many of them have been actually launched within the number initially planned. This data is able to give an approximation of the parallel worker draught a system is facing, making easier the tuning of related configuration parameters. These fields will be used by some follow-up patches to populate other parts of the system with their data. Author: Guillaume Lelarge, Benoit Lobréau Discussion: https://postgr.es/m/783bc7f7-659a-42fa-99dd-ee0565644e25@dalibo.com Discussion: https://postgr.es/m/CAECtzeWtTGOK0UgKXdDGpfTVSa5bd_VbUt6K6xn8P7X+_dZqKw@mail.gmail.com
* Add min and max aggregates for bytea type.Tom Lane2024-10-08
| | | | | | | | | Similar to a0f1fce80, although we chose to duplicate logic rather than invoke byteacmp, primarily to avoid repeat detoasting. Marat Buharov, Aleksander Alekseev Discussion: https://postgr.es/m/CAPCEVGXiASjodos4P8pgyV7ixfVn-ZgG9YyiRZRbVqbGmfuDyg@mail.gmail.com
* Use aux process resource owner in walsenderAndres Freund2024-10-08
| | | | | | | | | | AIO will need a resource owner to do IO. Right now we create a resowner on-demand during basebackup, and we could do the same for AIO. But it seems easier to just always create an aux process resowner. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Noah Misch <noah@leadboat.com> Discussion: https://postgr.es/m/1f6b50a7-38ef-4d87-8246-786d39f46ab9@iki.fi
* bufmgr/smgr: Don't cross segment boundaries in StartReadBuffers()Andres Freund2024-10-08
| | | | | | | | | | | | | | | | | With real AIO it doesn't make sense to cross segment boundaries with one IO. Add smgrmaxcombine() to allow upper layers to query which buffers can be merged. We could continue to cross segment boundaries when not using AIO, but it doesn't really make sense, because md.c will never be able to perform the read across the segment boundary in one system call. Which means we'll mark more buffers as undergoing IO than really makes sense - if another backend desires to read the same blocks, it'll be blocked longer than necessary. So it seems better to just never cross the boundary. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Noah Misch <noah@leadboat.com> Discussion: https://postgr.es/m/1f6b50a7-38ef-4d87-8246-786d39f46ab9@iki.fi
* bufmgr: Return early in ScheduleBufferTagForWriteback() if fsync=offAndres Freund2024-10-08
| | | | | | | | | | | As pg_flush_data() doesn't do anything with fsync disabled, there's no point in tracking the buffer for writeback. Arguably the better fix would be to change pg_flush_data() to flush data even with fsync off, but that's a behavioral change, whereas this is just a small optimization. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Noah Misch <noah@leadboat.com> Discussion: https://postgr.es/m/1f6b50a7-38ef-4d87-8246-786d39f46ab9@iki.fi
* Use an shmem_exit callback to remove backend from PMChildFlags on exitHeikki Linnakangas2024-10-08
| | | | | | | | | | | | This seems nicer than having to duplicate the logic between InitProcess() and ProcKill() for which child processes have a PMChildFlags slot. Move the MarkPostmasterChildActive() call earlier in InitProcess(), out of the section protected by the spinlock. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi
* Move check for binary mode and on_error option to the appropriate location.Fujii Masao2024-10-08
| | | | | | | | | | | | | | | Commit 9e2d870119 placed the check for binary mode and on_error before default values were inserted, which was not ideal. This commit moves the check to a more appropriate position after default values are set. Additionally, the comment incorrectly mentioned two checks before inserting defaults, when there are actually three. This commit corrects that comment. Author: Atsushi Torikoshi Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/8830518a-28ac-43a2-8a11-1676d9a3cdf8@oss.nttdata.com
* Add REJECT_LIMIT option to the COPY command.Fujii Masao2024-10-08
| | | | | | | | | | | | | | | | Previously, when ON_ERROR was set to 'ignore', the COPY command would skip all rows with data type conversion errors, with no way to limit the number of skipped rows before failing. This commit introduces the REJECT_LIMIT option, allowing users to specify the maximum number of erroneous rows that can be skipped. If more rows encounter data type conversion errors than allowed by REJECT_LIMIT, the COPY command will fail with an error, even when ON_ERROR = 'ignore'. Author: Atsushi Torikoshi Reviewed-by: Junwang Zhao, Kirill Reshke, jian he, Fujii Masao Discussion: https://postgr.es/m/63f99327aa6b404cc951217fa3e61fe4@oss.nttdata.com
* Improve style of two code pathsMichael Paquier2024-10-08
| | | | | | | | | | In execGrouping.c, execTuplesMatchPrepare() was doing a memory allocation that was not necessary when the number of columns was 0. In foreign.c, pg_options_to_table() was assigning twice a variable to the same value. Author: Ranier Vilela Discussion: https://postgr.es/m/CAEudQAqup0agbSzMjSLSTn=OANyCzxENF1+HrSYnr3WyZib7=Q@mail.gmail.com
* Fix search_path cache initialization.Jeff Davis2024-10-07
| | | | | | | | | The cache needs to be available very early, so don't rely on InitializeSearchPath() to initialize the it. Reported-by: Murat Efendioğlu Discussion: https://postgr.es/m/CACbCzujQ4zS8MM1bx-==+tr+D3Hk5G1cjN4XkUQ+Q=cEpwhzqg@mail.gmail.com Backpatch-through: 17
* Fix Y2038 issues with MyStartTime.Nathan Bossart2024-10-07
| | | | | | | | | | | | | | | Several places treat MyStartTime as a "long", which is only 32 bits wide on some platforms. In reality, MyStartTime is a pg_time_t, i.e., a signed 64-bit integer. This will lead to interesting bugs on the aforementioned systems in 2038 when signed 32-bit integers are no longer sufficient to store Unix time (e.g., "pg_ctl start" hanging). To fix, ensure that MyStartTime is handled as a 64-bit value everywhere. (Of course, users will need to ensure that time_t is 64 bits wide on their system, too.) Co-authored-by: Max Johnson Discussion: https://postgr.es/m/CO1PR07MB905262E8AC270FAAACED66008D682%40CO1PR07MB9052.namprd07.prod.outlook.com Backpatch-through: 12
* Restrict password hash length.Nathan Bossart2024-10-07
| | | | | | | | | | | | | | | | | | | Commit 6aa44060a3 removed pg_authid's TOAST table because the only varlena column is rolpassword, which cannot be de-TOASTed during authentication because we haven't selected a database yet and cannot read pg_class. Since that change, attempts to set password hashes that require out-of-line storage will fail with a "row is too big" error. This error message might be confusing to users. This commit places a limit on the length of password hashes so that attempts to set long password hashes will fail with a more user-friendly error. The chosen limit of 512 bytes should be sufficient to avoid "row is too big" errors independent of BLCKSZ, but it should also be lenient enough for all reasonable use-cases (or at least all the use-cases we could imagine). Reviewed-by: Tom Lane, Jonathan Katz, Michael Paquier, Jacob Champion Discussion: https://postgr.es/m/89e8649c-eb74-db25-7945-6d6b23992394%40gmail.com
* Fix fetching default toast value during decoding of in-progress transactions.Amit Kapila2024-10-07
| | | | | | | | | | | | | | | During logical decoding of in-progress transactions, we perform the toast table scan while fetching the default toast value for an attribute. We forgot to initialize the flag during this scan to indicate that the system table scan is in progress. We need this flag to ensure that during logical decoding we never directly access the tableam or heap APIs because we check for concurrent aborts only in systable_* APIs. Reported-by: Alexander Lakhin Author: Takeshi Ideriha, Hou Zhijie Reviewed-by: Amit Kapila, Hou Zhijie Backpatch-through: 14 Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org
* Use camel case for "DateStyle" in some error messagesMichael Paquier2024-10-07
| | | | | | | | | | | | | | This GUC is written as camel-case in most of the documentation and the GUC table (but not postgresql.conf.sample), and two error messages hardcoded it with lower case characters. Let's use a style more consistent. Most of the noise comes from the regression tests, updated to reflect the GUC name in these error messages. Author: Peter Smith Reviewed-by: Peter Eisentraut, Álvaro Herrera Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
* Ignore not-yet-defined Portals in pg_cursors view.Tom Lane2024-10-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | pg_cursor() supposed that any Portal it finds in the hash table must have sourceText set up, but there's an edge case where that is not so. A newly-created Portal has sourceText = NULL, and that doesn't change until PortalDefineQuery is called. In SPI_cursor_open_internal, we perform GetCachedPlan between CreatePortal and PortalDefineQuery, and it's possible for user-defined code to execute during that planning and cause a fetch from the pg_cursors view, resulting in a null-pointer-dereference crash. (It looks like the same could happen in exec_bind_message, but I've not tried to provoke a failure there.) I considered trying to fix this by setting sourceText sooner, but there may be instances of this same calling pattern in extensions, and we couldn't be sure they'd get the memo promptly. It seems better to redefine pg_cursor as not showing Portals that have not yet had PortalDefineQuery called on them, which we can do by just skipping them if sourceText is still NULL. (Before a1c692358, pg_cursor would instead return a row with NULL in the statement column. We could revert to that behavior but it doesn't really seem like a better definition, especially since our documentation doesn't suggest that the column could be NULL.) Per report from PetSerAl. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKygsHTBXLXjwV43kpZa+Cs+XTiaeeJiZdL4cPBm9f4MTdw7wg@mail.gmail.com
* Use generateClonedIndexStmt to propagate CREATE INDEX to partitions.Tom Lane2024-10-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When instantiating an existing partitioned index for a new child partition, we use generateClonedIndexStmt to build a suitable IndexStmt to pass to DefineIndex. However, when DefineIndex needs to recurse to instantiate a newly created partitioned index on an existing child partition, it was doing copyObject on the given IndexStmt and then applying a bunch of ad-hoc fixups. This has a number of problems, primarily that it implies fresh lookups of referenced objects such as opclasses and collations. Since commit 2af07e2f7 caused DefineIndex to restrict search_path internally, those lookups could fail or deliver different results than the original one. We can avoid those problems and save a few dozen lines of code by using generateClonedIndexStmt in this code path too. Another thing this fixes is incorrect propagation of parent-index comments to child indexes (because the copyObject approach copies the idxcomment field while generateClonedIndexStmt doesn't). I had noticed this in connection with commit c01eb619a, but not run the problem to ground. I'm tempted to back-patch this further than v17, but the only thing it's known to fix in older branches is the comment issue, which is pretty minor and doesn't seem worth the risk of introducing new issues in stable branches. (If anyone does care about that, clearing idxcomment in the copied IndexStmt would be a safer fix.) Per bug #18637 from usamoi. Back-patch to v17 where the search_path change came in. Discussion: https://postgr.es/m/18637-f51e314546e3ba2a@postgresql.org
* Clean up WaitLatch calls that passed latch without WL_LATCH_SETHeikki Linnakangas2024-10-05
| | | | | | | The 'latch' argument is ignored if WL_LATCH_SET is not given. Clarify these calls by not pointlessly passing MyLatch. Discussion: https://www.postgresql.org/message-id/391abe21-413e-4d91-a650-b663af49500c@iki.fi
* Remove unused latchHeikki Linnakangas2024-10-05
| | | | | | | It was left unused by commit bc971f4025, which replaced the latch usage with a condition variable Discussion: https://www.postgresql.org/message-id/391abe21-413e-4d91-a650-b663af49500c@iki.fi
* Reject non-ASCII locale names.Thomas Munro2024-10-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit bf03cfd1 started scanning all available BCP 47 locale names on Windows. This caused an abort/crash in the Windows runtime library if the default locale name contained non-ASCII characters, because of our use of the setlocale() save/restore pattern with "char" strings. After switching to another locale with a different encoding, the saved name could no longer be understood, and setlocale() would abort. "Turkish_Türkiye.1254" is the example from recent reports, but there are other examples of countries and languages with non-ASCII characters in their names, and they appear in Windows' (old style) locale names. To defend against this: 1. In initdb, reject non-ASCII locale names given explicity on the command line, or returned by the operating system environment with setlocale(..., ""), or "canonicalized" by the operating system when we set it. 2. In initdb only, perform the save-and-restore with Windows' non-standard wchar_t variant of setlocale(), so that it is not subject to round trip failures stemming from char string encoding confusion. 3. In the backend, we don't have to worry about the save-and-restore problem because we have already vetted the defaults, so we just have to make sure that CREATE DATABASE also rejects non-ASCII names in any new databases. SET lc_XXX doesn't suffer from the problem, but the ban applies to it too because it uses check_locale(). CREATE COLLATION doesn't suffer from the problem either, but it doesn't use check_locale() so it is not included in the new ban for now, to minimize the change. Anyone who encounters the new error message should either create a new duplicated locale with an ASCII-only name using Windows Locale Builder, or consider using BCP 47 names like "tr-TR". Users already couldn't initialize a cluster with "Turkish_Türkiye.1254" on PostgreSQL 16+, but the new failure mode is an error message that explains why, instead of a crash. Back-patch to 16, where bf03cfd1 landed. Older versions are affected in theory too, but only 16 and later are causing crash reports. Reviewed-by: Andrew Dunstan <andrew@dunslane.net> (the idea, not the patch) Reported-by: Haifang Wang (Centific Technologies Inc) <v-haiwang@microsoft.com> Discussion: https://postgr.es/m/PH8PR21MB3902F334A3174C54058F792CE5182%40PH8PR21MB3902.namprd21.prod.outlook.com
* Rename PageData to GenericXLogPageDataPeter Eisentraut2024-10-04
| | | | | | | | | | | | In the PostgreSQL C type naming schema, the type PageData should be what the pointer of type Page points to. But in this case it's actually an unrelated type local to generic_xlog.c. Rename that to a more specific name. This makes room to possible add a PageData type with the mentioned meaning, but this is not done here. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/001d457e-c118-4219-8132-e1846c2ae3c9%40eisentraut.org
* Speed up numeric division by always using the "fast" algorithm.Dean Rasheed2024-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Formerly there were two internal functions in numeric.c to perform numeric division, div_var() and div_var_fast(). div_var() performed division exactly to a specified rscale using Knuth's long division algorithm, while div_var_fast() used the algorithm from the "FM" library, which approximates each quotient digit using floating-point arithmetic, and computes a truncated quotient with DIV_GUARD_DIGITS extra digits. div_var_fast() could be many times faster than div_var(), but did not guarantee correct results in all cases, and was therefore only suitable for use in transcendental functions, where small errors are acceptable. This commit merges div_var() and div_var_fast() together into a single function with an extra "exact" boolean parameter, which can be set to false if the caller is OK with an approximate result. The new function uses the faster algorithm from the "FM" library, except that when "exact" is true, it does not truncate the computation with DIV_GUARD_DIGITS extra digits, but instead performs the full-precision computation, subtracting off complete multiples of the divisor for each quotient digit. However, it is able to retain most of the performance benefits of div_var_fast(), by delaying the propagation of carries, allowing the inner loop to be auto-vectorized. Since this may still lead to an inaccurate result, when "exact" is true, it then inspects the remainder and uses that to adjust the quotient, if necessary, to make it correct. In practice, the quotient rarely needs to be adjusted, and never by more than one in the final digit, though it's difficult to prove that, so the code allows for larger adjustments, just in case. In addition, use base-NBASE^2 arithmetic and a 64-bit dividend array, similar to mul_var(), so that the number of iterations of the outer loop is roughly halved. Together with the faster algorithm, this makes div_var() up to around 20 times as fast as the old Knuth algorithm when "exact" is true, and up to 2 or 3 times as fast as the old div_var_fast() function when "exact" is false. Dean Rasheed, reviewed by Joel Jacobson. Discussion: https://postgr.es/m/CAEZATCVHR10BPDJSANh0u2+Sg6atO3mD0G+CjKDNRMD-C8hKzQ@mail.gmail.com
* Remove assertion checking query ID in execMain.cMichael Paquier2024-10-04
| | | | | | | | | | This assertion has been added by 24f520594809, but Alexander Lakhin has proved that the ExecutorRun() one can be broken by using a PL function that manipulates compute_query_id and track_activities, while the ones in ExecutorFinish() and ExecutorEnd() could be triggered when cleaning up portals at the beginning of a new query execution. Discussion: https://postgr.es/m/b37d8e6c-e83d-e157-8865-1b2460a6aef2@gmail.com
* Fix wrong varnullingrels error for MERGE WHEN NOT MATCHED BY SOURCE.Dean Rasheed2024-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the source relation appears on the outer side of the join. Thus, any Vars referring to the source in the merge join condition, actions, and RETURNING list should be marked as nullable by the join, since they are used in the ModifyTable node above the join. Note that this only applies to the copy of join condition used in the executor to distinguish MATCHED from NOT MATCHED BY SOURCE cases. Vars in the original join condition, inside the join node itself, should not be marked. Failure to correctly mark these Vars led to a "wrong varnullingrels" error in the final stage of query planning, in some circumstances. We happened to get away without this in all previous tests, since they all involved a ModifyTable node directly on top of the join node, so that the top plan targetlist coincided with the output of the join, and the varnullingrels check was more lax. However, if another plan node, such as a one-time filter Result node, gets inserted between the ModifyTable node and the join node, then a stricter check is applied, which fails. Per bug #18634 from Alexander Lakhin. Thanks to Tom Lane and Richard Guo for review and analysis. Back-patch to v17, where WHEN NOT MATCHED BY SOURCE support was added to MERGE. Discussion: https://postgr.es/m/18634-db5299c937877f2b%40postgresql.org
* Fix incorrect non-strict join recheck in MERGE WHEN NOT MATCHED BY SOURCE.Dean Rasheed2024-10-03
| | | | | | | | | | | | | | | | | | | | | | If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the merge join condition is used by the executor to distinguish MATCHED from NOT MATCHED BY SOURCE cases. However, this qual is executed using the output from the join subplan node, which nulls the output from the source relation in the not matched case, and so the result may be incorrect if the join condition is "non-strict" -- for example, something like "src.col IS NOT DISTINCT FROM tgt.col". Fix this by enhancing the join recheck condition with an additional "src IS NOT NULL" check, so that it does the right thing when evaluated using the output from the join subplan. Noted by Tom Lane while investigating bug #18634 from Alexander Lakhin. Back-patch to v17, where WHEN NOT MATCHED BY SOURCE support was added to MERGE. Discussion: https://postgr.es/m/18634-db5299c937877f2b%40postgresql.org
* Replace Unicode apostrophe with ASCII apostropheAmit Langote2024-10-03
| | | | | | | | | In commit babb3993dbe9, I accidentally introduced a Unicode apostrophe (U+2019). This commit replaces it with the ASCII apostrophe (U+0027) for consistency. Reported-by: Alexander Korotkov <aekorotkov@gmail.com> Discussion: https://postgr.es/m/CAPpHfduNWMBjkJFtqXJremk6b6YQYO2s3_VEpnj-T_CaUNUYYQ@mail.gmail.com
* Refactor CopyFrom() in copyfrom.c.Fujii Masao2024-10-03
| | | | | | | | | | | | | | | | | | This commit simplifies CopyFrom() by removing the unnecessary local variable 'skipped', which tracked the number of rows skipped due to on_error = 'ignore'. That count is already handled by cstate->num_errors, so the 'skipped' variable was redundant. Additionally, the condition on_error != COPY_ON_ERROR_STOP is removed. Since on_error == COPY_ON_ERROR_IGNORE is already checked, and on_error only has two values (ignore and stop), the additional check was redundant and made the logic harder to read. Seemingly this was introduced in preparation for a future patch, but the current checks don’t offer clear value and have been removed to improve readability. Author: Atsushi Torikoshi Reviewed-by: Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com
* Add log_verbosity = 'silent' support to COPY command.Fujii Masao2024-10-03
| | | | | | | | | | | | | | | | | | | | | Previously, when the on_error option was set to ignore, the COPY command would always log NOTICE messages for input rows discarded due to data type incompatibility. Users had no way to suppress these messages. This commit introduces a new log_verbosity setting, 'silent', which prevents the COPY command from emitting NOTICE messages when on_error = 'ignore' is used, even if rows are discarded. This feature is particularly useful when processing malformed files frequently, where a flood of NOTICE messages can be undesirable. For example, when frequently loading malformed files via the COPY command or querying foreign tables using file_fdw (with an upcoming patch to add on_error support for file_fdw), users may prefer to suppress these messages to reduce log noise and improve clarity. Author: Atsushi Torikoshi Reviewed-by: Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com
* Fix expression list handling in ATExecAttachPartition()Amit Langote2024-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit addresses two issues related to the manipulation of the partition constraint expression list in ATExecAttachPartition(). First, the current use of list_concat() to combine the partition's constraint (retrieved via get_qual_from_partbound()) with the parent table’s partition constraint can lead to memory safety issues. After calling list_concat(), the original constraint (partBoundConstraint) might no longer be safe to access, as list_concat() may free or modify it. Second, there's a logical error in constructing the constraint for validating against the default partition. The current approach incorrectly includes a negated version of the parent table's partition constraint, which is redundant, as it always evaluates to false for rows in the default partition. To resolve these issues, list_concat() is replaced with list_concat_copy(), ensuring that partBoundConstraint remains unchanged and can be safely reused when constructing the validation constraint for the default partition. This fix is not applied to back-branches, as there is no live bug and the issue has not caused any reported problems in practice. Nitin Jadhav posted a patch to address the memory safety issue, but I decided to follow Alvaro Herrera's suggestion from the initial discussion, as it allows us to fix both the memory safety and logical issues. Reported-by: Andres Freund <andres@anarazel.de> Reported-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/20231115165737.zeulb575cgrbqo74@awork3.anarazel.de Discussion: https://postgr.es/m/CAMm1aWbmYHM3bqtjyMQ-a+4Ub=dgsb_2E3_up2cn=UGdHNrGTg@mail.gmail.com
* Remove support for unlogged on partitioned tablesMichael Paquier2024-10-03
| | | | | | | | | | | | | | | | | | | | | | The following commands were allowed on partitioned tables, with different effects: 1) ALTER TABLE SET [UN]LOGGED did not issue an error, and did not update pg_class.relpersistence. 2) CREATE UNLOGGED TABLE was working with pg_class.relpersistence marked as initially defined, but partitions did not inherit the UNLOGGED property, which was confusing. This commit causes the commands mentioned above to fail for partitioned tables, instead. pg_dump is tweaked so as partitioned tables marked as UNLOGGED ignore the option when dumped from older server versions. pgbench needs a tweak for --unlogged and --partitions=N to ignore the UNLOGGED option on the partitioned tables created, its partitions still being unlogged. Author: Michael Paquier Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/ZiiyGFTBNkqcMQi_@paquier.xyz
* Adjust json_manifest_per_file_callback API in one more place.Tom Lane2024-10-02
| | | | | | Oversight in commit d94cf5ca7 (and in my testing of same). Discussion: https://postgr.es/m/9468.1727895630@sss.pgh.pa.us
* Fix inconsistent reporting of checkpointer stats.Fujii Masao2024-10-02
| | | | | | | | | | | | | | | | | | | | | | | Previously, the pg_stat_checkpointer view and the checkpoint completion log message could show different numbers for buffers written during checkpoints. The view only counted shared buffers, while the log message included both shared and SLRU buffers, causing inconsistencies. This commit resolves the issue by updating both the view and the log message to separately report shared and SLRU buffers written during checkpoints. A new slru_written column is added to the pg_stat_checkpointer view to track SLRU buffers, while the existing buffers_written column now tracks only shared buffers. This change would help users distinguish between the two types of buffers, in the pg_stat_checkpointer view and the checkpoint complete log message, respectively. Bump catalog version. Author: Nitin Jadhav Reviewed-by: Bharath Rupireddy, Michael Paquier, Kyotaro Horiguchi, Robert Haas Reviewed-by: Andres Freund, vignesh C, Fujii Masao Discussion: https://postgr.es/m/CAMm1aWb18EpT0whJrjG+-nyhNouXET6ZUw0pNYYAe+NezpvsAA@mail.gmail.com
* Reject a copy EOF marker that has data ahead of it on the same line.Tom Lane2024-10-01
| | | | | | | | | | | | | | | | | | We have always documented that a copy EOF marker (\.) must appear by itself on a line, and that is how psql interprets the rule. However, the backend's actual COPY FROM logic only insists that there not be data between the \. and the following newline. Any data ahead of the \. is parsed as a final line of input. It's hard to interpret this as anything but an ancient mistake that we've faithfully carried forward. Continuing to allow it is not cost-free, since it could mask client-side bugs that unnecessarily backslash-escape periods (and thereby risk accidentally creating an EOF marker). So, let's remove that provision and throw error if the EOF marker isn't alone on its line, matching what the documentation has said right along. Adjust the relevant error messages to be clearer, too. Discussion: https://postgr.es/m/ed659f37-a9dd-42a7-82b9-0da562cc4006@manitou-mail.org
* Use macro to define the number of enum valuesPeter Eisentraut2024-10-01
| | | | | | | | | | | | Refactoring in the interest of code consistency, a follow-up to 2e068db56e31. The argument against inserting a special enum value at the end of the enum definition is that a switch statement might generate a compiler warning unless it has a default clause. Aleksander Alekseev, reviewed by Michael Paquier, Dean Rasheed, Peter Eisentraut Discussion: https://postgr.es/m/CAJ7c6TMsiaV5urU_Pq6zJ2tXPDwk69-NKVh4AMN5XrRiM7N%2BGA%40mail.gmail.com
* jit: Use opaque pointers in all supported LLVM versions.Peter Eisentraut2024-10-01
| | | | | | | | | | | | | | | | | | LLVM's opaque pointer change began in LLVM 14, but remained optional until LLVM 16. When commit 37d5babb added opaque pointer support, we didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk weird bitcode incompatibility problems in released branches of PostgreSQL. (That might have been overly cautious, I don't know.) Now that PostgreSQL 18 has dropped support for LLVM versions < 14, and since it hasn't been released yet and no extensions or bitcode have been built against it in the wild yet, we can be more aggressive. We can rip out the support code and build system clutter that made opaque pointer use optional. Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com
* jit: Require at least LLVM 14, if enabled.Peter Eisentraut2024-10-01
| | | | | | | | | | Remove support for LLVM versions 10-13. The default on all non-EOL'd OSes represented in our build farm will be at least LLVM 14 when PostgreSQL 18 ships. Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com
* Fix race condition in COMMIT PREPARED causing orphaned 2PC filesMichael Paquier2024-10-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | COMMIT PREPARED removes on-disk 2PC files near its end, but the state checked if a file is on-disk or not gets read from shared memory while not holding the two-phase state lock. Because of that, there was a small window where a second backend doing a PREPARE TRANSACTION could reuse the GlobalTransaction put back into the 2PC free list by the COMMIT PREPARED, overwriting the "ondisk" flag read afterwards by the COMMIT PREPARED to decide if its on-disk two-phase state file should be removed, preventing the file deletion. This commit fixes this issue so as the "ondisk" flag in the GlobalTransaction is read while holding the two-phase state lock, not from shared memory after its entry has been added to the free list. Orphaned two-phase state files flushed to disk after a checkpoint are discarded at the beginning of recovery. However, a truncation of pg_xact/ would make the startup process issue a FATAL when it cannot read the SLRU page holding the state of the transaction whose 2PC file was orphaned, which is a necessary step to decide if the 2PC file should be removed or not. Removing manually the file would be necessary in this case. Issue introduced by effe7d9552dd, so backpatch all the way down. Mea culpa. Author: wuchengwen Discussion: https://postgr.es/m/tencent_A7F059B5136A359625C7B2E4A386B3C3F007@qq.com Backpatch-through: 12
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-02 23:31:03 +0000