Re: pgsql: pg_upgrade: Parallelize retrieving relation information.
On Tue, Sep 17, 2024 at 12:11 AM Nathan Bossart wrote: > pg_upgrade: Parallelize retrieving relation information. > > This commit makes use of the new task framework in pg_upgrade to > parallelize retrieving relation and logical slot information. This > step will now process multiple databases concurrently when > pg_upgrade's --jobs option is provided a value greater than 1. > > Reviewed-by: Daniel Gustafsson, Ilya Gladyshev > Discussion: https://postgr.es/m/20240516211638.GA1688936%40nathanxps13 #include "access/transam.h" #include "catalog/pg_class_d.h" +#include "pqexpbuffer.h" #include "pg_upgrade.h" I think the alphabetic order of includes needs fixing. -- Regards, Alexander Korotkov Supabase
Re: pgsql: Introduce framework for parallelizing various pg_upgrade tasks.
Hi! On Tue, Sep 17, 2024 at 12:11 AM Nathan Bossart wrote: > Introduce framework for parallelizing various pg_upgrade tasks. > > A number of pg_upgrade steps require connecting to every database > in the cluster and running the same query in each one. When there > are many databases, these steps are particularly time-consuming, > especially since they are performed sequentially, i.e., we connect > to a database, run the query, and process the results before moving > on to the next database. > > This commit introduces a new framework that makes it easy to > parallelize most of these once-in-each-database tasks by processing > multiple databases concurrently. This framework manages a set of > slots that follow a simple state machine, and it uses libpq's > asynchronous APIs to establish the connections and run the queries. > The --jobs option is used to determine the number of slots to use. > To use this new task framework, callers simply need to provide the > query and a callback function to process its results, and the > framework takes care of the rest. A more complete description is > provided at the top of the new task.c file. > > None of the eligible once-in-each-database tasks are converted to > use this new framework in this commit. That will be done via > several follow-up commits. > > Reviewed-by: Jeff Davis, Robert Haas, Daniel Gustafsson, Ilya Gladyshev, > Corey Huinker > Discussion: https://postgr.es/m/20240516211638.GA1688936%40nathanxps13 Should we add UpgradeTaskProcessCB to the typedefs.list? I don't see this would directly influence indentation right now, but probably we should do for uniformity? -- Regards, Alexander Korotkov Supabase
pgsql: Ensure standby promotion point in 043_wal_replay_wait.pl
Ensure standby promotion point in 043_wal_replay_wait.pl This commit ensures standby will be promoted at least at the primary insert LSN we have just observed. We use pg_switch_wal() to force the insert LSN to be written then wait for standby to catchup. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/1d7b08f2-64a2-77fb-c666-c9a74c68eeda%40gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/2520226c953c0b443791a185a8d1fb8b71d9fe9e Modified Files -- src/test/recovery/t/043_wal_replay_wait.pl | 6 ++ 1 file changed, 6 insertions(+)
pgsql: Minor cleanup related to pg_wal_replay_wait() procedure
Minor cleanup related to pg_wal_replay_wait() procedure * Rename $node_standby1 to $node_standby in 043_wal_replay_wait.pl as there is only one standby. * Remove useless debug printing in 043_wal_replay_wait.pl. * Fix typo in one check description in 043_wal_replay_wait.pl. * Fix some wording in comments and documentation. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/1d7b08f2-64a2-77fb-c666-c9a74c68eeda%40gmail.com Reviewed-by: Alexander Lakhin Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/85b98b8d5a48092acd03db76a8bf02d9c38c1d79 Modified Files -- doc/src/sgml/func.sgml | 2 +- src/backend/access/transam/xlog.c | 2 +- src/backend/commands/waitlsn.c | 4 +-- src/test/recovery/t/043_wal_replay_wait.pl | 48 ++ 4 files changed, 27 insertions(+), 29 deletions(-)
Re: pgsql: Implement pg_wal_replay_wait() stored procedure
On Tue, Sep 3, 2024 at 4:07 PM Alexander Korotkov wrote: > If no objections, I will push the patch moving code then go ahead > writing the two patches above. The patch for code movement missed couple of includes. Revised version is attached. -- Regards, Alexander Korotkov Supabase v2-0001-Move-pg_wal_replay_wait-to-xlogfuncs.c.patch Description: Binary data
Re: pgsql: Implement pg_wal_replay_wait() stored procedure
Hi, Michael! On Mon, Sep 2, 2024 at 3:25 AM Michael Paquier wrote: > > On Mon, Sep 02, 2024 at 02:55:50AM +0300, Alexander Korotkov wrote: > > Could you, please, check the attached patch? > > The patch moving the code looks correct at quick glance. Thank you for your feedback. > Now, I've been staring at this line, wondering why this is required > while WaitForLSNReplay() does not return any status: > + (void) WaitForLSNReplay(target_lsn, timeout); > > And this makes me question whether you have the right semantics > regarding the SQL function itself. Could it be more useful for > WaitForLSNReplay() to report an enum state that tells you the reason > why a wait may not have happened with a text or a tuple returned by > the function? There are quite a few reasons why a wait may or may not > happen: > - Recovery has ended, target LSN has been reached. > - We're not in recovery anymore. Is an error the most useful thing > here actually? > - Timeout may have been reached, where an error is also generated. > ERRCODE_QUERY_CANCELED is not a correct error state. > - Perhaps more of these in the future? > > My point is: this is a feature that can be used for monitoring as far > as I know, still it does not stick as a feature that could be most > useful for such applications. Wouldn't it be more useful for client > applications to deal with a state returned by the SQL function rather > than having to parse error strings to know what happened? What is > returned by pg_wal_replay_wal() reflects on the design of > WaitForLSNReplay() itself. By design, pg_wal_replay_wal() should be followed up with read-only query to standby. The read-only query gets guarantee that it's executed after the replay of LSN specified as pg_wal_replay_wal() design. Returning the status from pg_wal_replay_wal() would require additional cycle of its processing. But one of the main objectives of this feature was reducing roundtrips during waiting for LSN. On the other hand, I see that returning status could make sense for certain use cases. I think I could write two patches to provide that. 1. Make WaitForLSNReplay() return status, and make pg_wal_replay_wal() be responsible for throwing all the errors. 2. New procedure pg_wal_replay_wal_status() (or some better name?), which returns status to the user instead of throwing errors. If no objections, I will push the patch moving code then go ahead writing the two patches above. -- Regards, Alexander Korotkov Supabase
Re: pgsql: Implement pg_wal_replay_wait() stored procedure
On Mon, Sep 2, 2024 at 2:28 AM Michael Paquier wrote: > On Sun, Sep 01, 2024 at 10:35:27PM +0300, Alexander Korotkov wrote: > > This path hasn't changes since the patch revision when it was a > > utility command. I agree that this doesn't look like proper path for > > stored procedure. But I don't think src/backend/utils/adt is > > appropriate path either, because it's not really about data type. > > pg_wal_replay_wait() looks a good neighbor for > > pg_wal_replay_pause()/pg_wal_replay_resume() and other WAL-related > > functions. So, what about moving it to src/backend/access/transam? > > Moving the new function to xlogfuncs.c while publishing > WaitForLSNReplay() makes sense to me. Thank you for proposal. I like this. Could you, please, check the attached patch? -- Regards, Alexander Korotkov Supabase v1-0001-Move-pg_wal_replay_wait-to-xlogfuncs.c.patch Description: Binary data
Re: pgsql: Implement pg_wal_replay_wait() stored procedure
On Fri, Aug 30, 2024 at 10:42 PM Peter Eisentraut wrote: > On 02.08.24 20:22, Alexander Korotkov wrote: > > Implement pg_wal_replay_wait() stored procedure > > Why is this under src/backend/command/? Wouldn't it belong under > src/backend/utils/adt/? This path hasn't changes since the patch revision when it was a utility command. I agree that this doesn't look like proper path for stored procedure. But I don't think src/backend/utils/adt is appropriate path either, because it's not really about data type. pg_wal_replay_wait() looks a good neighbor for pg_wal_replay_pause()/pg_wal_replay_resume() and other WAL-related functions. So, what about moving it to src/backend/access/transam? -- Regards, Alexander Korotkov Supabase
pgsql: Revert: Avoid looping over all type cache entries in TypeCacheRe
Revert: Avoid looping over all type cache entries in TypeCacheRelCallback() This commit reverts c14d4acb8 as the patch design didn't take into account that TypeCacheEntry could be invalidated during the lookup_type_cache() call. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/1927cba4-177e-5c23-cbcc-d444a850304f%40gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/8daa62a10c911c851f7e9ec5ef7b90cfd4b73212 Modified Files -- src/backend/utils/cache/typcache.c | 275 ++--- src/tools/pgindent/typedefs.list | 1 - 2 files changed, 44 insertions(+), 232 deletions(-)
pgsql: Avoid looping over all type cache entries in TypeCacheRelCallbac
Avoid looping over all type cache entries in TypeCacheRelCallback() Currently when a single relcache entry gets invalidated, TypeCacheRelCallback() has to loop over all type cache entries to find appropriate typentry to invalidate. Unfortunately, using the syscache here is impossible, because this callback could be called outside a transaction and this makes impossible catalog lookups. This is why present commit introduces RelIdToTypeIdCacheHash to map relation OID to its composite type OID. We are keeping RelIdToTypeIdCacheHash entry while corresponding type cache entry have something to clean. Therefore, RelIdToTypeIdCacheHash shouldn't get bloat in the case of temporary tables flood. Discussion: https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1%40sigaev.ru Author: Teodor Sigaev Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov Reviewed-by: Andrei Lepikhov, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c14d4acb81348a34497b53a19dce2924cc4f6551 Modified Files -- src/backend/utils/cache/typcache.c | 275 +++-- src/tools/pgindent/typedefs.list | 1 + 2 files changed, 232 insertions(+), 44 deletions(-)
pgsql: Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) comm
Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) commands This commit reverts 1adf16b8fb, 87c21bb941, and subsequent fixes and improvements including df64c81ca9, c99ef1811a, 9dfcac8e15, 885742b9f8, 842c9b2705, fcf80c5d5f, 96c7381c4c, f4fc7cb54b, 60ae37a8bc, 259c96fa8f, 449cdcd486, 3ca43dbbb6, 2a679ae94e, 3a82c689fd, fbd4321fd5, d53a4286d7, c086896625, 4e5d6c4091, 04158e7fa3. The reason for reverting is security issues related to repeatable name lookups (CVE-2014-0062). Even though 04158e7fa3 solved part of the problem, there are still remaining issues, which aren't feasible to even carefully analyze before the RC deadline. Reported-by: Noah Misch, Robert Haas Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com Backpatch-through: 17 Branch -- REL_17_STABLE Details --- https://git.postgresql.org/pg/commitdiff/84f594da358861cceeaeb7a97bb58f3765eeb284 Modified Files -- doc/src/sgml/ddl.sgml | 38 - doc/src/sgml/ref/alter_table.sgml | 165 +- src/backend/commands/tablecmds.c | 819 +- src/backend/parser/gram.y | 61 +- src/backend/parser/parse_utilcmd.c | 193 +-- src/backend/partitioning/partbounds.c | 901 +-- src/backend/tcop/utility.c |6 - src/backend/utils/adt/ruleutils.c | 18 - src/bin/psql/tab-complete.c| 18 +- src/include/nodes/parsenodes.h | 14 +- src/include/parser/kwlist.h|2 - src/include/partitioning/partbounds.h | 11 - src/include/utils/ruleutils.h |2 - src/test/isolation/expected/partition-merge.out| 199 --- src/test/isolation/expected/partition-split.out| 190 --- src/test/isolation/isolation_schedule |2 - src/test/isolation/specs/partition-merge.spec | 54 - src/test/isolation/specs/partition-split.spec | 54 - .../modules/test_ddl_deparse/test_ddl_deparse.c|6 - src/test/regress/expected/partition_merge.out | 945 src/test/regress/expected/partition_split.out | 1589 src/test/regress/parallel_schedule |2 +- src/test/regress/sql/partition_merge.sql | 609 src/test/regress/sql/partition_split.sql | 962 src/tools/pgindent/typedefs.list |2 - 25 files changed, 40 insertions(+), 6822 deletions(-)
pgsql: Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) comm
Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) commands This commit reverts 1adf16b8fb, 87c21bb941, and subsequent fixes and improvements including df64c81ca9, c99ef1811a, 9dfcac8e15, 885742b9f8, 842c9b2705, fcf80c5d5f, 96c7381c4c, f4fc7cb54b, 60ae37a8bc, 259c96fa8f, 449cdcd486, 3ca43dbbb6, 2a679ae94e, 3a82c689fd, fbd4321fd5, d53a4286d7, c086896625, 4e5d6c4091, 04158e7fa3. The reason for reverting is security issues related to repeatable name lookups (CVE-2014-0062). Even though 04158e7fa3 solved part of the problem, there are still remaining issues, which aren't feasible to even carefully analyze before the RC deadline. Reported-by: Noah Misch, Robert Haas Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com Backpatch-through: 17 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3890d90c1508125729ed20038d90513694fc3a7b Modified Files -- doc/src/sgml/ddl.sgml | 38 - doc/src/sgml/ref/alter_table.sgml | 165 +- src/backend/commands/tablecmds.c | 819 +- src/backend/parser/gram.y | 61 +- src/backend/parser/parse_utilcmd.c | 193 +-- src/backend/partitioning/partbounds.c | 901 +-- src/backend/tcop/utility.c |6 - src/backend/utils/adt/ruleutils.c | 18 - src/bin/psql/tab-complete.c| 18 +- src/include/nodes/parsenodes.h | 16 - src/include/parser/kwlist.h|2 - src/include/partitioning/partbounds.h | 11 - src/include/utils/ruleutils.h |2 - src/test/isolation/expected/partition-merge.out| 199 --- src/test/isolation/expected/partition-split.out| 190 --- src/test/isolation/isolation_schedule |2 - src/test/isolation/specs/partition-merge.spec | 54 - src/test/isolation/specs/partition-split.spec | 54 - .../modules/test_ddl_deparse/test_ddl_deparse.c|6 - src/test/regress/expected/partition_merge.out | 945 src/test/regress/expected/partition_split.out | 1589 src/test/regress/parallel_schedule |2 +- src/test/regress/sql/partition_merge.sql | 609 src/test/regress/sql/partition_split.sql | 962 src/tools/pgindent/typedefs.list |2 - 25 files changed, 36 insertions(+), 6828 deletions(-)
pgsql: Avoid repeated table name lookups in createPartitionTable()
Avoid repeated table name lookups in createPartitionTable() Currently, createPartitionTable() opens newly created table using its name. This approach is prone to privilege escalation attack, because we might end up opening another table than we just created. This commit address the issue above by opening newly created table by its OID. It appears to be tricky to get a relation OID out of ProcessUtility(). We have to extend TableLikeClause with new newRelationOid field, which is filled within ProcessUtility() to be further accessed by caller. Security: CVE-2014-0062 Reported-by: Noah Misch Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com Reviewed-by: Pavel Borisov, Dmitry Koval Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/04158e7fa37c2dda9c3421ca922d02807b86df19 Modified Files -- src/backend/commands/tablecmds.c | 3 ++- src/backend/parser/gram.y| 1 + src/backend/tcop/utility.c | 6 ++ src/include/nodes/parsenodes.h | 1 + 4 files changed, 10 insertions(+), 1 deletion(-)
pgsql: Avoid repeated table name lookups in createPartitionTable()
Avoid repeated table name lookups in createPartitionTable() Currently, createPartitionTable() opens newly created table using its name. This approach is prone to privilege escalation attack, because we might end up opening another table than we just created. This commit address the issue above by opening newly created table by its OID. It appears to be tricky to get a relation OID out of ProcessUtility(). We have to extend TableLikeClause with new newRelationOid field, which is filled within ProcessUtility() to be further accessed by caller. Security: CVE-2014-0062 Reported-by: Noah Misch Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com Reviewed-by: Pavel Borisov, Dmitry Koval Branch -- REL_17_STABLE Details --- https://git.postgresql.org/pg/commitdiff/f636ab41aba215eaa3303e21a10f12d81357f1f6 Modified Files -- src/backend/commands/tablecmds.c | 3 ++- src/backend/parser/gram.y| 1 + src/backend/tcop/utility.c | 6 ++ src/include/nodes/parsenodes.h | 1 + 4 files changed, 10 insertions(+), 1 deletion(-)
Re: pgsql: Add injection-point test for new multixact CV usage
On Tue, Aug 20, 2024 at 9:35 PM Alvaro Herrera wrote: > Add injection-point test for new multixact CV usage > > Before commit a0e0fb1ba56f, multixact.c contained a case in the > multixact-read path where it would loop sleeping 1ms each time until > another multixact-create path completed, which was uncovered by any > tests. That commit changed the code to rely on a condition variable > instead. Add a test now, which relies on injection points and "loading" > thereof (because of it being in a critical section), per commit > 4b211003ecc2. > > Author: Andrey Borodin > Reviewed-by: Michaël Paquier > Discussion: > https://postgr.es/m/0925f9a9-4d53-4b27-a87e-3d83a757b...@yandex-team.ru > > Branch > -- > master > > Details > --- > https://git.postgresql.org/pg/commitdiff/768a9fd5535fddb781088b6f83132b9a1b1f5bd3 > > Modified Files > -- > src/backend/access/transam/multixact.c| 5 ++ > src/test/modules/test_slru/Makefile | 7 +- > src/test/modules/test_slru/meson.build| 9 ++ > src/test/modules/test_slru/t/001_multixact.pl | 124 ++ > src/test/modules/test_slru/test_multixact.c | 57 > src/test/modules/test_slru/test_slru--1.0.sql | 6 ++ > 6 files changed, 207 insertions(+), 1 deletion(-) It seems that header files aren't alphabetically ordered here. #include "storage/proc.h" #include "storage/procarray.h" #include "utils/fmgrprotos.h" +#include "utils/injection_point.h" #include "utils/guc_hooks.h" #include "utils/memutils.h" -- Regards, Alexander Korotkov Supabase
pgsql: Add missing wait_for_catchup() to pg_visibility tap test
Add missing wait_for_catchup() to pg_visibility tap test e2ed7e32271a introduced check of pg_visibility on standby. This commit adds missing wait_for_catchup() to synchronize standby before querying it. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/e3ec9dc1bf4983fcedb6f43c71ea12ee26aefc7a Modified Files -- contrib/pg_visibility/t/001_concurrent_transaction.pl | 1 + 1 file changed, 1 insertion(+)
Re: pgsql: Fix GetStrictOldestNonRemovableTransactionId() on standby
On Fri, Aug 16, 2024 at 12:19 AM Alexander Korotkov wrote: > > Fix GetStrictOldestNonRemovableTransactionId() on standby > > e85662df44 implemented GetStrictOldestNonRemovableTransactionId() function > for computation of xid horizon that avoid reporting of false errors. > However, GetStrictOldestNonRemovableTransactionId() uses > GetRunningTransactionData() even on standby leading to an assertion failure. > > Given that we decided to ignore KnownAssignedXids and standby can't have > own running xids, we switch to use TransamVariables->nextXid as a xid horizon. > > Also, revise the comment regarding ignoring KnownAssignedXids with more > detailed reasoning provided by Heikki. I see the buildfarm failures. I will try to fix them shortly. -- Regards, Alexander Korotkov Supabase
pgsql: Fix GetStrictOldestNonRemovableTransactionId() on standby
Fix GetStrictOldestNonRemovableTransactionId() on standby e85662df44 implemented GetStrictOldestNonRemovableTransactionId() function for computation of xid horizon that avoid reporting of false errors. However, GetStrictOldestNonRemovableTransactionId() uses GetRunningTransactionData() even on standby leading to an assertion failure. Given that we decided to ignore KnownAssignedXids and standby can't have own running xids, we switch to use TransamVariables->nextXid as a xid horizon. Also, revise the comment regarding ignoring KnownAssignedXids with more detailed reasoning provided by Heikki. Reported-by: Heikki Linnakangas Discussion: https://postgr.es/m/42218c4f-2c8d-40a3-8743-4d34dd0e4cce%40iki.fi Reviewed-by: Heikki Linnakangas Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/e2ed7e32271a82179c3f8c7c93ce52ff93c6dd3c Modified Files -- contrib/pg_visibility/pg_visibility.c | 26 +++--- .../pg_visibility/t/001_concurrent_transaction.pl | 19 ++-- 2 files changed, 40 insertions(+), 5 deletions(-)
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
On Thu, Aug 15, 2024 at 2:07 AM Michael Paquier wrote: > > On Aug 15, 2024, at 6:26, Alexander Korotkov wrote: > > I mean, I'd like to push it if you don't object. > > If you object and like to push it yourself, feel free to use this patch. > > I’d like to take a look at what you have here to get an opinion. Could you > wait for a few days? Sure, NP. -- Regards, Alexander Korotkov Supabase
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
On Wed, Aug 14, 2024 at 5:13 AM Alexander Korotkov wrote: > On Tue, Aug 13, 2024 at 1:30 AM Michael Paquier wrote: > > > On Aug 13, 2024, at 6:35, Alexander Korotkov > > > wrote:. > > > > > > As pointed by Noah Misch [1], unlike the commit the patch [2] also > > > changed segment-returning functions to return int64. Thus, in the > > > patch output formats make much more sense, because they match the > > > input data types. Michael, are you intended to push the remaining > > > part of the patch [2]? > > > > Guess so. I could look at that next week, not before. > > OK. I made an attached patch from the missing parts. > Do you mind if I push that? If yes, feel free to use it. I mean, I'd like to push it if you don't object. If you object and like to push it yourself, feel free to use this patch. -- Regards, Alexander Korotkov Supabase
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
On Tue, Aug 13, 2024 at 1:30 AM Michael Paquier wrote: > > On Aug 13, 2024, at 6:35, Alexander Korotkov wrote:. > > > > As pointed by Noah Misch [1], unlike the commit the patch [2] also > > changed segment-returning functions to return int64. Thus, in the > > patch output formats make much more sense, because they match the > > input data types. Michael, are you intended to push the remaining > > part of the patch [2]? > > Guess so. I could look at that next week, not before. OK. I made an attached patch from the missing parts. Do you mind if I push that? If yes, feel free to use it. -- Regards, Alexander Korotkov Supabase v1-0001-Fix-even-more-holes-with-SLRU-code-in-need-of-int.patch Description: Binary data
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
On Fri, Aug 9, 2024 at 4:09 PM Peter Eisentraut wrote: > On 08.08.24 01:15, Michael Paquier wrote: > > > >> On Aug 8, 2024, at 5:05, Alexander Korotkov wrote: > >> On Wed, Aug 7, 2024 at 10:52 PM Peter Eisentraut > >> wrote: > >>> It looks like the commit I'm talking about here is a subset of v55-0001 > >>> from that thread? > >> > >> Yes, looks like this. > >> > >>> So why is some of this being committed now into v17? > >>> But as I wrote above, I think this approach is a bad idea. > >> > >> OK, I agree that might look annoying. So, it's better to revert now. > >> Michael, what do you think? > > > > The argument is two-fold here. The point of this change is that we were > > forcibly doing a cast to int with int64 values returned, so this commit > > limits the risks of missing paths in the future, while being consistent > > with all the SLRU code marking segment numbers with int64 for short *and* > > long segment file names. > > No, this is not what *this* patch does. (I suppose some of the related > patches might be doing that.) This patch just casts a few things that > are int to unsigned long long int before printing them. As pointed by Noah Misch [1], unlike the commit the patch [2] also changed segment-returning functions to return int64. Thus, in the patch output formats make much more sense, because they match the input data types. Michael, are you intended to push the remaining part of the patch [2]? Links 1. https://www.postgresql.org/message-id/20240810175055.cd.nmisch%40google.com 2. https://www.postgresql.org/message-id/ZqGvzSbW5TGKqZcE%40paquier.xyz -- Regards, Alexander Korotkov Supabase
pgsql: Add tests for pg_wal_replay_wait() errors
Add tests for pg_wal_replay_wait() errors Improve test coverage for pg_wal_replay_wait() procedure by adding test cases when it errors out. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0868d7ae70e57f4eeba43792db92ce2d9fe4340d Modified Files -- src/test/recovery/t/043_wal_replay_wait.pl | 42 -- 1 file changed, 40 insertions(+), 2 deletions(-)
pgsql: Adjust pg_wal_replay_wait() procedure behavior on promoted stand
Adjust pg_wal_replay_wait() procedure behavior on promoted standby pg_wal_replay_wait() is intended to be called on standby. However, standby can be promoted to primary at any moment, even concurrently with the pg_wal_replay_wait() call. If recovery is not currently in progress that doesn't mean the wait was unsuccessful. Thus, we always need to recheck if the target LSN is replayed. Reported-by: Kevin Hale Boyes Discussion: https://postgr.es/m/CAPpHfdu5QN%2BZGACS%2B7foxmr8_nekgA2PA%2B-G3BuOUrdBLBFb6Q%40mail.gmail.com Author: Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/867d396ccd2a7f0ce55e1fa7ebda00bc8c81147b Modified Files -- doc/src/sgml/func.sgml | 9 +++ src/backend/commands/waitlsn.c | 42 +++--- src/test/recovery/t/043_wal_replay_wait.pl | 15 +-- 3 files changed, 55 insertions(+), 11 deletions(-)
pgsql: Improve header comment for WaitLSNSetLatches()
Improve header comment for WaitLSNSetLatches() Reflect the fact that we remove waiters from the heap, not just set their latches. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3ac3ec580c6f4f991d32252814e4b04c0e903a41 Modified Files -- src/backend/commands/waitlsn.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Re: pgsql: Introduce hash_search_with_hash_value() function
On Thu, Aug 8, 2024 at 7:45 AM Pavel Stehule wrote: > st 7. 8. 2024 v 22:25 odesílatel Alexander Korotkov > napsal: >> >> On Wed, Aug 7, 2024 at 1:34 PM Pavel Stehule wrote: >> > st 7. 8. 2024 v 12:22 odesílatel Alexander Korotkov >> > napsal: >> >> Thank you. >> >> Please see 40064a8ee1 takes special efforts to match HTAB hash >> >> function to syscache hash function. By default, their hash values >> >> don't match and you can't simply use syscache hash value to search >> >> HTAB. This probably should be mentioned in the header comment of >> >> hash_seq_init_with_hash_value(). >> > >> > >> > yes, enhancing doc should be great + maybe assert >> >> Please check the patch, which adds a caveat to the function header >> comment. I don't particularly like an assert here, because there >> could be use-cases besides syscache callbacks, which could legally use >> default hash function. > > > looks well Thank you, pushed. -- Regards, Alexander Korotkov Supabase
pgsql: Add a caveat to hash_seq_init_with_hash_value() header comment
Add a caveat to hash_seq_init_with_hash_value() header comment The typical use-case for hash_seq_init_with_hash_value() is syscache callback. Add a caveat that the default hash function doesn't match syscache hash function. So, one needs to define a custom hash function. Reported-by: Pavel Stehule Discussion: https://postgr.es/m/CAFj8pRAXmv6eyYx%3DE_BTfyK%3DO_%2ByOF8sXB%3D0bn9eOBt90EgWRA%40mail.gmail.com Reviewed-by: Pavel Stehule Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d0c8cf2a56fadb08705433bffb301559d97b0712 Modified Files -- src/backend/utils/hash/dynahash.c | 5 + 1 file changed, 5 insertions(+)
Re: pgsql: Introduce hash_search_with_hash_value() function
On Wed, Aug 7, 2024 at 1:34 PM Pavel Stehule wrote: > st 7. 8. 2024 v 12:22 odesílatel Alexander Korotkov > napsal: >> Thank you. >> Please see 40064a8ee1 takes special efforts to match HTAB hash >> function to syscache hash function. By default, their hash values >> don't match and you can't simply use syscache hash value to search >> HTAB. This probably should be mentioned in the header comment of >> hash_seq_init_with_hash_value(). > > > yes, enhancing doc should be great + maybe assert Please check the patch, which adds a caveat to the function header comment. I don't particularly like an assert here, because there could be use-cases besides syscache callbacks, which could legally use default hash function. -- Regards, Alexander Korotkov Supabase v1-0001-Add-a-caveat-to-hash_seq_init_with_hash_value-hea.patch Description: Binary data
Re: pgsql: Introduce hash_search_with_hash_value() function
On Wed, Aug 7, 2024 at 7:30 PM Robert Haas wrote: > On Wed, Aug 7, 2024 at 11:55 AM Alexander Korotkov > wrote: > > On Wed, Aug 7, 2024 at 3:24 PM Robert Haas wrote: > > > You may have already realized this, but the name of the function the > > > patch adds is not the same as the name that appears in the commit > > > message. > > > > :sigh: > > I didn't realize that before your message. That would be another item > > for my checklist: ensure entities referenced from commit message and > > comments didn't change their names. > > I really wish there was some way to fix commit messages. I had a typo > in mine today, too. +1, One of the scariest things that happened to me was forgetting to mention reviewers or even authors. People don't get credit for their work, and you can't fix that. -- Regards, Alexander Korotkov Supabase
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
On Wed, Aug 7, 2024 at 10:52 PM Peter Eisentraut wrote: > On 07.08.24 17:53, Alexander Korotkov wrote: > > On Wed, Aug 7, 2024 at 12:07 PM Peter Eisentraut > > wrote: > >> On 27.07.24 00:24, Michael Paquier wrote: > >>> Fix more holes with SLRU code in need of int64 for segment numbers > >>> > >>> This is a continuation of 3937cadfd438, taking care of more areas I have > >>> managed to miss previously. > >>> > >>> Reported-by: Noah Misch > >>> Reviewed-by: Noah Misch > >>> Discussion: https://postgr.es/m/20240724130059.1f.nmi...@google.com > >>> Backpatch-through: 17 > >>> > >>> Branch > >>> -- > >>> master > >>> > >>> Details > >>> --- > >>> https://git.postgresql.org/pg/commitdiff/c9e24573905bef7fc3e4efb02bdb4d0cc8e43c51 > >> > >> I don't understand this patch. The previous patches that this > >> references changed various variables to int64 and made adjustments > >> following from that. But this patch takes variables and function > >> results that are of type int and casts them to unsigned long long before > >> printing. I don't see what that accomplishes, and it's not clear based > >> on just the explanation that this is a continuation of a previous patch > >> that doesn't do that. Is there a plan to change these things to int64 > >> as well at some point? > > > > There is a plan indeed. The patchset [1] should include conversion > > multixacts to 64-bit (It surely included that in earlier versions, I > > didn't look the last versions though). I doubt this will be ready for > > v18. So this commit might be quite preliminary. But I would prefer > > to leave it there as soon as it has already landed. Opinions? > > I think you should change the output formats at the same time as you > change the variable types. That way the compiler can cross-check this. > Otherwise, if you later forget to change a variable, these casts will > hide it. Or if the future patches turn out differently, then we have > this useless code. > > > Links. > > 1. > > https://www.postgresql.org/message-id/CAJ7c6TND0bCnwU1SmxTsFewK4XJGBep343vf%2BT%2BGQ-a5S5hC0w%40mail.gmail.com > > It looks like the commit I'm talking about here is a subset of v55-0001 > from that thread? Yes, looks like this. > So why is some of this being committed now into v17? > But as I wrote above, I think this approach is a bad idea. OK, I agree that might look annoying. So, it's better to revert now. Michael, what do you think? -- Regards, Alexander Korotkov Supabase
Re: pgsql: Introduce hash_search_with_hash_value() function
On Wed, Aug 7, 2024 at 3:24 PM Robert Haas wrote: > You may have already realized this, but the name of the function the > patch adds is not the same as the name that appears in the commit > message. :sigh: I didn't realize that before your message. That would be another item for my checklist: ensure entities referenced from commit message and comments didn't change their names. -- Regards, Alexander Korotkov Supabase
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
On Wed, Aug 7, 2024 at 12:07 PM Peter Eisentraut wrote: > On 27.07.24 00:24, Michael Paquier wrote: > > Fix more holes with SLRU code in need of int64 for segment numbers > > > > This is a continuation of 3937cadfd438, taking care of more areas I have > > managed to miss previously. > > > > Reported-by: Noah Misch > > Reviewed-by: Noah Misch > > Discussion: https://postgr.es/m/20240724130059.1f.nmi...@google.com > > Backpatch-through: 17 > > > > Branch > > -- > > master > > > > Details > > --- > > https://git.postgresql.org/pg/commitdiff/c9e24573905bef7fc3e4efb02bdb4d0cc8e43c51 > > I don't understand this patch. The previous patches that this > references changed various variables to int64 and made adjustments > following from that. But this patch takes variables and function > results that are of type int and casts them to unsigned long long before > printing. I don't see what that accomplishes, and it's not clear based > on just the explanation that this is a continuation of a previous patch > that doesn't do that. Is there a plan to change these things to int64 > as well at some point? There is a plan indeed. The patchset [1] should include conversion multixacts to 64-bit (It surely included that in earlier versions, I didn't look the last versions though). I doubt this will be ready for v18. So this commit might be quite preliminary. But I would prefer to leave it there as soon as it has already landed. Opinions? Links. 1. https://www.postgresql.org/message-id/CAJ7c6TND0bCnwU1SmxTsFewK4XJGBep343vf%2BT%2BGQ-a5S5hC0w%40mail.gmail.com -- Regards, Alexander Korotkov Supabase
Re: pgsql: Introduce hash_search_with_hash_value() function
On Wed, Aug 7, 2024 at 1:03 PM Pavel Stehule wrote:
>
> st 7. 8. 2024 v 10:52 odesílatel Alexander Korotkov
> napsal:
>>
>> Hi, Pavel!
>>
>> On Wed, Aug 7, 2024 at 9:35 AM Pavel Stehule wrote:
>> > st 7. 8. 2024 v 6:08 odesílatel Alexander Korotkov
>> > napsal:
>> >>
>> >> Introduce hash_search_with_hash_value() function
>> >>
>> >> This new function iterates hash entries with given hash values. This
>> >> function
>> >> is designed to avoid full sequential hash search in the syscache
>> >> invalidation
>> >> callbacks.
>> >>
>> >> Discussion:
>> >> https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1%40sigaev.ru
>> >> Author: Teodor Sigaev
>> >> Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov
>> >> Reviewed-by: Andrei Lepikhov
>> >
>> >
>> > I tried to use hash_seq_init_with_hash_value in session variables patch,
>> > but it doesn't work there.
>> >
>> > <-->if (!sessionvars)
>> > <--><-->return;
>> >
>> > elog(NOTICE, "%u", hashvalue);
>> >
>> >
>> > <-->hash_seq_init(&status, sessionvars);
>> >
>> > <-->while ((svar = (SVariable) hash_seq_search(&status)) != NULL)
>> > <-->{
>> > <--><-->if (hashvalue == 0 || svar->hashvalue == hashvalue)
>> > <--><-->{
>> > <--><--><-->elog(NOTICE, "FOUND OLD");
>> > <--><--><-->svar->is_valid = false;
>> > <--><-->}
>> > <-->}
>> >
>> >
>> >
>> > <-->/*
>> > <--> * If the hashvalue is not specified, we have to recheck all currently
>> > <--> * used session variables. Since we can't tell the exact session
>> > variable
>> > <--> * from its hashvalue, we have to iterate over all items in the hash
>> > bucket.
>> > <--> */
>> > <-->if (hashvalue == 0)
>> > <--><-->hash_seq_init(&status, sessionvars);
>> > <-->else
>> > <--><-->hash_seq_init_with_hash_value(&status, sessionvars, hashvalue);
>> >
>> > <-->while ((svar = (SVariable) hash_seq_search(&status)) != NULL)
>> > <-->{
>> > <--><-->Assert(hashvalue == 0 || svar->hashvalue == hashvalue);
>> >
>> > elog(NOTICE, "found");
>> >
>> > <--><-->svar->is_valid = false;
>> > <--><-->needs_validation = true;
>> > <-->}
>> > }
>> >
>> > Old methods found an entry, but new not.
>> >
>> > What am I doing wrong?
>>
>> I'm trying to check this. Applying this patch [1], but got conflicts.
>> Could you please, rebase the patch, so I can recheck the issue?
>
> I sent rebased patchset
Thank you.
Please see 40064a8ee1 takes special efforts to match HTAB hash
function to syscache hash function. By default, their hash values
don't match and you can't simply use syscache hash value to search
HTAB. This probably should be mentioned in the header comment of
hash_seq_init_with_hash_value().
--
Regards,
Alexander Korotkov
Supabase
Re: pgsql: Introduce hash_search_with_hash_value() function
Hi, Pavel!
On Wed, Aug 7, 2024 at 9:35 AM Pavel Stehule wrote:
> st 7. 8. 2024 v 6:08 odesílatel Alexander Korotkov
> napsal:
>>
>> Introduce hash_search_with_hash_value() function
>>
>> This new function iterates hash entries with given hash values. This
>> function
>> is designed to avoid full sequential hash search in the syscache invalidation
>> callbacks.
>>
>> Discussion:
>> https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1%40sigaev.ru
>> Author: Teodor Sigaev
>> Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov
>> Reviewed-by: Andrei Lepikhov
>
>
> I tried to use hash_seq_init_with_hash_value in session variables patch, but
> it doesn't work there.
>
> <-->if (!sessionvars)
> <--><-->return;
>
> elog(NOTICE, "%u", hashvalue);
>
>
> <-->hash_seq_init(&status, sessionvars);
>
> <-->while ((svar = (SVariable) hash_seq_search(&status)) != NULL)
> <-->{
> <--><-->if (hashvalue == 0 || svar->hashvalue == hashvalue)
> <--><-->{
> <--><--><-->elog(NOTICE, "FOUND OLD");
> <--><--><-->svar->is_valid = false;
> <--><-->}
> <-->}
>
>
>
> <-->/*
> <--> * If the hashvalue is not specified, we have to recheck all currently
> <--> * used session variables. Since we can't tell the exact session variable
> <--> * from its hashvalue, we have to iterate over all items in the hash
> bucket.
> <--> */
> <-->if (hashvalue == 0)
> <--><-->hash_seq_init(&status, sessionvars);
> <-->else
> <--><-->hash_seq_init_with_hash_value(&status, sessionvars, hashvalue);
>
> <-->while ((svar = (SVariable) hash_seq_search(&status)) != NULL)
> <-->{
> <--><-->Assert(hashvalue == 0 || svar->hashvalue == hashvalue);
>
> elog(NOTICE, "found");
>
> <--><-->svar->is_valid = false;
> <--><-->needs_validation = true;
> <-->}
> }
>
> Old methods found an entry, but new not.
>
> What am I doing wrong?
I'm trying to check this. Applying this patch [1], but got conflicts.
Could you please, rebase the patch, so I can recheck the issue?
Links.
1.
https://www.postgresql.org/message-id/flat/CAFj8pRD053CY_N4%3D6SvPe7ke6xPbh%3DK50LUAOwjC3jm8Me9Obg%40mail.gmail.com
--
Regards,
Alexander Korotkov
Supabase
pgsql: Optimize InvalidateAttoptCacheCallback() and TypeCacheTypCallbac
Optimize InvalidateAttoptCacheCallback() and TypeCacheTypCallback() These callbacks are receiving hash values as arguments, which doesn't allow direct lookups for AttoptCacheHash and TypeCacheHash. This is why subject callbacks currently use full iteration over corresponding hashes. This commit avoids full hash iteration in InvalidateAttoptCacheCallback(), and TypeCacheTypCallback(). At first, we switch AttoptCacheHash and TypeCacheHash to use same hash function as syscache. As second, we use hash_seq_init_with_hash_value() to iterate only hash entries with matching hash value. Discussion: https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1%40sigaev.ru Author: Teodor Sigaev Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov Reviewed-by: Andrei Lepikhov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/40064a8ee1b34d8a128d6007416acd89077a2c11 Modified Files -- src/backend/utils/cache/attoptcache.c | 39 + src/backend/utils/cache/typcache.c| 55 +-- 2 files changed, 73 insertions(+), 21 deletions(-)
pgsql: Introduce hash_search_with_hash_value() function
Introduce hash_search_with_hash_value() function This new function iterates hash entries with given hash values. This function is designed to avoid full sequential hash search in the syscache invalidation callbacks. Discussion: https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1%40sigaev.ru Author: Teodor Sigaev Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov Reviewed-by: Andrei Lepikhov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d0f020037e19c33c74d683eb7e0c7cc5725294b4 Modified Files -- src/backend/utils/hash/dynahash.c | 38 ++ src/include/utils/hsearch.h | 5 + 2 files changed, 43 insertions(+)
pgsql: pg_wal_replay_wait(): Fix typo in the doc
pg_wal_replay_wait(): Fix typo in the doc Reported-by: Kevin Hale Boyes Discussion: https://postgr.es/m/CADAecHWKpaPuPGXAMOH%3DwmhTpydHWGPOk9KWX97UYhp5GdqCWw%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/8036d73ae3d4014a9dde21b0746dc1ac139d4dc1 Modified Files -- doc/src/sgml/func.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Implement pg_wal_replay_wait() stored procedure
Implement pg_wal_replay_wait() stored procedure pg_wal_replay_wait() is to be used on standby and specifies waiting for the specific WAL location to be replayed. This option is useful when the user makes some data changes on primary and needs a guarantee to see these changes are on standby. The queue of waiters is stored in the shared memory as an LSN-ordered pairing heap, where the waiter with the nearest LSN stays on the top. During the replay of WAL, waiters whose LSNs have already been replayed are deleted from the shared memory pairing heap and woken up by setting their latches. pg_wal_replay_wait() needs to wait without any snapshot held. Otherwise, the snapshot could prevent the replay of WAL records, implying a kind of self-deadlock. This is why it is only possible to implement pg_wal_replay_wait() as a procedure working without an active snapshot, not a function. Catversion is bumped. Discussion: https://postgr.es/m/eb12f9b03851bb2583adab5df9579b4b%40postgrespro.ru Author: Kartyshov Ivan, Alexander Korotkov Reviewed-by: Michael Paquier, Peter Eisentraut, Dilip Kumar, Amit Kapila Reviewed-by: Alexander Lakhin, Bharath Rupireddy, Euler Taveira Reviewed-by: Heikki Linnakangas, Kyotaro Horiguchi Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3c5db1d6b01642bcd8dbf5e34b68f034365747bb Modified Files -- doc/src/sgml/func.sgml | 117 src/backend/access/transam/xact.c | 6 + src/backend/access/transam/xlog.c | 7 + src/backend/access/transam/xlogrecovery.c | 11 + src/backend/catalog/system_functions.sql| 3 + src/backend/commands/Makefile | 3 +- src/backend/commands/meson.build| 1 + src/backend/commands/waitlsn.c | 363 src/backend/lib/pairingheap.c | 18 +- src/backend/storage/ipc/ipci.c | 3 + src/backend/storage/lmgr/proc.c | 6 + src/backend/tcop/pquery.c | 9 +- src/backend/utils/activity/wait_event_names.txt | 2 + src/include/catalog/catversion.h| 2 +- src/include/catalog/pg_proc.dat | 6 + src/include/commands/waitlsn.h | 80 ++ src/include/lib/pairingheap.h | 3 + src/include/storage/lwlocklist.h| 1 + src/test/recovery/meson.build | 1 + src/test/recovery/t/043_wal_replay_wait.pl | 150 ++ src/tools/pgindent/typedefs.list| 2 + 21 files changed, 786 insertions(+), 8 deletions(-)
pgsql: amcheck: Optimize speed of checking for unique constraint violat
amcheck: Optimize speed of checking for unique constraint violation Currently, when amcheck validates a unique constraint, it visits the heap for each index tuple. This commit implements skipping keys, which have only one non-dedeuplicated index tuple (quite common case for unique indexes). That gives substantial economy on index checking time. Reported-by: Noah Misch Discussion: https://postgr.es/m/20240325020323.fd.nmisch%40google.com Author: Alexander Korotkov, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/cdd6ab9d1f5396ec1097d51c21a224aa41118c9c Modified Files -- contrib/amcheck/verify_nbtree.c | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-)
Re: pgsql: Wait for WAL summarization to catch up before creating .partial
On Sun, Jul 28, 2024 at 7:12 AM Tom Lane wrote: > Alexander Korotkov writes: > > It appears that I was late with my review [1]. But the new tap test > > could still use pgperltidy. > > I believe our current policy is that we're asking committers to > maintain pgindent cleanliness, but not pgperltidy (which is why > BF member koel isn't checking pgperltidy). We might get to that > eventually, but we're not there yet. Got it, thank you. Sorry for noise, then. -- Regards, Alexander Korotkov Supabase
Re: pgsql: Wait for WAL summarization to catch up before creating .partial
On Fri, Jul 26, 2024 at 10:01 PM Robert Haas wrote: > Wait for WAL summarization to catch up before creating .partial file. > > When a standby is promoted, CleanupAfterArchiveRecovery() may decide > to rename the final WAL file from the old timeline by adding ".partial" > to the name. If WAL summarization is enabled and this file is renamed > before its partial contents are summarized, WAL summarization breaks: > the summarizer gets stuck at that point in the WAL stream and just > errors out. > > To fix that, first make the startup process wait for WAL summarization > to catch up before renaming the file. Generally, this should be quick, > and if it's not, the user can shut off summarize_wal and try again. > To make this fix work, also teach the WAL summarizer that after a > promotion has occurred, no more WAL can appear on the previous > timeline: previously, the WAL summarizer wouldn't switch to the new > timeline until we actually started writing WAL there, but that meant > that when the startup process was waiting for the WAL summarizer, it > was waiting for an action that the summarizer wasn't yet prepared to > take. > > In the process of fixing these bugs, I realized that the logic to wait > for WAL summarization to catch up was spread out in a way that made > it difficult to reuse properly, so this code refactors things to make > it easier. > > Finally, add a test case that would have caught this bug and the > previously-fixed bug that WAL summarization sometimes needs to back up > when the timeline changes. It appears that I was late with my review [1]. But the new tap test could still use pgperltidy. Links. 1. https://www.postgresql.org/message-id/CAPpHfduW3du0W%3D3noztdaJ6evGP9gqT1AGk_rwXrqDyus1zZoQ%40mail.gmail.com -- Regards, Alexander Korotkov Supabase
pgsql: Fix typo in GetRunningTransactionData()
Fix typo in GetRunningTransactionData() e85662df44 made GetRunningTransactionData() calculate the oldest running transaction id within the current database. However, because of the typo, the new code uses oldestRunningXid instead of oldestDatabaseRunningXid in comparison before updating oldestDatabaseRunningXid. This commit fixes that issue. Reported-by: Noah Misch Discussion: https://postgr.es/m/20240630231816.bf.nmisch%40google.com Backpatch-through: 17 Branch -- REL_17_STABLE Details --- https://git.postgresql.org/pg/commitdiff/619f76cce11dc51458eb4ea81b0e48d15d0b2d07 Modified Files -- src/backend/storage/ipc/procarray.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix typo in GetRunningTransactionData()
Fix typo in GetRunningTransactionData() e85662df44 made GetRunningTransactionData() calculate the oldest running transaction id within the current database. However, because of the typo, the new code uses oldestRunningXid instead of oldestDatabaseRunningXid in comparison before updating oldestDatabaseRunningXid. This commit fixes that issue. Reported-by: Noah Misch Discussion: https://postgr.es/m/20240630231816.bf.nmisch%40google.com Backpatch-through: 17 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6c1af5482e6943a5f29b7f4ca773c720ec8202b0 Modified Files -- src/backend/storage/ipc/procarray.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Optimize memory access in GetRunningTransactionData()
Optimize memory access in GetRunningTransactionData() e85662df44 made GetRunningTransactionData() calculate the oldest running transaction id within the current database. This commit optimized this calculation by performing a cheap transaction id comparison before fetching the process database id, while the latter could cause extra cache misses. Reported-by: Noah Misch Discussion: https://postgr.es/m/20240630231816.bf.nmisch%40google.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6897f0ec024582a570868939d3f34a6853374723 Modified Files -- src/backend/storage/ipc/procarray.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-)
pgsql: Remove extra comment at TableAmRoutine.scan_analyze_next_block
Remove extra comment at TableAmRoutine.scan_analyze_next_block The extra comment was accidentally copied here by 6377e12a from heapam_scan_analyze_next_block(). Reported-by: Matthias van de Meent Discussion: https://postgr.es/m/CAEze2WjC5PiweG-4oe0hB_Zr59iF3tRE1gURm8w4Cg5b6JEBGw%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/70a845c04a47645b58f8276a6b3ab201ea8ec426 Modified Files -- src/include/access/tableam.h | 10 -- 1 file changed, 10 deletions(-)
pgsql: Add doc entry for the new GUC paramenter enable_group_by_reorder
Add doc entry for the new GUC paramenter enable_group_by_reordering 0452b461bc4 adds alternative orderings of group-by keys during the query optimization. This new feature is controlled by the new GUC parameter enable_group_by_reordering, which accidentally came without the documentation. This commit adds the missing documentation for that GUC. Reported-by: Bruce Momjian Discussion: https://postgr.es/m/ZnDx2FYlba_OafQd%40momjian.us Author: Andrei Lepikhov Reviewed-by: Pavel Borisov, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/82e79ee46b1c880cb7376cf4399c9883c1ddfaea Modified Files -- doc/src/sgml/config.sgml | 19 +++ 1 file changed, 19 insertions(+)
pgsql: Fix asymmetry in setting EquivalenceClass.ec_sortref
Fix asymmetry in setting EquivalenceClass.ec_sortref 0452b461bc made get_eclass_for_sort_expr() always set EquivalenceClass.ec_sortref if it's not done yet. This leads to an asymmetric situation when whoever first looks for the EquivalenceClass sets the ec_sortref. It is also counterintuitive that get_eclass_for_sort_expr() performs modification of data structures. This commit makes make_pathkeys_for_sortclauses_extended() responsible for setting EquivalenceClass.ec_sortref. Now we set the EquivalenceClass.ec_sortref's needed to explore alternative GROUP BY ordering specifically during building pathkeys by the list of grouping clauses. Discussion: https://postgr.es/m/17037754-f187-4138-8285-0e2bfebd0dea%40postgrespro.ru Reported-by: Tom Lane Author: Andrei Lepikhov Reviewed-by: Alexander Korotkov, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/199012a3d844c6283e0ab4b1139440840a91433d Modified Files -- src/backend/optimizer/path/equivclass.c | 13 + src/backend/optimizer/path/pathkeys.c| 18 ++-- src/backend/optimizer/plan/planner.c | 16 --- src/include/optimizer/paths.h| 3 +- src/test/regress/expected/aggregates.out | 47 src/test/regress/sql/aggregates.sql | 14 ++ 6 files changed, 92 insertions(+), 19 deletions(-)
pgsql: Add invariants check to get_useful_group_keys_orderings()
Add invariants check to get_useful_group_keys_orderings() This commit introduces invariants checking of generated orderings in get_useful_group_keys_orderings() for assert-enabled builds. Discussion: https://postgr.es/m/a663f0f6-cbf6-49aa-af2e-234dc6768a07%40postgrespro.ru Reported-by: Tom Lane Author: Andrei Lepikhov Reviewed-by: Alexander Korotkov, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/91143c03d4ca36406a53e05cd421b19e47d131d1 Modified Files -- src/backend/optimizer/path/pathkeys.c | 28 1 file changed, 28 insertions(+)
pgsql: Rename PathKeyInfo to GroupByOrdering
Rename PathKeyInfo to GroupByOrdering 0452b461bc made optimizer explore alternative orderings of group-by pathkeys. The PathKeyInfo data structure was used to store the particular ordering of group-by pathkeys and corresponding clauses. It turns out that PathKeyInfo is not the best name for that purpose. This commit renames this data structure to GroupByOrdering, and revises its comment. Discussion: https://postgr.es/m/db0fc3a4-966c-4cec-a136-94024d39212d%40postgrespro.ru Reported-by: Tom Lane Author: Andrei Lepikhov Reviewed-by: Alexander Korotkov, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0c1af2c35c7b456bd2fc76bbc9df5aa9c7911bde Modified Files -- src/backend/optimizer/path/pathkeys.c | 18 +- src/backend/optimizer/plan/planner.c | 8 src/include/nodes/pathnodes.h | 13 ++--- src/tools/pgindent/typedefs.list | 2 +- 4 files changed, 24 insertions(+), 17 deletions(-)
pgsql: Restore preprocess_groupclause()
Restore preprocess_groupclause() 0452b461bc made optimizer explore alternative orderings of group-by pathkeys. It eliminated preprocess_groupclause(), which was intended to match items between GROUP BY and ORDER BY. Instead, get_useful_group_keys_orderings() function generates orderings of GROUP BY elements at the time of grouping paths generation. The get_useful_group_keys_orderings() function takes into account 3 orderings of GROUP BY pathkeys and clauses: original order as written in GROUP BY, matching ORDER BY clauses as much as possible, and matching the input path as much as possible. Given that even before 0452b461b, preprocess_groupclause() could change the original order of GROUP BY clauses we don't need to consider it apart from ordering matching ORDER BY clauses. This commit restores preprocess_groupclause() to provide an ordering of GROUP BY elements matching ORDER BY before generation of paths. The new version of preprocess_groupclause() takes into account an incremental sort. The get_useful_group_keys_orderings() function now takes into 2 orderings of GROUP BY elements: the order generated preprocess_groupclause() and the order matching the input path as much as possible. Discussion: https://postgr.es/m/CAPpHfdvyWLMGwvxaf%3D7KAp-z-4mxbSH8ti2f6mNOQv5metZFzg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Andrei Lepikhov, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/505c008ca37c4f6f2fffcde370b5d8354c4d4dc3 Modified Files -- src/backend/optimizer/path/pathkeys.c | 55 +-- src/backend/optimizer/plan/planner.c | 108 +++--- src/include/nodes/pathnodes.h | 6 +- src/test/regress/expected/partition_aggregate.out | 6 +- 4 files changed, 108 insertions(+), 67 deletions(-)
pgsql: amcheck: Fixes for right page check during unique constraint che
amcheck: Fixes for right page check during unique constraint check * Don't forget to pfree() the right page when it's to be ignored. * Report error on unexpected non-leaf right page even if this page is not to be ignored. This restores the logic which was unintendedly changed in 97e5b0026f. Reported-by: Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/945ec4c4bca1e1c4347cd3f93135e96770ac1b4c Modified Files -- contrib/amcheck/verify_nbtree.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-)
pgsql: Provide deterministic order for catalog queries in partition_spl
Provide deterministic order for catalog queries in partition_split.sql System catalog tables are subject to modification by parallel tests. This is the source of instability when querying them without explicit ORDER BY. This commit adds explicit ORDER BY to system catalog queries in partition_split.sql to stabilize the result. Reported-by: Tom Lane Discussion: https://postgr.es/m/695264.1716578979%40sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d53a4286d772c50ad7a8ff72ca637de613532592 Modified Files -- src/test/regress/expected/partition_split.out | 34 +-- src/test/regress/sql/partition_split.sql | 34 +-- 2 files changed, 34 insertions(+), 34 deletions(-)
pgsql: Don't copy extended statistics during MERGE/SPLIT partition oper
Don't copy extended statistics during MERGE/SPLIT partition operations When MERGE/SPLIT created new partitions, it was cloning the extended statistics of the parent table. However, extended stats on partitioned tables don't behave like indexes on partitioned tables (which exist only to create physical indexes on child tables). Rather, extended stats on a parent 1) cause extended stats to be collected and computed across the whole partition hierarchy, and 2) do not cause extended stats to be computed for the individual partitions. "CREATE TABLE ... PARTITION OF" command doesn't copy extended statistics. This commit makes createPartitionTable() behave consistently. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/ZiJW1g2nbQs9ekwK%40pryzbyj2023 Author: Alexander Korotkov, Justin Pryzby Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/fbd4321fd5b4400fbbf356d686af6ad6d3208c66 Modified Files -- doc/src/sgml/ref/alter_table.sgml | 9 +++-- src/backend/commands/tablecmds.c | 8 +--- 2 files changed, 12 insertions(+), 5 deletions(-)
pgsql: Fix the name collision detection in MERGE/SPLIT partition operat
Fix the name collision detection in MERGE/SPLIT partition operations Both MERGE and SPLIT partition operations support the case when the name of the new partition matches the name of the existing partition to be merged/split. But the name collision detection doesn't always work as intended. The SPLIT partition operation finds the namespace to search for an existing partition without taking into account the parent's persistence. The MERGE partition operation checks for the name collision with simple equal() on RangeVar's simply ignoring the search_path. This commit fixes this behavior as follows. 1. The SPLIT partition operation now finds the namespace to search for an existing partition according to the parent's persistence. 2. The MERGE partition operation now checks for the name collision similarly to the SPLIT partition operation using RangeVarGetAndCheckCreationNamespace() and get_relname_relid(). Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/86b4f1e3-0b5d-315c-9225-19860d64d685%40gmail.com Author: Dmitry Koval, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3a82c689fd1be9bdf9d60135e5db7d352c051269 Modified Files -- src/backend/commands/tablecmds.c | 62 ++- src/test/regress/expected/partition_merge.out | 3 +- src/test/regress/expected/partition_split.out | 8 src/test/regress/sql/partition_merge.sql | 3 +- src/test/regress/sql/partition_split.sql | 9 5 files changed, 72 insertions(+), 13 deletions(-)
pgsql: amcheck: Don't load the right sibling page into BtreeCheckState
amcheck: Don't load the right sibling page into BtreeCheckState 5ae2087202 implemented a cross-page unique constraint check by loading the right sibling to the BtreeCheckState.target variable. This is wrong, because bt_target_page_check() shouldn't change the target page. Also, BtreeCheckState.target shouldn't be changed alone without BtreeCheckState.targetblock. However, the above didn't cause any visible bugs for the following reasons. 1. We do a cross-page unique constraint check only for leaf index pages. 2. The only way target page get accessed after a cross-page unique constraint check is loading of the lowkey. 3. The only place lowkey is used is bt_child_highkey_check(), and that applies only to non-leafs. The reasons above don't diminish the fact that changing BtreeCheckState.target for a cross-page unique constraint check is wrong. This commit changes this check to temporarily store the right sibling to the local variable. Reported-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wzk%2B2116uOXdOViA27SHcr31WKPgmjsxXLBs_aTxAeThzg%40mail.gmail.com Author: Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0b5c161248110b164a006004c78f9529a109 Modified Files -- contrib/amcheck/verify_nbtree.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-)
pgsql: amcheck: Refactoring the storage of the last visible entry
amcheck: Refactoring the storage of the last visible entry This commit introduces a new data structure BtreeLastVisibleEntry comprising information about the last visible heap entry with the current value of key. Usage of this data structure allows us to avoid passing all this information as individual function arguments. Reported-by: Alexander Korotkov Discussion: https://www.postgresql.org/message-id/CAPpHfdsVbB9ToriaB1UHuOKwjKxiZmTFQcEF%3DjuzzC_nby31uA%40mail.gmail.com Author: Pavel Borisov, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/532d94fec32ac11263b53932365560491d1fd50a Modified Files -- contrib/amcheck/verify_nbtree.c | 117 --- src/tools/pgindent/typedefs.list | 1 + 2 files changed, 60 insertions(+), 58 deletions(-)
pgsql: amcheck: Report an error when the next page to a leaf is not a l
amcheck: Report an error when the next page to a leaf is not a leaf This is a very unlikely condition during checking a B-tree unique constraint, meaning that the index structure is violated badly, and we shouldn't continue checking to avoid endless loops, etc. So it's worth immediately throwing an error. Reported-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wzk%2B2116uOXdOViA27SHcr31WKPgmjsxXLBs_aTxAeThzg%40mail.gmail.com Author: Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/97e5b0026fc276ab1bcde58ae98ae1fcd9c3acc3 Modified Files -- contrib/amcheck/verify_nbtree.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-)
pgsql: Fix regression tests conflict in 3ca43dbbb6
Fix regression tests conflict in 3ca43dbbb6 3ca43dbbb6 adds regression tests with permission checks. The conflict has been observed at buildfarm member piculet. This commit fixes the conflict in the following way. 1. partition_split.sql now uses role names regress_partition_split_alice and regress_partition_split_bob (it mistakenly used regress_partition_merge_alice and regress_partition_merge_bob before). 2. Permissions on schemas partitions_merge_schema and partition_split_schema are granted to corresponding roles. Before, the lack of permissions led to the creation of objects in the public schema and potential conflict. Reported-by: Daniel Gustafsson Discussion: https://postgr.es/m/03A07EF6-98D2-419B-A3AA-A111C64CC207%40yesql.se Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/2a679ae94e46ea2fa1ff5461c9d0767faecc6c8c Modified Files -- src/test/regress/expected/partition_merge.out | 4 src/test/regress/expected/partition_split.out | 25 +++-- src/test/regress/sql/partition_merge.sql | 4 src/test/regress/sql/partition_split.sql | 25 +++-- 4 files changed, 38 insertions(+), 20 deletions(-)
pgsql: Add permission check for MERGE/SPLIT partition operations
Add permission check for MERGE/SPLIT partition operations Currently, we check only owner permission for the parent table before MERGE/SPLIT partition operations. This leads to a security hole when users can get access to the data of partitions without permission. This commit fixes this problem by requiring owner permission on all the partitions involved. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/0520c72e-8d97-245e-53f9-173beca2ab2e%40gmail.com Author: Dmitry Koval, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3ca43dbbb67fbfb96dec8de2e268b96790555148 Modified Files -- src/backend/parser/parse_utilcmd.c| 5 src/test/regress/expected/partition_merge.out | 29 +++ src/test/regress/expected/partition_split.out | 29 +++ src/test/regress/sql/partition_merge.sql | 33 +++ src/test/regress/sql/partition_split.sql | 33 +++ 5 files changed, 129 insertions(+)
pgsql: Revert: Remove useless self-joins
Revert: Remove useless self-joins This commit reverts d3d55ce5713 and subsequent fixes 2b26a694554, 93c85db3b5b, b44a1708abe, b7f315c9d7d, 8a8ed916f73, b5fb6736ed3, 0a93f803f45, e0477837ce4, a7928a57b9f, 5ef34a8fc38, 30b4955a466, 8c441c08279, 028b15405b4, fe093994db4, 489072ab7a9, and 466979ef031. We are quite late in the release cycle and new bugs continue to appear. Even though we have fixes for all known bugs, there is a risk of throwing many bugs to end users. The plan for self-join elimination would be to do more review and testing, then re-commit in the early v18 cycle. Reported-by: Tom Lane Discussion: https://postgr.es/m/2422119.1714691974%40sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d1d286d83c0eed695910cb20d970ea9bea2e5001 Modified Files -- doc/src/sgml/config.sgml | 16 - src/backend/optimizer/path/indxpath.c | 39 - src/backend/optimizer/plan/analyzejoins.c | 1284 ++--- src/backend/optimizer/plan/planmain.c |5 - src/backend/utils/misc/guc_tables.c | 10 - src/include/nodes/pathnodes.h | 35 +- src/include/optimizer/paths.h |3 - src/include/optimizer/planmain.h |6 - src/test/regress/expected/equivclass.out | 30 - src/test/regress/expected/join.out| 1032 --- src/test/regress/expected/sysviews.out|3 +- src/test/regress/sql/equivclass.sql | 16 - src/test/regress/sql/join.sql | 470 --- src/tools/pgindent/typedefs.list |3 - 14 files changed, 69 insertions(+), 2883 deletions(-)
pgsql: Stabilize regression tests introduced by 259c96fa8f
Stabilize regression tests introduced by 259c96fa8f Add the ORDER BY clause to new queries to avoid ordering ambiguity. Per buildfarm member rorqual. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/449cdcd486bfc6864e4fa6784cc1526a94fe69db Modified Files -- src/test/regress/expected/partition_merge.out | 5 +++-- src/test/regress/expected/partition_split.out | 3 ++- src/test/regress/sql/partition_merge.sql | 3 ++- src/test/regress/sql/partition_split.sql | 3 ++- 4 files changed, 9 insertions(+), 5 deletions(-)
pgsql: Inherit parent's AM for partition MERGE/SPLIT operations
Inherit parent's AM for partition MERGE/SPLIT operations This commit makes new partitions created by ALTER TABLE ... SPLIT PARTITION and ALTER TABLE ... MERGE PARTITIONS commands inherit the paret table access method. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/84ada05b-be5c-473e-6d1c-ebe5dd21b190%40gmail.com Reviewed-by: Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/259c96fa8f78c0446eba1880850ea6fbe5092120 Modified Files -- doc/src/sgml/ref/alter_table.sgml | 2 ++ src/backend/commands/tablecmds.c | 6 ++ src/test/regress/expected/partition_merge.out | 18 ++ src/test/regress/expected/partition_split.out | 23 +-- src/test/regress/sql/partition_merge.sql | 13 + src/test/regress/sql/partition_split.sql | 14 ++ 6 files changed, 74 insertions(+), 2 deletions(-)
pgsql: Change the way ATExecMergePartitions() handles the name collisio
Change the way ATExecMergePartitions() handles the name collision The name collision happens when the name of the new partition is the same as the name of one of the merging partitions. Currently, ATExecMergePartitions() first gives the new partition a temporary name and then renames it when old partitions are deleted. That negatively influences the naming of related objects like indexes and constrains, which could inherit a temporary name. This commit changes the implementation in the following way. A merging partition gets renamed first, then the new partition is created with the right name immediately. This resolves the issue of the naming of related objects. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/edfbd846-dcc1-42d1-ac26-715691b687d3%40postgrespro.ru Author: Dmitry Koval, Alexander Korotkov Reviewed-by: Robert Haas, Justin Pryzby, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/885742b9f88b9386368ee94df8c94d154677ffba Modified Files -- src/backend/commands/tablecmds.c | 63 +-- src/test/regress/expected/partition_merge.out | 25 +++ src/test/regress/sql/partition_merge.sql | 18 3 files changed, 73 insertions(+), 33 deletions(-)
pgsql: Add tab completion for partition MERGE/SPLIT operations
Add tab completion for partition MERGE/SPLIT operations This commit implements psql tab completion for ALTER TABLE ... SPLIT PARTITION and ALTER TABLE ... MERGE PARTITIONS commands. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/5dee3937-8e9f-cca4-11fb-737709a92b37%40gmail.com Author: Dagfinn Ilmari Mannsåker, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/60ae37a8bc02f7a4beef442ee7b4656fba0e4e71 Modified Files -- src/bin/psql/tab-complete.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-)
pgsql: Document the way partition MERGE/SPLIT operations create new par
Document the way partition MERGE/SPLIT operations create new partitions Reported-by: Justin Pryzby Discussion: https://postgr.es/m/ZilrByTp-pbz6Mvf%40pryzbyj2023 Reviewed-by: Justin Pryzby Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/842c9b27057e8ecea02b816e3ec6c208779b3d39 Modified Files -- doc/src/sgml/ref/alter_table.sgml | 12 1 file changed, 12 insertions(+)
pgsql: Rename tables in tests of partition MERGE/SPLIT operations
Rename tables in tests of partition MERGE/SPLIT operations Replace "salesman" with "salesperson", "salesmen" with "salespeople". The names are both gramatically correct and gender-neutral. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/fdaa003e-919c-cbc9-4f0c-e4546e96bd65%40gmail.com Reviewed-by: Robert Haas, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/f4fc7cb54b6a9041f7064de71cb3ee3c4f48e061 Modified Files -- src/test/regress/expected/partition_merge.out | 666 ++--- src/test/regress/expected/partition_split.out | 1266 - src/test/regress/sql/partition_merge.sql | 160 ++-- src/test/regress/sql/partition_split.sql | 280 +++--- 4 files changed, 1186 insertions(+), 1186 deletions(-)
pgsql: Fix error message in check_partition_bounds_for_split_range()
Fix error message in check_partition_bounds_for_split_range() Currently, the error message is produced by a system of complex substitutions making it quite untranslatable and hard to read. This commit splits this into 4 plain error messages suitable for translation. Reported-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/20240408.152402.1485994009160660141.horikyota.ntt%40gmail.com Reviewed-by: Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/96c7381c4ceee2ef7c0a7327beb71dec47cf855e Modified Files -- src/backend/partitioning/partbounds.c | 70 --- 1 file changed, 49 insertions(+), 21 deletions(-)
pgsql: Make new partitions with parent's persistence during MERGE/SPLIT
Make new partitions with parent's persistence during MERGE/SPLIT The createPartitionTable() function is responsible for creating new partitions for ALTER TABLE ... MERGE PARTITIONS, and ALTER TABLE ... SPLIT PARTITION commands. It emulates the behaviour of CREATE TABLE ... (LIKE ...), where new table persistence should be specified by the user. In the table partitioning persistent of the partition and its parent must match. So, this commit makes createPartitionTable() copy the persistence of the parent partition. Also, this commit makes createPartitionTable() recheck the persistence after the new table creation. This is needed because persistence might be affected by pg_temp in search_path. This commit also changes the signature of createPartitionTable() making it take the parent's Relation itself instead of the name of the parent relation, and return the Relation of new partition. That doesn't lead to complications, because both callers have the parent table open and need to open the new partition. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com Author: Dmitry Koval Reviewed-by: Alexander Korotkov, Robert Haas, Justin Pryzby, Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/fcf80c5d5f0f3787e70fca8fd029d2e08a923f91 Modified Files -- doc/src/sgml/ref/alter_table.sgml | 6 ++ src/backend/commands/tablecmds.c | 75 +- src/test/regress/expected/partition_merge.out | 134 ++ src/test/regress/expected/partition_split.out | 75 +- src/test/regress/sql/partition_merge.sql | 88 +++-- src/test/regress/sql/partition_split.sql | 43 - 6 files changed, 364 insertions(+), 57 deletions(-)
Re: pgsql: Fix failure to track role dependencies of pg_init_privs entries.
On Tue, Apr 30, 2024 at 2:26 AM Tom Lane wrote: > Fix failure to track role dependencies of pg_init_privs entries. I just noticed that test_pg_dump checks don't pass for me. And for most of the buildfarm members too. You must be already aware of this, just in case. -- Regards, Alexander Korotkov
pgsql: Refactoring for CommitTransactionCommand()/AbortCurrentTransacti
Refactoring for CommitTransactionCommand()/AbortCurrentTransaction() fefd9a3fed turned tail recursion of CommitTransactionCommand() and AbortCurrentTransaction() into iteration. However, it splits the handling of cases between different functions. This commit puts the handling of all the cases into AbortCurrentTransactionInternal() and CommitTransactionCommandInternal(). Now CommitTransactionCommand() and AbortCurrentTransaction() are just doing the repeated calls of internal functions. Reported-by: Andres Freund Discussion: https://postgr.es/m/20240415224834.w6piwtefskoh32mv%40awork3.anarazel.de Author: Andres Freund Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/40126ac68f2ff96351cd6071350eb2d5cbd50145 Modified Files -- src/backend/access/transam/xact.c | 155 +- 1 file changed, 71 insertions(+), 84 deletions(-)
pgsql: revert: Generalize relation analyze in table AM interface
revert: Generalize relation analyze in table AM interface This commit reverts 27bc1772fc and dd1f6b0c17. Per review by Andres Freund. Discussion: https://postgr.es/m/20240415201057.khoyxbwwxfgzomeo%40awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6377e12a5a5278446bb0d215439b4825ef8996d1 Modified Files -- src/backend/access/heap/heapam_handler.c | 29 ++--- src/backend/access/table/tableamapi.c| 2 + src/backend/commands/analyze.c | 55 + src/include/access/heapam.h | 8 --- src/include/access/tableam.h | 101 --- src/include/commands/vacuum.h| 69 - src/include/foreign/fdwapi.h | 8 ++- src/tools/pgindent/typedefs.list | 3 - 8 files changed, 107 insertions(+), 168 deletions(-)
pgsql: Grammar fixes for split/merge partitions code
Grammar fixes for split/merge partitions code The fixes relate to comments, error messages, and corresponding expected output of regression tests. Discussion: https://postgr.es/m/CAMbWs49DDsknxyoycBqiE72VxzL_sYHF6zqL8dSeNehKPJhkKg%40mail.gmail.com Discussion: https://postgr.es/m/86bfd241-a58c-479a-9a72-2c67a02becf8%40postgrespro.ru Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com Author: Richard Guo, Dmitry Koval, Tender Wang Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9dfcac8e15acc3b4d4d5bc02383a56ccb07229fe Modified Files -- doc/src/sgml/ddl.sgml | 2 +- doc/src/sgml/ref/alter_table.sgml | 6 +-- src/backend/commands/tablecmds.c | 21 +- src/backend/parser/parse_utilcmd.c| 6 +-- src/backend/partitioning/partbounds.c | 35 src/test/isolation/specs/partition-merge.spec | 2 +- src/test/regress/expected/partition_merge.out | 14 +++ src/test/regress/expected/partition_split.out | 58 +-- src/test/regress/sql/partition_merge.sql | 12 +++--- src/test/regress/sql/partition_split.sql | 48 +++--- 10 files changed, 104 insertions(+), 100 deletions(-)
pgsql: Revert: Implement pg_wal_replay_wait() stored procedure
Revert: Implement pg_wal_replay_wait() stored procedure This commit reverts 06c418e163, e37662f221, bf1e650806, 25f42429e2, ee79928441, and 74eaf66f98 per review by Heikki Linnakangas. Discussion: https://postgr.es/m/b155606b-e744-4218-bda5-29379779da1a%40iki.fi Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/772faafca1b288c4dd66b7150a7831c27b768003 Modified Files -- doc/src/sgml/func.sgml | 113 src/backend/access/transam/xact.c | 6 - src/backend/access/transam/xlog.c | 7 - src/backend/access/transam/xlogrecovery.c | 11 - src/backend/catalog/system_functions.sql| 3 - src/backend/commands/Makefile | 3 +- src/backend/commands/meson.build| 1 - src/backend/commands/waitlsn.c | 337 src/backend/lib/pairingheap.c | 18 +- src/backend/storage/ipc/ipci.c | 3 - src/backend/storage/lmgr/proc.c | 6 - src/backend/utils/activity/wait_event_names.txt | 2 - src/include/catalog/catversion.h| 2 +- src/include/catalog/pg_proc.dat | 5 - src/include/commands/waitlsn.h | 77 -- src/include/lib/pairingheap.h | 3 - src/include/storage/lwlocklist.h| 1 - src/test/recovery/meson.build | 1 - src/test/recovery/t/043_wal_replay_wait.pl | 97 --- src/tools/pgindent/typedefs.list| 2 - 20 files changed, 4 insertions(+), 694 deletions(-)
pgsql: Revert: Let table AM insertion methods control index insertion
Revert: Let table AM insertion methods control index insertion This commit reverts b1484a3f19 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/da841aa4dc279bb0053de56121c927ec943edff3 Modified Files -- src/backend/access/heap/heapam.c | 4 +--- src/backend/access/heap/heapam_handler.c | 4 +--- src/backend/access/table/tableam.c | 6 ++ src/backend/catalog/indexing.c | 4 +--- src/backend/commands/copyfrom.c | 13 - src/backend/commands/createas.c | 4 +--- src/backend/commands/matview.c | 4 +--- src/backend/commands/tablecmds.c | 22 +- src/backend/executor/execReplication.c | 6 ++ src/backend/executor/nodeModifyTable.c | 6 ++ src/include/access/heapam.h | 2 +- src/include/access/tableam.h | 25 +++-- 12 files changed, 28 insertions(+), 72 deletions(-)
pgsql: Revert: Allow table AM to store complex data structures in rd_am
Revert: Allow table AM to store complex data structures in rd_amcache This commit reverts 02eb07ea89 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/922c4c461d213a422ee7eb6c38e399607539210a Modified Files -- src/backend/access/heap/heapam_handler.c | 1 - src/backend/utils/cache/relcache.c | 11 --- src/include/access/tableam.h | 34 src/include/utils/rel.h | 10 -- 4 files changed, 12 insertions(+), 44 deletions(-)
pgsql: Revert: Allow table AM tuple_insert() method to return the diffe
Revert: Allow table AM tuple_insert() method to return the different slot This commit reverts c35a3fb5e0 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/8dd0bb84da7d56a9e41241b26bfbf6b79644d574 Modified Files -- src/backend/access/heap/heapam_handler.c | 4 +--- src/backend/commands/tablecmds.c | 8 src/backend/executor/nodeModifyTable.c | 6 +++--- src/include/access/tableam.h | 21 + 4 files changed, 17 insertions(+), 22 deletions(-)
pgsql: Revert: Allow locking updated tuples in tuple_update() and tuple
Revert: Allow locking updated tuples in tuple_update() and tuple_delete() This commit reverts 87985cc925 and 818861eb57 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/193e6d18e553a104d315ff81892d509d89a30fd8 Modified Files -- src/backend/access/heap/heapam.c | 205 +- src/backend/access/heap/heapam_handler.c | 93 ++-- src/backend/access/table/tableam.c | 26 +-- src/backend/commands/trigger.c | 55 +++-- src/backend/executor/execReplication.c | 19 +- src/backend/executor/nodeModifyTable.c | 353 +-- src/include/access/heapam.h | 19 +- src/include/access/tableam.h | 73 ++- src/include/commands/trigger.h | 4 +- 9 files changed, 346 insertions(+), 501 deletions(-)
pgsql: Revert: Custom reloptions for table AM
Revert: Custom reloptions for table AM This commit reverts 9bd99f4c26 and 422041542f per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/bc1e2092ebb857802a9713d0d3588079e2f0216a Modified Files -- src/backend/access/common/reloptions.c | 149 + src/backend/access/heap/heapam.c | 4 +- src/backend/access/heap/heapam_handler.c | 13 -- src/backend/access/heap/heaptoast.c| 9 +- src/backend/access/heap/hio.c | 4 +- src/backend/access/heap/pruneheap.c| 4 +- src/backend/access/heap/rewriteheap.c | 4 +- src/backend/access/table/tableam.c | 2 +- src/backend/access/table/tableamapi.c | 25 src/backend/commands/createas.c| 13 +- src/backend/commands/tablecmds.c | 63 - src/backend/commands/vacuum.c | 8 +- src/backend/postmaster/autovacuum.c| 13 +- src/backend/tcop/utility.c | 28 +--- src/backend/utils/cache/relcache.c | 54 +--- src/include/access/reloptions.h| 11 +- src/include/access/tableam.h | 50 --- src/include/utils/rel.h| 148 ++-- src/test/modules/Makefile | 1 - src/test/modules/meson.build | 1 - src/test/modules/test_tam_options/.gitignore | 4 - src/test/modules/test_tam_options/Makefile | 23 .../test_tam_options/expected/test_tam_options.out | 36 - src/test/modules/test_tam_options/meson.build | 33 - .../test_tam_options/sql/test_tam_options.sql | 25 .../test_tam_options/test_tam_options--1.0.sql | 12 -- .../modules/test_tam_options/test_tam_options.c| 66 - .../test_tam_options/test_tam_options.control | 4 - src/tools/pgindent/typedefs.list | 4 +- 29 files changed, 161 insertions(+), 650 deletions(-)
pgsql: revert: Transform OR clauses to ANY expression
revert: Transform OR clauses to ANY expression This commit reverts 72bd38cc99 due to implementation and design issues. Reported-by: Tom Lane Discussion: https://postgr.es/m/3604469.1712628736%40sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/ff9f72c68f678ded340b431c3e280fe56644a3e7 Modified Files -- doc/src/sgml/config.sgml | 57 src/backend/nodes/queryjumblefuncs.c | 27 -- src/backend/optimizer/prep/prepqual.c | 400 +- src/backend/utils/misc/guc_tables.c | 12 - src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/nodes/queryjumble.h | 1 - src/include/optimizer/optimizer.h | 2 - src/test/regress/expected/create_index.out| 159 -- src/test/regress/expected/join.out| 50 src/test/regress/expected/partition_prune.out | 37 ++- src/test/regress/sql/create_index.sql | 45 --- src/test/regress/sql/join.sql | 11 - src/test/regress/sql/partition_prune.sql | 2 - src/tools/pgindent/typedefs.list | 2 - 14 files changed, 20 insertions(+), 786 deletions(-)
pgsql: Checks for ALTER TABLE ... SPLIT/MERGE PARTITIONS ... commands
Checks for ALTER TABLE ... SPLIT/MERGE PARTITIONS ... commands Check that the target partition actually belongs to the parent table. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/cd842601-cf1a-9806-f7b7-d2509b93ba61%40gmail.com Author: Dmitry Koval Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c99ef1811a064a94fb78917f35bb5853059a92b7 Modified Files -- src/backend/commands/tablecmds.c | 12 - src/backend/parser/parse_utilcmd.c| 39 +++ src/test/regress/expected/partition_merge.out | 29 +++- src/test/regress/expected/partition_split.out | 13 + src/test/regress/sql/partition_merge.sql | 24 ++--- src/test/regress/sql/partition_split.sql | 15 +++ 6 files changed, 111 insertions(+), 21 deletions(-)
pgsql: Provide a way block-level table AMs could re-use acquire_sample_
Provide a way block-level table AMs could re-use acquire_sample_rows() While keeping API the same, this commit provides a way for block-level table AMs to re-use existing acquire_sample_rows() by providing custom callbacks for getting the next block and the next tuple. Reported-by: Andres Freund Discussion: https://postgr.es/m/20240407214001.jgpg5q3yv33ve6y3%40awork3.anarazel.de Reviewed-by: Pavel Borisov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/dd1f6b0c172a643a73d6b71259fa2d10378b39eb Modified Files -- src/backend/access/heap/heapam_handler.c | 12 +++ src/backend/commands/analyze.c | 42 +++- src/include/access/tableam.h | 13 src/include/commands/vacuum.h| 56 ++-- src/tools/pgindent/typedefs.list | 2 ++ 5 files changed, 106 insertions(+), 19 deletions(-)
pgsql: Fix some grammer errors from error messages and codes comments
Fix some grammer errors from error messages and codes comments Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com Author: Tender Wang Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/df64c81ca9cba7b0fcb0ded5f735e99e676671c1 Modified Files -- src/backend/commands/tablecmds.c | 18 +- src/backend/parser/parse_utilcmd.c| 2 +- src/backend/partitioning/partbounds.c | 10 +++--- src/include/nodes/parsenodes.h| 4 ++-- src/test/regress/expected/partition_split.out | 4 ++-- 5 files changed, 21 insertions(+), 17 deletions(-)
pgsql: Fill CommonRdOptions with default values in extract_autovac_opts
Fill CommonRdOptions with default values in extract_autovac_opts() Reported-by: Thomas Munro Reported-by: Pavel Borisov Discussion: https://postgr.es/m/CA%2BhUKGLZzLR50RBvuqOO3MZ%3DF54ETz-rTp1PDX9uDGP_GqyYqA%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/422041542f313f23ca66cad26e9b2b99c4d1999a Modified Files -- src/backend/access/common/reloptions.c | 28 src/backend/postmaster/autovacuum.c| 1 + src/backend/utils/cache/relcache.c | 21 + src/include/access/reloptions.h| 1 + 4 files changed, 31 insertions(+), 20 deletions(-)
Re: pgsql: Custom reloptions for table AM
Hi, Thomas! On Mon, Apr 8, 2024 at 12:03 PM Thomas Munro wrote: > I think this is uninitialised memory: I'm already working on this. Will be fixed in 10-15 minutes. -- Regards, Alexander Korotkov
pgsql: Custom reloptions for table AM
Custom reloptions for table AM Let table AM define custom reloptions for its tables. This allows specifying AM-specific parameters by the WITH clause when creating a table. The reloptions, which could be used outside of table AM, are now extracted into the CommonRdOptions data structure. These options could be by decision of table AM directly specified by a user or calculated in some way. The new test module test_tam_options evaluates the ability to set up custom reloptions and calculate fields of CommonRdOptions on their base. The code may use some parts from prior work by Hao Wu. Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com Discussion: https://postgr.es/m/AMUA1wBBBxfc3tKRLLdU64rb.1.1683276279979.Hmail.wuhao%40hashdata.cn Reviewed-by: Reviewed-by: Pavel Borisov, Matthias van de Meent, Jess Davis Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9bd99f4c26fe37b8ee2f199aa868a0e2fdba4c43 Modified Files -- src/backend/access/common/reloptions.c | 121 - src/backend/access/heap/heapam.c | 4 +- src/backend/access/heap/heapam_handler.c | 13 ++ src/backend/access/heap/heaptoast.c| 9 +- src/backend/access/heap/hio.c | 4 +- src/backend/access/heap/pruneheap.c| 4 +- src/backend/access/heap/rewriteheap.c | 4 +- src/backend/access/table/tableam.c | 2 +- src/backend/access/table/tableamapi.c | 25 src/backend/commands/createas.c| 13 +- src/backend/commands/tablecmds.c | 63 + src/backend/commands/vacuum.c | 8 +- src/backend/postmaster/autovacuum.c| 12 +- src/backend/tcop/utility.c | 28 +++- src/backend/utils/cache/relcache.c | 73 +- src/include/access/reloptions.h| 10 +- src/include/access/tableam.h | 50 +++ src/include/utils/rel.h| 148 +++-- src/test/modules/Makefile | 1 + src/test/modules/meson.build | 1 + src/test/modules/test_tam_options/.gitignore | 4 + src/test/modules/test_tam_options/Makefile | 23 .../test_tam_options/expected/test_tam_options.out | 36 + src/test/modules/test_tam_options/meson.build | 33 + .../test_tam_options/sql/test_tam_options.sql | 25 .../test_tam_options/test_tam_options--1.0.sql | 12 ++ .../modules/test_tam_options/test_tam_options.c| 66 + .../test_tam_options/test_tam_options.control | 4 + src/tools/pgindent/typedefs.list | 4 +- 29 files changed, 639 insertions(+), 161 deletions(-)
Re: pgsql: Transform OR clauses to ANY expression
On Mon, Apr 8, 2024 at 9:24 AM Kyotaro Horiguchi
wrote:
> At Mon, 08 Apr 2024 14:46:57 +0900 (JST), Kyotaro Horiguchi
> wrote in
> > At Sun, 07 Apr 2024 22:28:06 +0000, Alexander Korotkov
> > wrote in
> > > Transform OR clauses to ANY expression
> >
> > This commit introduces a message like this:
> >
> > > gettext_noop("Set the minimum length of the list of OR clauses to attempt
> > > the OR-to-ANY transformation."),
> >
> > Unlike the usual phrasing of similar messages in this context, which
> > use the form "Sets the minimum length of...", this message uses an
> > imperative form ("Set"). I believe it would be better to align the
> > tone of this message with the others by changing "Set" to "Sets".
> >
> > regards.
> >
> >
> > diff --git a/src/backend/utils/misc/guc_tables.c
> > b/src/backend/utils/misc/guc_tables.c
> > index 83e3a59d7e..a527ffe0b0 100644
> > --- a/src/backend/utils/misc/guc_tables.c
> > +++ b/src/backend/utils/misc/guc_tables.c
> > @@ -3670,7 +3670,7 @@ struct config_int ConfigureNamesInt[] =
> >
> > {
> > {"or_to_any_transform_limit", PGC_USERSET, QUERY_TUNING_OTHER,
> > - gettext_noop("Set the minimum length of the list of
> > OR clauses to attempt the OR-to-ANY transformation."),
> > + gettext_noop("Sets the minimum length of the list of
> > OR clauses to attempt the OR-to-ANY transformation."),
> > gettext_noop("Once the limit is reached, the planner
> > will try to replace expression like "
> >"'x=c1 OR x=c2 ..' to the
> > expression 'x = ANY(ARRAY[c1,c2,..])'"),
> > GUC_EXPLAIN
>
> Sorry for the sequential posts, but I found the following contruct in
> the same patch to be quite untranslatable.
No worries. But these are distinct patches.
> > errmsg("%s bound of partition \"%s\" is %s %s bound of split partition",
> > first ? "lower" : "upper",
> > relname,
> > defaultPart ? (first ? "less than" : "greater than") : "not
> > equals to",
> > first ? "lower" : "upper"),
> > parser_errposition(pstate, datum->location)));
>
> I might be misunderstanding this, but if the expressions are correct,
> the message could be divided into four fixed messages based on "first"
> and "defaultPart". Alternatively, it might be better to provide
> simpler explation of the situation.
Yes, splitting this into multiple messages helps both translating and
readability. Will be fixed in the next couple of days.
--
Regards,
Alexander Korotkov
pgsql: Fix the wording of or_to_any_transform_limit description
Fix the wording of or_to_any_transform_limit description Reported-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/20240408.144657.1746688590065601661.horikyota.ntt%40gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/b453a7a16a7ed2ba96522e521143bc652b74875f Modified Files -- src/backend/utils/misc/guc_tables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix the value of or_to_any_transform_limit in postgresql.conf.sa
Fix the value of or_to_any_transform_limit in postgresql.conf.sample Reported-by: Justin Pryzby Discussion: https://postgr.es/m/ZhM8jH8gsKm5Q-9p%40pryzbyj2023 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/2af75e1174786a02fc35755c0cb98c522d72a065 Modified Files -- src/backend/utils/misc/postgresql.conf.sample | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix usage of same ListCell transform_or_to_any()'s in nested loo
Fix usage of same ListCell transform_or_to_any()'s in nested loops Discussion: https://postgr.es/m/CAAKRu_b4SXNW4GAM0bv3e6wcL5ODSXg1ZdRCn6uyLLjSPbveBg%40mail.gmail.com Author: Melanie Plageman Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/beabea6c2063e583628c59d03102dba996975b4a Modified Files -- src/backend/optimizer/prep/prepqual.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Re: pgsql: Transform OR clauses to ANY expression
On Mon, Apr 8, 2024 at 1:35 AM Melanie Plageman wrote: > On Sun, Apr 7, 2024 at 6:28 PM Alexander Korotkov > wrote: > > > > Transform OR clauses to ANY expression > > > > Replace (expr op C1) OR (expr op C2) ... with expr op ANY(ARRAY[C1, C2, > > ...]) > > on the preliminary stage of optimization when we are still working with the > > expression tree. > > > > Here Cn is a n-th constant expression, 'expr' is non-constant expression, > > 'op' > > is an operator which returns boolean result and has a commuter (for the case > > of reverse order of constant and non-constant parts of the expression, > > like 'Cn op expr'). > > > > Sometimes it can lead to not optimal plan. This is why there is a > > or_to_any_transform_limit GUC. It specifies a threshold value of length of > > arguments in an OR expression that triggers the OR-to-ANY transformation. > > Generally, more groupable OR arguments mean that transformation will be more > > likely to win than to lose. > > I'm getting this warning now > > /src/backend/optimizer/prep/prepqual.c:582:33: warning: declaration of > ‘lc__state’ shadows a previous local [-Wshadow=compatible-local] > 582 | foreach(lc, entry->consts) Thank you for catching. I'm fixing this now. -- Regards, Alexander Korotkov
pgsql: Transform OR clauses to ANY expression
Transform OR clauses to ANY expression Replace (expr op C1) OR (expr op C2) ... with expr op ANY(ARRAY[C1, C2, ...]) on the preliminary stage of optimization when we are still working with the expression tree. Here Cn is a n-th constant expression, 'expr' is non-constant expression, 'op' is an operator which returns boolean result and has a commuter (for the case of reverse order of constant and non-constant parts of the expression, like 'Cn op expr'). Sometimes it can lead to not optimal plan. This is why there is a or_to_any_transform_limit GUC. It specifies a threshold value of length of arguments in an OR expression that triggers the OR-to-ANY transformation. Generally, more groupable OR arguments mean that transformation will be more likely to win than to lose. Discussion: https://postgr.es/m/567ED6CA.2040504%40sigaev.ru Author: Alena Rybakina Author: Andrey Lepikhov Reviewed-by: Peter Geoghegan Reviewed-by: Ranier Vilela Reviewed-by: Alexander Korotkov Reviewed-by: Robert Haas Reviewed-by: Jian He Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/72bd38cc99a15da6f97373fae98027c908c398ea Modified Files -- doc/src/sgml/config.sgml | 57 src/backend/nodes/queryjumblefuncs.c | 27 ++ src/backend/optimizer/prep/prepqual.c | 399 +- src/backend/utils/misc/guc_tables.c | 12 + src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/nodes/queryjumble.h | 1 + src/include/optimizer/optimizer.h | 2 + src/test/regress/expected/create_index.out| 159 ++ src/test/regress/expected/join.out| 50 src/test/regress/expected/partition_prune.out | 37 +-- src/test/regress/sql/create_index.sql | 45 +++ src/test/regress/sql/join.sql | 11 + src/test/regress/sql/partition_prune.sql | 2 + src/tools/pgindent/typedefs.list | 2 + 14 files changed, 785 insertions(+), 20 deletions(-)
pgsql: Implement ALTER TABLE ... MERGE PARTITIONS ... command
Implement ALTER TABLE ... MERGE PARTITIONS ... command This new DDL command merges several partitions into the one partition of the target table. The target partition is created using new createPartitionTable() function with parent partition as the template. This commit comprises quite naive implementation which works in single process and holds the ACCESS EXCLUSIVE LOCK on the parent table during all the operations including the tuple routing. This is why this new DDL command can't be recommended for large partitioned tables under a high load. However, this implementation come in handy in certain cases even as is. Also, it could be used as a foundation for future implementations with lesser locking and possibly parallel. Discussion: https://postgr.es/m/c73a1746-0cd0-6bdd-6b23-3ae0b7c0c582%40postgrespro.ru Author: Dmitry Koval Reviewed-by: Matthias van de Meent, Laurenz Albe, Zhihong Yu, Justin Pryzby Reviewed-by: Alvaro Herrera, Robert Haas, Stephane Tachoires Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/1adf16b8fba45f77056d91573cd7138ed9da4ebf Modified Files -- doc/src/sgml/ddl.sgml | 19 + doc/src/sgml/ref/alter_table.sgml | 77 ++- src/backend/commands/tablecmds.c | 354 +- src/backend/parser/gram.y | 22 +- src/backend/parser/parse_utilcmd.c | 89 +++ src/backend/partitioning/partbounds.c | 207 ++ src/include/nodes/parsenodes.h | 14 + src/include/parser/kwlist.h| 1 + src/include/partitioning/partbounds.h | 6 + src/test/isolation/expected/partition-merge.out| 199 ++ src/test/isolation/isolation_schedule | 1 + src/test/isolation/specs/partition-merge.spec | 54 ++ .../modules/test_ddl_deparse/test_ddl_deparse.c| 3 + src/test/regress/expected/partition_merge.out | 732 + src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/partition_merge.sql | 430 src/tools/pgindent/typedefs.list | 1 + 17 files changed, 2189 insertions(+), 22 deletions(-)
pgsql: Implement ALTER TABLE ... SPLIT PARTITION ... command
Implement ALTER TABLE ... SPLIT PARTITION ... command This new DDL command splits a single partition into several parititions. Just like ALTER TABLE ... MERGE PARTITIONS ... command, new patitions are created using createPartitionTable() function with parent partition as the template. This commit comprises quite naive implementation which works in single process and holds the ACCESS EXCLUSIVE LOCK on the parent table during all the operations including the tuple routing. This is why this new DDL command can't be recommended for large partitioned tables under a high load. However, this implementation come in handy in certain cases even as is. Also, it could be used as a foundation for future implementations with lesser locking and possibly parallel. Discussion: https://postgr.es/m/c73a1746-0cd0-6bdd-6b23-3ae0b7c0c582%40postgrespro.ru Author: Dmitry Koval Reviewed-by: Matthias van de Meent, Laurenz Albe, Zhihong Yu, Justin Pryzby Reviewed-by: Alvaro Herrera, Robert Haas, Stephane Tachoires Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/87c21bb9412c8ba2727dec5ebcd74d44c2232d11 Modified Files -- doc/src/sgml/ddl.sgml | 19 + doc/src/sgml/ref/alter_table.sgml | 65 +- src/backend/commands/tablecmds.c | 411 ++ src/backend/parser/gram.y | 38 +- src/backend/parser/parse_utilcmd.c | 62 +- src/backend/partitioning/partbounds.c | 657 + src/backend/utils/adt/ruleutils.c | 18 + src/include/nodes/parsenodes.h |1 + src/include/parser/kwlist.h|1 + src/include/partitioning/partbounds.h |5 + src/include/utils/ruleutils.h |2 + src/test/isolation/expected/partition-split.out| 190 +++ src/test/isolation/isolation_schedule |1 + src/test/isolation/specs/partition-split.spec | 54 + .../modules/test_ddl_deparse/test_ddl_deparse.c|3 + src/test/regress/expected/partition_split.out | 1417 src/test/regress/parallel_schedule |2 +- src/test/regress/sql/partition_split.sql | 833 src/tools/pgindent/typedefs.list |1 + 19 files changed, 3766 insertions(+), 14 deletions(-)
pgsql: Use an LWLock instead of a spinlock in waitlsn.c
Use an LWLock instead of a spinlock in waitlsn.c This should prevent busy-waiting when number of waiting processes is high. Discussion: https://postgr.es/m/202404030658.hhj3vfxeyhft%40alvherre.pgsql Author: Alvaro Herrera Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/25f42429e2ff2acca35c9154fc2e36b75c79227a Modified Files -- src/backend/commands/waitlsn.c | 15 +++ src/backend/utils/activity/wait_event_names.txt | 1 + src/include/commands/waitlsn.h | 5 + src/include/storage/lwlocklist.h| 1 + 4 files changed, 10 insertions(+), 12 deletions(-)
pgsql: Call WaitLSNCleanup() in AbortTransaction()
Call WaitLSNCleanup() in AbortTransaction() Even though waiting for replay LSN happens without explicit transaction, AbortTransaction() is responsible for the cleanup of the shared memory if the error is thrown in a stored procedure. So, we need to do WaitLSNCleanup() there to clean up after some unexpected error happened while waiting for replay LSN. Discussion: https://postgr.es/m/202404051815.eri4u5q6oj26%40alvherre.pgsql Author: Alvaro Herrera Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/74eaf66f988c868deb0816bae4dd184eedae1448 Modified Files -- src/backend/access/transam/xact.c | 6 ++ 1 file changed, 6 insertions(+)
pgsql: Clarify what is protected by WaitLSNLock
Clarify what is protected by WaitLSNLock Not just WaitLSNState.waitersHeap, but also WaitLSNState.procInfos and updating of WaitLSNState.minWaitedLSN is protected by WaitLSNLock. There is one now documented exclusion on fast-path checking of WaitLSNProcInfo.inHeap flag. Discussion: https://postgr.es/m/202404030658.hhj3vfxeyhft%40alvherre.pgsql Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/ee79928441e7e291532b833455ebfee27d7cab5c Modified Files -- src/backend/commands/waitlsn.c | 10 -- src/include/commands/waitlsn.h | 7 +-- 2 files changed, 13 insertions(+), 4 deletions(-)
pgsql: Fix the parameters order for TableAmRoutine.relation_copy_for_cl
Fix the parameters order for TableAmRoutine.relation_copy_for_cluster() Specify OldTable first, NewTable second as used by table_relation_copy_for_cluster() and as implemented in heapam_relation_copy_for_cluster(). Backpatch to PostgreSQL 12, where TableAmRoutine was introduced. Discussion: https://postgr.es/m/ME3P282MB3166860D4911AE82F92DF7C5B63F2%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM Author: Japin Li Reviewed-by: Pavel Borisov Backpatch-through: 12 Branch -- REL_16_STABLE Details --- https://git.postgresql.org/pg/commitdiff/bafad43c37bf531f392e65ba726c253040b608f2 Modified Files -- src/include/access/tableam.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix the parameters order for TableAmRoutine.relation_copy_for_cl
Fix the parameters order for TableAmRoutine.relation_copy_for_cluster() Specify OldTable first, NewTable second as used by table_relation_copy_for_cluster() and as implemented in heapam_relation_copy_for_cluster(). Backpatch to PostgreSQL 12, where TableAmRoutine was introduced. Discussion: https://postgr.es/m/ME3P282MB3166860D4911AE82F92DF7C5B63F2%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM Author: Japin Li Reviewed-by: Pavel Borisov Backpatch-through: 12 Branch -- REL_15_STABLE Details --- https://git.postgresql.org/pg/commitdiff/c2faf48fa3b262f57cb999a3ab1e00e8d46176cd Modified Files -- src/include/access/tableam.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix the parameters order for TableAmRoutine.relation_copy_for_cl
Fix the parameters order for TableAmRoutine.relation_copy_for_cluster() Specify OldTable first, NewTable second as used by table_relation_copy_for_cluster() and as implemented in heapam_relation_copy_for_cluster(). Backpatch to PostgreSQL 12, where TableAmRoutine was introduced. Discussion: https://postgr.es/m/ME3P282MB3166860D4911AE82F92DF7C5B63F2%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM Author: Japin Li Reviewed-by: Pavel Borisov Backpatch-through: 12 Branch -- REL_14_STABLE Details --- https://git.postgresql.org/pg/commitdiff/19b8481b42efd29ac265e0d14566a7609b8d474b Modified Files -- src/include/access/tableam.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix the parameters order for TableAmRoutine.relation_copy_for_cl
Fix the parameters order for TableAmRoutine.relation_copy_for_cluster() Specify OldTable first, NewTable second as used by table_relation_copy_for_cluster() and as implemented in heapam_relation_copy_for_cluster(). Backpatch to PostgreSQL 12, where TableAmRoutine was introduced. Discussion: https://postgr.es/m/ME3P282MB3166860D4911AE82F92DF7C5B63F2%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM Author: Japin Li Reviewed-by: Pavel Borisov Backpatch-through: 12 Branch -- REL_13_STABLE Details --- https://git.postgresql.org/pg/commitdiff/68044d15b737fe567c9c2e00a1c8466530ed02ce Modified Files -- src/include/access/tableam.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
