aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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
* Silence buildfarm warning chatter from bd1276a3c.Tom Lane2024-10-08
| | | | | | Buildfarm members using -Wextra complained about "warning: suggest braces around empty body in an 'if' statement". Do it gcc's way, though I see no actual readability benefit in this.
* Fix typo and run pgperltidy on newly-added testHeikki Linnakangas2024-10-08
| | | | From commit 85ec945b78.
* 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
* Add test for dead-end backendsHeikki Linnakangas2024-10-08
| | | | | | | | | | | The code path for launching a dead-end backend because we're out of slots was not covered by any tests, so add one. (Some tests did hit the case of launching a dead-end backend because the server is still starting up, though, so the gap in our test coverage wasn't as big as it sounds.) Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi
* Add test for connection limitsHeikki Linnakangas2024-10-08
| | | | | 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 test for password hash length limit.Nathan Bossart2024-10-07
| | | | | | In commit 8275325a06, I forgot to update password_1.out (an alternative expected test output file added by commit 3c44e7d8d4), so this test began failing on machines with FIPS mode enabled.
* vacuumdb: Schema-qualify operator in catalog query's WHERE clause.Nathan Bossart2024-10-07
| | | | | | | | | | | | | | | | | | Commit 1ab67c9dfa, which modified this catalog query so that it doesn't return temporary relations, forgot to schema-qualify the operator. A comment earlier in the function implores us to fully qualify everything in the query: * Since we execute the constructed query with the default search_path * (which could be unsafe), everything in this query MUST be fully * qualified. This commit fixes that. While at it, add a newline for consistency with surrounding code. Reviewed-by: Noah Misch Discussion: https://postgr.es/m/ZwQJYcuPPUsF0reU%40nathan Backpatch-through: 12
* 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
* Convert tab-complete's long else-if chain to a switch statement.Tom Lane2024-10-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename tab-complete.c to tab-complete.in.c, create the preprocessor script gen_tabcomplete.pl, and install Makefile/meson.build rules to create tab-complete.c from tab-complete.in.c. The preprocessor converts match_previous_words' else-if chain into a switch and populates tcpatterns[] with the data needed by the driver loop. The initial HeadMatches/TailMatches/Matches test in each else-if arm is now performed in a table-driven loop. Where we get a match, the corresponding switch case is invoked to see if the match succeeds. (It might not, if there were additional conditions in the original else-if test.) The total number of string comparisons done is just about the same as it was in the previous coding; however, now that we have table-driven logic underlying the handmade rules, there is room to improve that. For now I haven't bothered because tab completion is still plenty fast enough for human use. If the number of rules keeps increasing, we might someday need to do more in that area. The immediate benefit of all this thrashing is that C compilers frequently don't deal well with long else-if chains. On gcc 8.5.0, this reduces the compile time of tab-complete.c by about a factor of four, while MSVC is reported to crash outright with the previous coding. Discussion: https://postgr.es/m/2208466.1720729502@sss.pgh.pa.us
* Prepare tab-complete.c for preprocessing.Tom Lane2024-10-07
| | | | | | | | | | | | | | | | | | | | Separate out psql_completion's giant else-if chain of *Matches tests into a new function. Add the infrastructure needed for table-driven checking of the initial match of each completion rule. As-is, however, the code continues to operate as it did. The new behavior applies only if SWITCH_CONVERSION_APPLIED is #defined, which it is not here. (The preprocessor added in the next patch will add a #define for that.) The first and last couple of bits of psql_completion are not based on HeadMatches/TailMatches/Matches tests, so they stay where they are; they won't become part of the switch. This patch also fixes up a couple of if-conditions that didn't meet the conditions enumerated in the comment for match_previous_words(). Those restrictions exist to simplify the preprocessor. Discussion: https://postgr.es/m/2208466.1720729502@sss.pgh.pa.us
* Invent "MatchAnyN" option for tab-complete.c's Matches/MatchesCS.Tom Lane2024-10-07
| | | | | | | | | | | | | | | | | | | | | | | | | | This argument matches any number (including zero) of previous words. Use it to replace the common coding pattern if (HeadMatches("A", "B") && TailMatches("X", "Y")) with if (Matches("A", "B", MatchAnyN, "X", "Y")) In itself this feature doesn't do much except (arguably) make the code slightly shorter and more readable. However, it reduces the number of complex if-condition patterns that have to be dealt with in the next commits in this series. While here, restructure the *Matches implementation functions so that the actual work is done in functions that take a char ** array of pattern strings, and the versions taking variadic arguments are thin wrappers around the array ones. This simplifies the new Matches logic considerably. At the end of this patch series, the array functions will be the only ones that are material to performance, so having the variadic ones be wrappers makes sense. Discussion: https://postgr.es/m/2208466.1720729502@sss.pgh.pa.us
* 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
* Move Cluster.pm initialization code to a more obvious placeAndrew Dunstan2024-10-06
| | | | | | | Commit 460c0076e8 added some module intialization code to set signal handlers. However, that code has now become somewhat buried, as later commits added new subroutines. Therefore, move the initialization code to the module's INIT block where it won't become obscured.
* libpq: Discard leading and trailing spaces for parameters and values in URIsMichael Paquier2024-10-06
| | | | | | | | | | | | | | | | | | | Integer values applied a parsing rule through pqParseIntParam() that made URIs like this one working, even if these include spaces around values: "postgresql://localhost:5432/postgres?keepalives=1 &keepalives_idle=1 " This commit changes the parsing so as spaces before and after parameters and values are discarded, offering more consistency with the parsing that already applied to libpq for integer values in URIs. Note that %20 can be used in a URI for a space character. ECPGconnect() has been discarded leading and trailing spaces around parameters and values that for a long time, as well. Like f22e84df1dea, this is done as a HEAD-only change. Reviewed-by: Yuto Sasaki Discussion: https://postgr.es/m/Zv3oWOfcrHTph7JK@paquier.xyz
* 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 unneeded #includeHeikki Linnakangas2024-10-05
| | | | | | Unneeded since commit d72731a704. 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
* ecpg: avoid adding whitespace around '&' in connection URLs.Tom Lane2024-10-04
| | | | | | | | | | | The preprocessor really should not have done this to begin with. The space after '&' was masked by ECPGconnect's skipping spaces before option keywords, and the space before by dint of libpq being (mostly) insensitive to trailing space in option values. We fixed the one known problem with that in 920d51979. Hence this patch is mostly cosmetic, and we'll just change it in HEAD. Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.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
* Parse libpq's "keepalives" option more like other integer options.Tom Lane2024-10-02
| | | | | | | | | | | | | | | | | | | | | Use pqParseIntParam (nee parse_int_param) instead of using strtol directly. This allows trailing whitespace, which the previous coding didn't, and makes the spelling of the error message consistent with other similar cases. This seems to be an oversight in commit e7a221797, which introduced parse_int_param. That fixed places that were using atoi(), but missed this place which was randomly using strtol() instead. Ordinarily I'd consider this minor cleanup not worth back-patching. However, it seems that ecpg assumes it can add trailing whitespace to URL parameters, so that use of the keepalives option fails in that context. Perhaps that's worth improving as a separate matter. In the meantime, back-patch this to all supported branches. Yuto Sasaki (some further cleanup by me) Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
* File size in a backup manifest should use uint64, not size_t.Robert Haas2024-10-02
| | | | | | | | size_t is the size of an object in memory, not the size of a file on disk. Thanks to Tom Lane for noting the error. Discussion: http://postgr.es/m/1865585.1727803933@sss.pgh.pa.us
* Add fastpaths for when no objects are foundDaniel Gustafsson2024-10-02
| | | | | | | | | | | If there are no objects found, there is no reason to inspect the result columns and mallocing a zero-sized (which will be 1 byte in reality) heap buffer for it. Add a fast-path for immediately returning like how other object inspection functions are already doing it. Reviewed-by: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se
* Remove superfluous PQExpBuffer resettingDaniel Gustafsson2024-10-02
| | | | | | | | Since the buffer was just created, there is no reason to immediately reset it. Reviewed-by: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se
* 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
* initdb: Add new option "--no-data-checksums"Peter Eisentraut2024-10-01
| | | | | | | | | | Right now this does nothing except override any earlier --data-checksums option. But the idea is that --data-checksums could become the default, and then this option would allow forcing it off instead. Author: Greg Sabino Mullane <greg@turnstep.com> Discussion: https://www.postgresql.org/message-id/flat/CAKAnmmKwiMHik5AHmBEdf5vqzbOBbcwEPHo4-PioWeAbzwcTOQ@mail.gmail.com
* 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