summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
4 daysSupport COPY TO for partitioned tables.Masahiko Sawada
Previously, COPY TO command didn't support directly specifying partitioned tables so users had to use COPY (SELECT ...) TO variant. This commit adds direct COPY TO support for partitioned tables, improving both usability and performance. Performance tests show it's faster than the COPY (SELECT ...) TO variant as it avoids the overheads of query processing and sending results to the COPY TO command. When used with partitioned tables, COPY TO copies the same rows as SELECT * FROM table. Row-level security policies of the partitioned table are applied in the same way as when executing COPY TO on a plain table. Author: jian he <jian.universality@gmail.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Melih Mutlu <m.melihmutlu@gmail.com> Reviewed-by: Kirill Reshke <reshkekirill@gmail.com> Reviewed-by: Atsushi Torikoshi <torikoshia@oss.nttdata.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CACJufxEZt%2BG19Ors3bQUq-42-61__C%3Dy5k2wk%3DsHEFRusu7%3DiQ%40mail.gmail.com
4 daysFix thinko in commit 7d129ba54.Tom Lane
The revised logic in 001_ssltests.pl would fail if openssl doesn't work or if Perl is a 32-bit build, because it had already overwritten $serialno with something inappropriate to use in the eventual match. We could go back to the previous code layout, but it seems best to introduce a separate variable for the output of openssl. Per failure on buildfarm member mamba, which has a 32-bit Perl.
4 dayspg_dump: Remove unnecessary code for security labels on extensions.Fujii Masao
Commit d9572c4e3b4 added extension support and made pg_dump attempt to dump security labels on extensions. However, security labels on extensions are not actually supported, so this code was unnecessary. This commit removes it. Suggested-by: Jian He <jian.universality@gmail.com> Author: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxF8=z0v=888NKKEoTHQ+Jc4EXutFi91BF0fFjgFsZT6JQ@mail.gmail.com
4 dayspg_checksums: Use new routine to retrieve data of PG_VERSIONMichael Paquier
Previously, attempting to use pg_checksums on a cluster with a control file whose version does not match with what thetool is able to support would lead to the following error: pg_checksums: error: pg_control CRC value is incorrect This is confusing, because it would look like the control file is corrupted. However, the contents of the control file are correct, pg_checksums not being able to understand how the past control file is shaped. This commit adds a check based on PG_VERSION, using the facility added by cd0be131ba6f, using the same error message as some of the other frontend tools. A note is added in the documentation about the major version requirement. Author: Michael Banck <mbanck@gmx.net> Discussion: https://postgr.es/m/68f1ff21.170a0220.2c9b5f.4df5@mx.google.com
5 daysAdd static assertion that RELSEG_SIZE fits in an int.Tom Lane
Our configure script intended to ensure this, but it supposed that expr(1) would report an error for integer overflow. Maybe that was true when the code was written (commit 3c6248a82 of 2008-05-02), but all the modern expr's I tried will deliver bigger-than-int32 results without complaint. Moreover, if you use --with-segsize-blocks then there's no check at all. Ideally we'd add a test in configure itself to check that the value fits in int, but to do that we'd need to suppose that test(1) handles bigger-than-int32 numbers correctly. Probably modern ones do, but that's an assumption I could do without; and I'm not too trusting about meson either. Instead, let's install a static assertion, so that even people who ignore all the compiler warnings you get from such values will be forced to confront the fact that it won't work. This has been hazardous for awhile, but given that we hadn't heard a complaint about it till now, I don't feel a need to back-patch. Reported-by: Casey Shobe <casey.allen.shobe@icloud.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/C5DC82D6-C76D-4E8F-BC2E-DF03EFC4FA24@icloud.com
5 daysDon't rely on zlib's gzgetc() macro.Tom Lane
It emerges that zlib's configuration logic is not robust enough to guarantee that the macro will have the same ideas about struct field layout as the library itself does, leading to corruption of zlib's state struct followed by unintelligible failure messages. This hazard has existed for a long time, but we'd not noticed for several reasons: (1) We only use gzgetc() when trying to read a manually-compressed TOC file within a directory-format dump, which is a rarely-used scenario that we weren't even testing before 20ec99589. (2) No corruption actually occurs unless sizeof(long) is different from sizeof(off_t) and the platform is big-endian. (3) Some platforms have already fixed the configuration instability, at least sufficiently for their environments. Despite (3), it seems foolish to assume that the problem isn't going to be present in some environments for a long time to come. Hence, avoid relying on this macro. We can just #undef it and fall back on the underlying function of the same name. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/2122679.1760846783@sss.pgh.pa.us Backpatch-through: 13
5 daysFix Coverity issue reported in commit 2273fa32bce.Tatsuo Ishii
Coverity complains that the return value from gettuple_eval_partition (stored in variable "datum") in a do..while loop in WinGetFuncArgInPartition is overwritten when exiting the while loop. This commit tries to fix the issue by changing the gettuple_eval_partition call to: (void) gettuple_eval_partition() explicitly stating that we discard the return value. We are just interested in whether we are inside or outside of partition, NULL or NOT NULL here. Also enhance some comments for easier code reading. Reported-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/aPCOabSE4VfJLaky%40paquier.xyz
5 daysAdd pg_database_locale() to retrieve database default locale.Jeff Davis
Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/0151ad01239e2cc7b3139644358cf8f7b9622ff7.camel@j-davis.com
5 daysAdd pg_iswxdigit(), useful for tsearch.Jeff Davis
Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/0151ad01239e2cc7b3139644358cf8f7b9622ff7.camel@j-davis.com
6 daysAllow role created by new test to log in on Windows.Tom Lane
We must tell init about each role name we plan to connect as, else SSPI auth fails. Similar to previous patches such as 14793f471, 973542866. Oversight in 208927e65, per buildfarm member drongo. (Although that was back-patched to v13, the test script only exists in v16 and up.)
6 daysTidyup truncate_useless_pathkeys() functionDavid Rowley
This removes a few static functions and replaces them with 2 functions which aim to be more reusable. The upper planner's pathkey requirements can be simplified down to operations which require pathkeys in the same order as the pathkeys for the given operation, and operations which can make use of a Path's pathkeys in any order. Here we also add some short-circuiting to truncate_useless_pathkeys(). At any point we discover that all pathkeys are useful to a single operation, we can stop checking the remaining operations as we're not going to be able to find any further useful pathkeys - they're all possibly useful already. Adjusting this seems to warrant trying to put the checks roughly in order of least-expensive-first so that the short-circuits have the most chance of skipping the more expensive checks. In passing clean up has_useful_pathkeys() as it seems to have grown a redundant check for group_pathkeys. This isn't needed as standard_qp_callback will set query_pathkeys if there's any requirement to have group_pathkeys. All this code does is waste run-time effort and take up needless space. Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CAApHDvpbsEoTksvW5901MMoZo-hHf78E5up3uDOfkJnxDe_WAw@mail.gmail.com
6 daysFix determination of not-null constraint "locality" for inherited columnsÁlvaro Herrera
It is possible to have a non-inherited not-null constraint on an inherited column, but we were failing to preserve such constraints during pg_upgrade where the source is 17 or older, because of a bug in the pg_dump query for it. Oversight in commit 14e87ffa5c54. Fix that query. In passing, touch-up a bogus nearby comment introduced by the same commit. In version 17, make the regression tests leave a table in this situation, so that this scenario is tested in the cross-version upgrade tests of 18 and up. Author: Dilip Kumar <dilipbalaut@gmail.com> Reported-by: Andrew Bille <andrewbille@gmail.com> Bug: #19074 Backpatch-through: 18 Discussion: https://postgr.es/m/19074-ae2548458cf0195c@postgresql.org
6 daysFix pg_dump sorting of foreign key constraintsÁlvaro Herrera
Apparently, commit 04bc2c42f765 failed to notice that DO_FK_CONSTRAINT objects require identical handling as DO_CONSTRAINT ones, which causes some pg_upgrade tests in debug builds to fail spuriously. Add that. Author: Álvaro Herrera <alvherre@kurilemu.de> Backpatch-through: 13 Discussion: https://postgr.es/m/202510181201.k6y75v2tpf5r@alvherre.pgsql
6 daysFix reset of incorrect hash iterator in GROUPING SETS queriesDavid Rowley
This fixes an unlikely issue when fetching GROUPING SET results from their internally stored hash tables. It was possible in rare cases that the hash iterator would be set up incorrectly which could result in a crash. This was introduced in 4d143509c, so backpatch to v18. Many thanks to Yuri Zamyatin for reporting and helping to debug this issue. Bug: #19078 Reported-by: Yuri Zamyatin <yuri@yrz.am> Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/19078-dfd62f840a2c0766@postgresql.org Backpatch-through: 18
6 daysEnglishify comment wordingDavid Rowley
Switch to using the English word here rather than using a verbified function name. The full word still fits within a single comment line, so it's probably better just to use that instead of trying to shorten it, which might cause confusion. Author: Rafia Sabih <rafia.pghackers@gmail.com> Discussion: https://postgr.es/m/CA+FpmFe7LnRF2NA_QfARjkSWme4mNt+Udwbh2Yb=zZm35Ji31w@mail.gmail.com
7 daysFix hashjoin memory balancing logicTomas Vondra
Commit a1b4f289beec improved the hashjoin sizing to also consider the memory used by BufFiles for batches. The code however had multiple issues, making it ineffective or not working as expected in some cases. * The amount of memory needed by buffers was calculated using uint32, so it would overflow for nbatch >= 262144. If this happened the loop would exit prematurely and the memory usage would not be reduced. The nbatch overflow is fixed by reworking the condition to not use a multiplication at all, so there's no risk of overflow. An explicit cast was added to a similar calculation in ExecHashIncreaseBatchSize. * The loop adjusting the nbatch value used hash_table_bytes to calculate the old/new size, but then updated only space_allowed. The consequence is the total memory usage was not reduced, but all the memory saved by reducing the number of batches was used for the internal hash table. This was fixed by using only space_allowed. This is also more correct, because hash_table_bytes does not account for skew buckets. * The code was also doubling multiple parameters (e.g. the number of buckets for hash table), but was missing overflow protections. The loop now checks for overflow, and terminates if needed. It'd be possible to cap the value and continue the loop, but it's not worth the complexity. And the overflow implies the in-memory hash table is already very large anyway. While at it, rework the comment explaining how the memory balancing works, to make it more concise and easier to understand. The initial nbatch overflow issue was reported by Vaibhav Jain. The other issues were noticed by me and Melanie Plageman. Fix by me, with a lot of review and feedback by Melanie. Backpatch to 18, where the hashjoin memory balancing was introduced. Reported-by: Vaibhav Jain <jainva@google.com> Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Backpatch-through: 18 Discussion: https://postgr.es/m/CABa-Az174YvfFq7rLS+VNKaQyg7inA2exvPWmPWqnEn6Ditr_Q@mail.gmail.com
7 daysRemove unused data_bufsz from DecodedBkpBlock struct.Masahiko Sawada
Author: Mikhail Gribkov <youzhick@gmail.com> Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com> Discussion: https://postgr.es/m/CAMEv5_sxuaiAfSy1ZyN%3D7UGbHg3C10cwHhEk8nXEjiCsBVs4vQ%40mail.gmail.com
7 daysFix privilege checks for pg_prewarm() on indexes.Nathan Bossart
pg_prewarm() currently checks for SELECT privileges on the target relation. However, indexes do not have access rights of their own, so a role may be denied permission to prewarm an index despite having the SELECT privilege on its parent table. This commit fixes this by locking the parent table before the index (to avoid deadlocks) and checking for SELECT on the parent table. Note that the code is largely borrowed from amcheck_lock_relation_and_check(). An obvious downside of this change is the extra AccessShareLock on the parent table during prewarming, but that isn't expected to cause too much trouble in practice. Author: Ayush Vatsa <ayushvatsa1810@gmail.com> Co-authored-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/CACX%2BKaMz2ZoOojh0nQ6QNBYx8Ak1Dkoko%3DD4FSb80BYW%2Bo8CHQ%40mail.gmail.com Backpatch-through: 13
7 daysImprove TAP tests by replacing ok() with better Test::More functionsTom Lane
Transpose the changes made by commit fabb33b35 in 002_pg_dump.pl into its recently-created clone 006_pg_dump_compress.pl.
7 daysAvoid warnings in tests when openssl binary isn't availableDaniel Gustafsson
The SSL tests for pg_stat_ssl tries to exactly match the serial from the certificate by extracting it with the openssl binary. If that fails due to the binary not being available, a fallback match is used, but the attempt to execute a missing binary adds a warning to the output which can confuse readers for a failure in the test. Fix by only attempting if the openssl binary was found by autoconf/meson. Backpatch down to v16 where commit c8e4030d1bdd made the test use the OPENSSL variable from autoconf/meson instead of a hard- coded value. Author: Daniel Gustafsson <daniel@yesql.se> Reported-by: Christoph Berg <myon@debian.org> Discussion: https://postgr.es/m/aNPSp1-RIAs3skZm@msg.df7cb.de Backpatch-through: 16
7 daysChange config_generic.vartype to be initialized at compile timePeter Eisentraut
Previously, this was initialized at run time so that it did not have to be maintained by hand in guc_tables.c. But since that table is now generated anyway, we might as well generate this bit as well. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/8fdfb91e-60fb-44fa-8df6-f5dea47353c9@eisentraut.org
7 daysUse designated initializers for guc_tablesPeter Eisentraut
This makes the generating script simpler and the output easier to read. In the future, it will make it easier to reorder and rearrange the underlying C structures. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/8fdfb91e-60fb-44fa-8df6-f5dea47353c9@eisentraut.org
7 daysecpg: check return value of replace_variables()Daniel Gustafsson
The function returns false if it fails to allocate memory, so make sure to check the return value in callsites. Author: Aleksander Alekseev <aleksander@tigerdata.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAJ7c6TNPrU8ZxgdfN3PyGY1tzo0bgszx+KkqW0Z7zt3heyC1GQ@mail.gmail.com
7 daysReplace defunct URL with stable archive.org URL in rbtree.cDaniel Gustafsson
The URL for "Sorting and Searching Algorithms: A Cookbook" by Thomas Niemann has started returning 404, and since we refer to the page for license terms this replaces the now defunct link with one to the copy on archive.org. Author: Chao Li <lic@highgo.com> Discussion: https://postgr.es/m/6DED3DEF-875E-4D1D-8F8F-7353D5AF7B79@gmail.com
7 daysImprove TAP tests by replacing ok() with better Test::More functionsMichael Paquier
The TAP tests whose ok() calls are changed in this commit were relying on perl operators, rather than equivalents available in Test::More. For example, rather than the following: ok($data =~ qr/expr/m, "expr matching"); ok($data !~ qr/expr/m, "expr not matching"); The new test code uses this equivalent: like($data, qr/expr/m, "expr matching"); unlike($data, qr/expr/m, "expr not matching"); A huge benefit of the new formulation is that it is possible to know about the values we are checking if a failure happens, making debugging easier, should the test runs happen in the buildfarm, in the CI or locally. This change leads to more test code overall as perltidy likes to make the code pretty the way it is in this commit. Author: Sadhuprasad Patro <b.sadhu@gmail.com> Discussion: https://postgr.es/m/CAFF0-CHhwNx_Cv2uy7tKjODUbeOgPrJpW4Rpf1jqB16_1bU2sg@mail.gmail.com
7 daysdoc: Clarify when backend_xmin in pg_stat_replication can be NULL.Fujii Masao
Improve the documentation of pg_stat_replication to explain when the backend_xmin column becomes NULL. This happens when a replication slot is used (the xmin is then shown in pg_replication_slots) or when hot_standby_feedback is disabled. Author: Renzo Dani <arons7@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/CA+XOKQAMXzskpdUmj2sg03_5fmiXc2Gs0r3TX1_rmcFcqh+=xQ@mail.gmail.com
7 daysFix matching check in recovery test 042_low_level_backupMichael Paquier
042_low_level_backup compared the result of a query two times with a comparison operator based on an integer, while the result should be compared with a string. The outcome of the tests is currently not impacted by this change. However, it could be possible that the tests fail to detect future issues if the query results become different, for some reason. Oversight in 99b4a63bef94. Author: Sadhuprasad Patro <b.sadhu@gmail.com> Discussion: https://postgr.es/m/CAFF0-CHhwNx_Cv2uy7tKjODUbeOgPrJpW4Rpf1jqB16_1bU2sg@mail.gmail.com Backpatch-through: 17
7 dayspg_createsubscriber: Fix matching check in TAP testMichael Paquier
040_pg_createsubscriber has been calling safe_psql(), that returns the result of a SQL query, with ok() without checking the result generated (in this case 't', for a number of publications). The outcome of the tests is currently not impacted by this change. However, it could be possible that the test fails to detect future issues if the query results become different. The test is rewritten so as the number of publications is checked. This is not the fix suggested originally by the author, but this is more reliable in the long run. Oversight in e5aeed4b8020. Author: Sadhuprasad Patro <b.sadhu@gmail.com> Discussion: https://postgr.es/m/CAFF0-CHhwNx_Cv2uy7tKjODUbeOgPrJpW4Rpf1jqB16_1bU2sg@mail.gmail.com Backpatch-through: 18
8 daysFix update-po for the PGXS caseÁlvaro Herrera
The original formulation failed to take into account the fact that for the PGXS case, the source dir is not $(top_srcdir), so it ended up not doing anything. Handle it explicitly. Author: Ryo Matsumura <matsumura.ryo@fujitsu.com> Reviewed-by: Bryan Green <dbryan.green@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/TYCPR01MB113164770FB0B0BE6ED21E68EE8DCA@TYCPR01MB11316.jpnprd01.prod.outlook.com
8 daysAdd more TAP test coverage for pg_dump.Tom Lane
Add a test case to cover pg_dump with --compress=none. This brings the coverage of compress_none.c up from about 64% to 90%, in particular covering the new code added in a previous patch. Include compression of toc.dat in manually-compressed test cases. We would have found the bug fixed in commit a239c4a0c much sooner if we'd done this. As far as I can tell, this doesn't reduce test coverage at all, since there are other tests of directory format that still use an uncompressed toc.dat. Widen the wide row used to verify correct (de) compression. Commit 1a05c1d25 advises us (not without reason) to ensure that this test case fully fills DEFAULT_IO_BUFFER_SIZE, so that loops within the compression logic will iterate completely. To follow that advice with the proposed DEFAULT_IO_BUFFER_SIZE of 128K, we need something close to this. This does indeed increase the reported code coverage by a few lines. While here, fix a glitch that I noticed in testing: the $glob_patterns tests were incapable of failing, because glob() will return 'foo' as 'foo' whether there is a matching file or not. (Indeed, the stanza just above that one relies on that.) In my testing, this patch adds approximately as much runtime as was saved by the previous patch, so that it's about a wash compared to the old code. However, we get better test coverage. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3515357.1760128017@sss.pgh.pa.us
8 daysSplit 002_pg_dump.pl into two test files.Tom Lane
Add a new test script 006_pg_dump_compress.pl, containing just the pg_dump tests specifically concerned with compression, and remove those tests from 002_pg_dump.pl. We can also drop some infrastructure in 002_pg_dump.pl that was used only for these tests. The point of this is to avoid the cost of running these test cases over and over in all the scenarios (runs) that 002_pg_dump.pl exercises. We don't learn anything more about the behavior of the compression code that way, and we expend significant amounts of time, since one of these test cases is quite large and due to get larger. The intent of this specific patch is to provide exactly the same coverage as before, except that I went back to using --no-sync in all the test runs moved over to 006_pg_dump_compress.pl. I think that avoiding that had basically been cargo-culted into these test cases as a result of modeling them on the defaults_custom_format test case; again, doing that over and over isn't going to teach us anything new. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3515357.1760128017@sss.pgh.pa.us
8 daysAlign the data block sizes of pg_dump's various compression modes.Tom Lane
After commit fe8192a95, compress_zstd.c tends to produce data block sizes around 128K, and we don't really have any control over that unless we want to overrule ZSTD_CStreamOutSize(). Which seems like a bad idea. But let's try to align the other compression modes to produce block sizes roughly comparable to that, so that pg_restore's skip-data performance isn't enormously different for different modes. gzip compression can be brought in line simply by setting DEFAULT_IO_BUFFER_SIZE = 128K, which this patch does. That increases some unrelated buffer sizes, but none of them seem problematic for modern platforms. lz4's idea of appropriate block size is highly nonlinear: if we just increase DEFAULT_IO_BUFFER_SIZE then the output blocks end up around 200K. I found that adjusting the slop factor in LZ4State_compression_init was a not-too-ugly way of bringing that number roughly into line. With compress = none you get data blocks the same sizes as the table rows, which seems potentially problematic for narrow tables. Introduce a layer of buffering to make that case match the others. Comments in compress_io.h and 002_pg_dump.pl suggest that if we increase DEFAULT_IO_BUFFER_SIZE then we need to increase the amount of data fed through the tests in order to improve coverage. I've not done that here, leaving it for a separate patch. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3515357.1760128017@sss.pgh.pa.us
8 daysRemove partColsUpdated.Nathan Bossart
This information appears to have been unused since commit c5b7ba4e67. We could not find any references in third-party code, either. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/aO_CyFRpbVMtgJWM%40nathan
8 daysRefactor logical worker synchronization code into a separate file.Amit Kapila
To support the upcoming addition of a sequence synchronization worker, this patch extracts common synchronization logic shared by table sync workers and the new sequence sync worker into a dedicated file. This modularization improves code reuse, maintainability, and clarity in the logical workers framework. Author: vignesh C <vignesh21@gmail.com> Author: Hou Zhijie <houzj.fnst@fujitsu.com> Reviewed-by: shveta malik <shveta.malik@gmail.com> Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com> Reviewed-by: Peter Smith <smithpb2250@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CAA4eK1LC+KJiAkSrpE_NwvNdidw9F2os7GERUeSxSKv71gXysQ@mail.gmail.com
8 daysFix EPQ crash from missing partition directory in EStateAmit Langote
EvalPlanQualStart() failed to propagate es_partition_directory into the child EState used for EPQ rechecks. When execution time partition pruning ran during the EPQ scan, executor code dereferenced a NULL partition directory and crashed. Previously, propagating es_partition_directory into the EPQ EState was unnecessary because CreatePartitionPruneState(), which sets it on demand, also initialized the exec-pruning context. After commit d47cbf474, CreatePartitionPruneState() now initializes only the init- time pruning context, leaving exec-pruning context initialization to ExecInitNode(). Since EvalPlanQualStart() runs only ExecInitNode() and not CreatePartitionPruneState(), it can encounter a NULL es_partition_directory. Other executor fields initialized during CreatePartitionPruneState() are already copied into the child EState thanks to commit 8741e48e5d, but es_partition_directory was missed. Fix by borrowing the parent estate's es_partition_directory in EvalPlanQualStart(), and by clearing that field in EvalPlanQualEnd() so the parent remains responsible for freeing the directory. Add an isolation test permutation that triggers EPQ with execution- time partition pruning, the case that reproduces this crash. Bug: #19078 Reported-by: Yuri Zamyatin <yuri@yrz.am> Diagnosed-by: David Rowley <dgrowleyml@gmail.com> Author: David Rowley <dgrowleyml@gmail.com> Co-authored-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/19078-dfd62f840a2c0766@postgresql.org Backpatch-through: 18
8 daysOverride log_error_verbosity to "default" in test 009_log_temp_filesMichael Paquier
Per report from buildfarm member prion. The CI does not use this parameter, and this buildfarm member sets log_error_verbosity to "verbose". This would generate extra LOCATION entries in the logs, causing the regexps of the test to fail. Trying to support log_error_verbosity=verbose in the test would mean to tweak all the regexps used in the test to detect an optional set of LOCATION lines, at least. This would not improve the coverage, and forcing the GUC value is simpler. Oversight in 76bba033128a. Discussion: https://postgr.es/m/aPBaNNGiYT3xMBN1@paquier.xyz
8 daysAdd tests for logging of temporary file removal and statementMichael Paquier
Temporary file usage is sometimes attributed to the wrong query in the logs output. One identified reason is that unnamed portal cleanup (and consequently temp file logging) happens during the next BIND message as a, after debug_query_string has already been updated to the new query. Dropping an unnamed portal in the next BIND message is a rather old protocol behavior (fe19e56c572f, also mentioned in the docs). log_temp_files is a bit newer than that, as of be8a4318815. This commit adds tests to track which query is displayed next to the temporary file(s) removed when a portal is dropped, and in some cases if a query is displayed or not. We have not concluded how to improve the situation yet; these tests will at least help in checking what changes in the logs depending on the proposal discussed and how it affects the scenarios tracked by this new test. Author: Sami Imseih <samimseih@gmail.com> Author: Frédéric Yhuel <frederic.yhuel@dalibo.com> Reviewed-by: Mircea Cadariu <cadariu.mircea@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/3d07ee43-8855-42db-97e0-bad5db82d972@dalibo.com
9 daysFix lookup code for REINDEX INDEX.Nathan Bossart
This commit adjusts RangeVarCallbackForReindexIndex() to handle an extremely unlikely race condition involving concurrent OID reuse. In short, if REINDEX INDEX is executed at the same time that the index is re-created with the same name and OID but a different parent table OID, we might lock the wrong parent table. To fix, simply detect when this happens and emit an ERROR. Unfortunately, we can't gracefully handle this situation because we will have already locked the index, and we must lock the parent table before the index to avoid deadlocks. While at it, I've replaced all but one early return in this callback function with ERRORs that should be unreachable. While I haven't verified the presence of a live bug, the checks in question appear to be unnecessary, and the early returns seem prone to breaking the parent table locking code in subtle ways. If nothing else, this simplifies the code a bit. This is a bug fix and could be back-patched, but given the presumed rarity of the race condition and the lack of reports, I'm not going to bother. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/Z8zwVmGzXyDdkAXj%40nathan
9 daysAdd pg_iswalpha() and related functions.Jeff Davis
Per-character pg_locale_t APIs. Useful for tsearch parsing and potentially other places. Significant overlap with the regc_wc_isalpha() and related functions in regc_pg_locale.c, but this change leaves those intact for now. Discussion: https://postgr.es/m/0151ad01239e2cc7b3139644358cf8f7b9622ff7.camel@j-davis.com
9 daysFix lookups in pg_{clear,restore}_{attribute,relation}_stats().Nathan Bossart
Presently, these functions look up the relation's OID, lock it, and then check privileges. Not only does this approach provide no guarantee that the locked relation matches the arguments of the lookup, but it also allows users to briefly lock relations for which they do not have privileges, which might enable denial-of-service attacks. This commit adjusts these functions to use RangeVarGetRelidExtended(), which is purpose-built to avoid both of these issues. The new RangeVarGetRelidCallback function is somewhat complicated because it must handle both tables and indexes, and for indexes, we must check privileges on the parent table and lock it first. Also, it needs to handle a couple of extremely unlikely race conditions involving concurrent OID reuse. A downside of this change is that the coding doesn't allow for locking indexes in AccessShare mode anymore; everything is locked in ShareUpdateExclusive mode. Per discussion, the original choice of lock levels was intended for a now defunct implementation that used in-place updates, so we believe this change is okay. Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/Z8zwVmGzXyDdkAXj%40nathan Backpatch-through: 18
9 daysChange reset_extra into a config_generic common fieldPeter Eisentraut
This is not specific to the GUC parameter type, so it can be part of the generic struct rather than the type-specific struct (like the related "extra" field). This allows for some code simplifications. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/8fdfb91e-60fb-44fa-8df6-f5dea47353c9@eisentraut.org
9 daysAdd log_autoanalyze_min_durationPeter Eisentraut
The log output functionality of log_autovacuum_min_duration applies to both VACUUM and ANALYZE, so it is not possible to separate the VACUUM and ANALYZE log output thresholds. Logs are likely to be output only for VACUUM and not for ANALYZE. Therefore, we decided to separate the threshold for log output of VACUUM by autovacuum (log_autovacuum_min_duration) and the threshold for log output of ANALYZE by autovacuum (log_autoanalyze_min_duration). Author: Shinya Kato <shinya11.kato@gmail.com> Reviewed-by: Kasahara Tatsuhito <kasaharatt@oss.nttdata.com> Discussion: https://www.postgresql.org/message-id/flat/CAOzEurQtfV4MxJiWT-XDnimEeZAY+rgzVSLe8YsyEKhZcajzSA@mail.gmail.com
9 daysFix EvalPlanQual handling of foreign/custom joins in ExecScanFetch.Etsuro Fujita
If inside an EPQ recheck, ExecScanFetch would run the recheck method function for foreign/custom joins even if they aren't descendant nodes in the EPQ recheck plan tree, which is problematic at least in the foreign-join case, because such a foreign join isn't guaranteed to have an alternative local-join plan required for running the recheck method function; in the postgres_fdw case this could lead to a segmentation fault or an assert failure in an assert-enabled build when running the recheck method function. Even if inside an EPQ recheck, any scan nodes that aren't descendant ones in the EPQ recheck plan tree should be normally processed by using the access method function; fix by modifying ExecScanFetch so that if inside an EPQ recheck, it runs the recheck method function for foreign/custom joins that are descendant nodes in the EPQ recheck plan tree as before and runs the access method function for foreign/custom joins that aren't. This fix also adds to postgres_fdw an isolation test for an EPQ recheck that caused issues stated above. Oversight in commit 385f337c9. Reported-by: Kristian Lejao <kristianlejao@gmail.com> Author: Masahiko Sawada <sawada.mshk@gmail.com> Co-authored-by: Etsuro Fujita <etsuro.fujita@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com> Discussion: https://postgr.es/m/CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com Backpatch-through: 13
9 daysAdd some const qualifiersPeter Eisentraut
in guc-related source files, in anticipation of some further restructuring. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/8fdfb91e-60fb-44fa-8df6-f5dea47353c9@eisentraut.org
9 daysModernize some for loopsPeter Eisentraut
in guc-related source files, in anticipation of some further restructuring. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/8fdfb91e-60fb-44fa-8df6-f5dea47353c9@eisentraut.org
9 daysplpython: Remove support for major version conflict detectionPeter Eisentraut
This essentially reverts commit 866566a690b, which installed safeguards against loading plpython2 and plpython3 into the same process. We don't support plpython2 anymore, so this is obsolete. The Python and PL/Python initialization now happens again in _PG_init() rather than the first time a PL/Python call handler is invoked. (Often, these will be very close together.) I kept the separate PLy_initialize() function introduced by 866566a690b to keep _PG_init() a bit modular. Reviewed-by: Mario González Troncoso <gonzalemario@gmail.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/9eb9feb6-1df3-4f0c-a0dc-9bcf35273111%40eisentraut.org
9 daysStandardize use of REFRESH PUBLICATION in code and messages.Amit Kapila
This patch replaces ALTER SUBSCRIPTION REFRESH with ALTER SUBSCRIPTION REFRESH PUBLICATION in comments and error messages to improve clarity and support future extensibility. The change aligns with upcoming addition REFRESH SEQUENCES for sequence synchronization. Author: vignesh C <vignesh21@gmail.com> Author: Hou Zhijie <houzj.fnst@fujitsu.com> Reviewed-by: shveta malik <shveta.malik@gmail.com> Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com> Reviewed-by: Peter Smith <smithpb2250@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CAA4eK1LC+KJiAkSrpE_NwvNdidw9F2os7GERUeSxSKv71gXysQ@mail.gmail.com
9 dayspg_createsubscriber: Use new routine to retrieve data of PG_VERSIONMichael Paquier
pg_createsubscriber is documented as requiring the same major version as the target clusters. Attempting to use this tool on a cluster where the control file version read does not match with the version compiled with would lead to the following error message: pg_createsubscriber: error: control file appears to be corrupt This is confusing as the control file is correct: only the version expected does not match. This commit integrates pg_createsubscriber with the facility added by cd0be131ba6f, where the contents of PG_VERSION are read and compared with the value of PG_MAJORVERSION_NUM expected by the tool. This puts pg_createsubscriber in line with the documentation, with a better error message when the version does not match. Author: Michael Paquier <michael@paquier.xyz> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/aONDWig0bIGilixs@paquier.xyz
9 dayspg_resetwal: Use new routine to retrieve data of PG_VERSIONMichael Paquier
pg_resetwal's custom logic to retrieve the version number of a data folder's PG_VERSION can be replaced by the facility introduced in cd0be131ba6f. This removes some code. One thing specific to pg_resetwal is that the first line of PG_VERSION is read and reported in the error report generated when the major version read does not match with the version pg_resetwal has been compiled with. The new logic preserves this property, without changes to neither the error message nor the data used in the error report. Note that as a chdir() is done within the data folder before checking the data of PG_VERSION, get_pg_version() needs to be tweaked to look for PG_VERSION in the current folder. Author: Michael Paquier <michael@paquier.xyz> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/aOiirvWJzwdVCXph@paquier.xyz
9 dayspg_combinebackup: Use new routine to retrieve data of PG_VERSIONMichael Paquier
pg_combinebackup's custom logic to retrieve the version number of a data folder's PG_VERSION can be replaced by the facility introduced in cd0be131ba6f. This removes some code. One thing specific to this tool is that backend versions older than v10 are not supported. The new code does the same checks as the previous code. Author: Michael Paquier <michael@paquier.xyz> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/aOiirvWJzwdVCXph@paquier.xyz