public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v9 4/8] Propagate changes to indisclustered to child/parents 2+ messages / 2 participants [nested] [flat]
* [PATCH v9 4/8] Propagate changes to indisclustered to child/parents @ 2020-10-07 03:11 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-10-07 03:11 UTC (permalink / raw) --- src/backend/commands/cluster.c | 110 ++++++++++++++++---------- src/backend/commands/indexcmds.c | 2 + src/test/regress/expected/cluster.out | 46 +++++++++++ src/test/regress/sql/cluster.sql | 11 +++ 4 files changed, 126 insertions(+), 43 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 7d8d1d8a69..dd8014c206 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -74,6 +74,7 @@ static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose); static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); +static void set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index); static List *get_tables_to_cluster(MemoryContext cluster_context); static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid); @@ -513,66 +514,89 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD index_close(OldIndex, NoLock); } +/* + * Helper for mark_index_clustered + * Mark a single index as clustered or not. + * pg_index is passed by caller to avoid repeatedly re-opening it. + */ +static void +set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index) +{ + HeapTuple indexTuple; + Form_pg_index indexForm; + + indexTuple = SearchSysCacheCopy1(INDEXRELID, + ObjectIdGetDatum(indexOid)); + if (!HeapTupleIsValid(indexTuple)) + elog(ERROR, "cache lookup failed for index %u", indexOid); + indexForm = (Form_pg_index) GETSTRUCT(indexTuple); + + /* this was checked earlier, but let's be real sure */ + if (isclustered && !indexForm->indisvalid) + elog(ERROR, "cannot cluster on invalid index %u", indexOid); + + indexForm->indisclustered = isclustered; + CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple); + heap_freetuple(indexTuple); +} + /* * mark_index_clustered: mark the specified index as the one clustered on * - * With indexOid == InvalidOid, will mark all indexes of rel not-clustered. + * With indexOid == InvalidOid, mark all indexes of rel not-clustered. + * Otherwise, mark children of the clustered index as clustered, and parents of + * other indexes as unclustered. + * We wish to maintain the following properties: + * 1) Only one index on a relation can be marked clustered at once + * 2) If a partitioned index is clustered, then all its children must be + * clustered. */ void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) { - HeapTuple indexTuple; - Form_pg_index indexForm; - Relation pg_index; - ListCell *index; - - /* - * If the index is already marked clustered, no need to do anything. - */ - if (OidIsValid(indexOid)) - { - if (get_index_isclustered(indexOid)) - return; - } + ListCell *lc, *lc2; + List *indexes; + Relation pg_index = table_open(IndexRelationId, RowExclusiveLock); + List *inh = find_all_inheritors(RelationGetRelid(rel), + ShareRowExclusiveLock, NULL); /* * Check each index of the relation and set/clear the bit as needed. + * Iterate over the relation's children rather than the index's children + * since we need to unset cluster for indexes on intermediate children, + * too. */ - pg_index = table_open(IndexRelationId, RowExclusiveLock); - - foreach(index, RelationGetIndexList(rel)) + foreach(lc, inh) { - Oid thisIndexOid = lfirst_oid(index); - - indexTuple = SearchSysCacheCopy1(INDEXRELID, - ObjectIdGetDatum(thisIndexOid)); - if (!HeapTupleIsValid(indexTuple)) - elog(ERROR, "cache lookup failed for index %u", thisIndexOid); - indexForm = (Form_pg_index) GETSTRUCT(indexTuple); + Oid inhrelid = lfirst_oid(lc); + Relation thisrel = table_open(inhrelid, ShareRowExclusiveLock); - /* - * Unset the bit if set. We know it's wrong because we checked this - * earlier. - */ - if (indexForm->indisclustered) + indexes = RelationGetIndexList(thisrel); + foreach (lc2, indexes) { - indexForm->indisclustered = false; - CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple); - } - else if (thisIndexOid == indexOid) - { - /* this was checked earlier, but let's be real sure */ - if (!indexForm->indisvalid) - elog(ERROR, "cannot cluster on invalid index %u", indexOid); - indexForm->indisclustered = true; - CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple); - } + bool isclustered; + Oid thisIndexOid = lfirst_oid(lc2); + List *parentoids = get_rel_relispartition(thisIndexOid) ? + get_partition_ancestors(thisIndexOid) : NIL; - InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0, - InvalidOid, is_internal); + /* + * A child of the clustered index must be set clustered; + * indexes which are not children of the clustered index are + * set unclustered + */ + isclustered = (thisIndexOid == indexOid) || + list_member_oid(parentoids, indexOid); + Assert(OidIsValid(indexOid) || !isclustered); + set_indisclustered(thisIndexOid, isclustered, pg_index); + + InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0, + InvalidOid, is_internal); + } - heap_freetuple(indexTuple); + list_free(indexes); + table_close(thisrel, ShareRowExclusiveLock); } + list_free(inh); table_close(pg_index, RowExclusiveLock); } diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 8bc652ecd3..5f8fc2ea16 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -25,6 +25,7 @@ #include "catalog/catalog.h" #include "catalog/index.h" #include "catalog/indexing.h" +#include "catalog/partition.h" #include "catalog/pg_am.h" #include "catalog/pg_constraint.h" #include "catalog/pg_inherits.h" @@ -32,6 +33,7 @@ #include "catalog/pg_opfamily.h" #include "catalog/pg_tablespace.h" #include "catalog/pg_type.h" +#include "commands/cluster.h" #include "commands/comment.h" #include "commands/dbcommands.h" #include "commands/defrem.h" diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index c74cfa88cc..1d436dfaae 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -495,6 +495,52 @@ Indexes: "clstrpart_idx" btree (a) CLUSTER Number of partitions: 3 (Use \d+ to list them.) +-- Test that it recurses to grandchildren: +\d clstrpart33 + Table "public.clstrpart33" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart3 DEFAULT +Indexes: + "clstrpart33_a_idx" btree (a) CLUSTER + +ALTER TABLE clstrpart SET WITHOUT CLUSTER; +\d clstrpart33 + Table "public.clstrpart33" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart3 DEFAULT +Indexes: + "clstrpart33_a_idx" btree (a) + +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +\d clstrpart33 + Table "public.clstrpart33" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart3 DEFAULT +Indexes: + "clstrpart33_a_idx" btree (a) CLUSTER + +-- Check that only one child is marked clustered after marking clustered on a different parent +CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a); +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2; +\d clstrpart1 + Partitioned table "public.clstrpart1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: clstrpart FOR VALUES FROM (1) TO (10) +Partition key: RANGE (a) +Indexes: + "clstrpart1_a_idx" btree (a) + "clstrpart1_idx_2" btree (a) CLUSTER +Number of partitions: 2 (Use \d+ to list them.) + -- Test CLUSTER with external tuplesorting create table clstr_4 as select * from tenk1; create index cluster_sort on clstr_4 (hundred, thousand, tenthous); diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index 9bcc77695c..0ded2be1ca 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -220,6 +220,17 @@ CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitio CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf \d clstrpart +-- Test that it recurses to grandchildren: +\d clstrpart33 +ALTER TABLE clstrpart SET WITHOUT CLUSTER; +\d clstrpart33 +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +\d clstrpart33 +-- Check that only one child is marked clustered after marking clustered on a different parent +CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a); +ALTER TABLE clstrpart CLUSTER ON clstrpart_idx; +ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2; +\d clstrpart1 -- Test CLUSTER with external tuplesorting -- 2.17.0 --fmvA4kSBHQVZhkR6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0005-Invalidate-parent-indexes.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* PG buildfarm member cisticola @ 2024-07-24 10:57 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Alvaro Herrera @ 2024-07-24 10:57 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] Hello A couple of days ago, PG buildfarm member cisticola started failing: https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=cisticola&br=HEAD The failures[1] are pretty mysterious: make[3]: \347\246\273\345\274\200\347\233\256\345\275\225\342\200\234/home/postgres/buildfarm/HEAD/pgsql.build/src/backend/utils\342\200\235 ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 access/brin/brin.o [...] libpq/be-secure-openssl.o [...] ../../src/timezone/localtime.o ../../src/timezone/pgtz.o ../../src/timezone/strftime.o jit/jit.o ../../src/common/libpgcommon_srv.a ../../src/port/libpgport_srv.a -L../../src/port -L../../src/common -Wl,--as-needed -Wl,-rpath,'/home/postgres/buildfarm/HEAD/inst/lib',--enable-new-dtags -Wl,--export-dynamic -lxslt -lxml2 -lssl -lcrypto -lgssapi_krb5 -lz -lpthread -lrt -ldl -lm -lldap -licui18n -licuuc -licudata -o postgres /usr/bin/ld: libpq/be-secure-openssl.o: in function `.L388': be-secure-openssl.c:(.text+0x1f8c): undefined reference to `ERR_new' /usr/bin/ld: libpq/be-secure-openssl.o: in function `.L530': be-secure-openssl.c:(.text+0x1fa4): undefined reference to `ERR_set_debug' /usr/bin/ld: libpq/be-secure-openssl.o: in function `.LVL614': be-secure-openssl.c:(.text+0x1fb8): undefined reference to `ERR_set_error' /usr/bin/ld: libpq/be-secure-openssl.o: in function `.L539': be-secure-openssl.c:(.text+0x2010): undefined reference to `ERR_new' /usr/bin/ld: libpq/be-secure-openssl.o: in function `.L417': be-secure-openssl.c:(.text+0x20b0): undefined reference to `SSL_get1_peer_certificate' collect2: error: ld returned 1 exit status Previously, this was working fine. In this run, openssl is checking for openssl... /usr/bin/openssl configure: using openssl: OpenSSL 1.1.1g FIPS 21 Apr 2020 but that's the same that was used in the last successful run[2], too. Not sure what else could be relevant. [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=cisticola&dt=2024-07-24%2010%3A20%3A37 [2] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=cisticola&dt=2024-07-20%2022%3A20%... -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "No renuncies a nada. No te aferres a nada." ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-07-24 10:57 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-10-07 03:11 [PATCH v9 4/8] Propagate changes to indisclustered to child/parents Justin Pryzby <[email protected]> 2024-07-24 10:57 PG buildfarm member cisticola Alvaro Herrera <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox