From fa9933daf317713c78d0a8d877f6956d62ee3ef0 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 18 Aug 2021 00:44:37 +0300 Subject: [PATCH 1/5] Use get_am_name to find indexAM name --- src/backend/distributed/commands/alter_table.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/backend/distributed/commands/alter_table.c b/src/backend/distributed/commands/alter_table.c index b0f42cf38..0f3a2be3b 100644 --- a/src/backend/distributed/commands/alter_table.c +++ b/src/backend/distributed/commands/alter_table.c @@ -888,17 +888,12 @@ GetIndexAccessMethodName(Oid indexId) Oid indexAMId = indexForm->relam; ReleaseSysCache(indexTuple); - /* fetch pg_am tuple of index' access method */ - HeapTuple indexAMTuple = SearchSysCache1(AMOID, ObjectIdGetDatum(indexAMId)); - if (!HeapTupleIsValid(indexAMTuple)) + char *indexAmName = get_am_name(indexAMId); + if (!indexAmName) { ereport(ERROR, (errmsg("access method with oid %u does not exist", indexAMId))); } - Form_pg_am indexAMForm = (Form_pg_am) GETSTRUCT(indexAMTuple); - char *indexAmName = pstrdup(indexAMForm->amname.data); - ReleaseSysCache(indexAMTuple); - return indexAmName; } From 549ca4de6d9f4136f2a0104c11121b46b2fb1aae Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 18 Aug 2021 01:09:12 +0300 Subject: [PATCH 2/5] Use RelationGetIndexList instead of scanning pg_index --- src/backend/distributed/commands/index.c | 40 +++++++------------ .../distributed/operations/node_protocol.c | 6 +++ 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/backend/distributed/commands/index.c b/src/backend/distributed/commands/index.c index 6a0e39739..bd1b9eb88 100644 --- a/src/backend/distributed/commands/index.c +++ b/src/backend/distributed/commands/index.c @@ -306,37 +306,25 @@ ExecuteFunctionOnEachTableIndex(Oid relationId, PGIndexProcessor pgIndexProcesso int indexFlags) { List *result = NIL; - ScanKeyData scanKey[1]; - int scanKeyCount = 1; - PushOverrideEmptySearchPath(CurrentMemoryContext); - - /* open system catalog and scan all indexes that belong to this table */ - Relation pgIndex = table_open(IndexRelationId, AccessShareLock); - - ScanKeyInit(&scanKey[0], Anum_pg_index_indrelid, - BTEqualStrategyNumber, F_OIDEQ, relationId); - - SysScanDesc scanDescriptor = systable_beginscan(pgIndex, - IndexIndrelidIndexId, true, /* indexOK */ - NULL, scanKeyCount, scanKey); - - HeapTuple heapTuple = systable_getnext(scanDescriptor); - while (HeapTupleIsValid(heapTuple)) + Relation relation = RelationIdGetRelation(relationId); + List *indexIdList = RelationGetIndexList(relation); + Oid indexId = InvalidOid; + foreach_oid(indexId, indexIdList) { - Form_pg_index indexForm = (Form_pg_index) GETSTRUCT(heapTuple); - pgIndexProcessor(indexForm, &result, indexFlags); + HeapTuple indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexId)); + if (!HeapTupleIsValid(indexTuple)) + { + ereport(ERROR, (errmsg("cache lookup failed for index with oid %u", + indexId))); + } - heapTuple = systable_getnext(scanDescriptor); + Form_pg_index indexForm = (Form_pg_index) GETSTRUCT(indexTuple); + pgIndexProcessor(indexForm, &result, indexFlags); + ReleaseSysCache(indexTuple); } - /* clean up scan and close system catalog */ - systable_endscan(scanDescriptor); - table_close(pgIndex, AccessShareLock); - - /* revert back to original search_path */ - PopOverrideSearchPath(); - + RelationClose(relation); return result; } diff --git a/src/backend/distributed/operations/node_protocol.c b/src/backend/distributed/operations/node_protocol.c index f0ff1c82b..1638489e7 100644 --- a/src/backend/distributed/operations/node_protocol.c +++ b/src/backend/distributed/operations/node_protocol.c @@ -798,6 +798,9 @@ void GatherIndexAndConstraintDefinitionList(Form_pg_index indexForm, List **indexDDLEventList, int indexFlags) { + /* generate fully-qualified names */ + PushOverrideEmptySearchPath(CurrentMemoryContext); + Oid indexId = indexForm->indexrelid; bool indexImpliedByConstraint = IndexImpliedByAConstraint(indexForm); @@ -845,6 +848,9 @@ GatherIndexAndConstraintDefinitionList(Form_pg_index indexForm, List **indexDDLE *indexDDLEventList = list_concat(*indexDDLEventList, alterIndexStatisticsCommands); } + + /* revert back to original search_path */ + PopOverrideSearchPath(); } From 91544d019111ae37d09ea0f8a8d4793cf057b96c Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 18 Aug 2021 01:20:52 +0300 Subject: [PATCH 3/5] Use PGIndexProcessor infra to find explicitly created indexes --- .../citus_add_local_table_to_metadata.c | 59 ++++++------------- 1 file changed, 18 insertions(+), 41 deletions(-) diff --git a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c index 75b7661dd..ed02e55db 100644 --- a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c +++ b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c @@ -69,6 +69,9 @@ static char * GetRenameShardTriggerCommand(Oid shardRelationId, char *triggerNam static void DropRelationTruncateTriggers(Oid relationId); static char * GetDropTriggerCommand(Oid relationId, char *triggerName); static List * GetRenameStatsCommandList(List *statsOidList, uint64 shardId); +static void AppendExplicitIndexIdsToList(Form_pg_index indexForm, + List **explicitIndexIdList, + int flags); static void DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId, Oid targetRelationId); static void DropDefaultColumnDefinition(Oid relationId, char *columnName); @@ -834,51 +837,25 @@ GetDropTriggerCommand(Oid relationId, char *triggerName) List * GetExplicitIndexOidList(Oid relationId) { - int scanKeyCount = 1; - ScanKeyData scanKey[1]; + /* flags are not applicable for AppendExplicitIndexIdsToList */ + int flags = 0; + return ExecuteFunctionOnEachTableIndex(relationId, AppendExplicitIndexIdsToList, + flags); +} - PushOverrideEmptySearchPath(CurrentMemoryContext); - Relation pgIndex = table_open(IndexRelationId, AccessShareLock); - - ScanKeyInit(&scanKey[0], Anum_pg_index_indrelid, - BTEqualStrategyNumber, F_OIDEQ, relationId); - - bool useIndex = true; - SysScanDesc scanDescriptor = systable_beginscan(pgIndex, IndexIndrelidIndexId, - useIndex, NULL, scanKeyCount, - scanKey); - - List *indexOidList = NIL; - - HeapTuple heapTuple = systable_getnext(scanDescriptor); - while (HeapTupleIsValid(heapTuple)) +/* + * AppendExplicitIndexIdsToList adds the given index oid if it is + * explicitly created on its relation. + */ +static void +AppendExplicitIndexIdsToList(Form_pg_index indexForm, List **explicitIndexIdList, + int flags) +{ + if (!IndexImpliedByAConstraint(indexForm)) { - Form_pg_index indexForm = (Form_pg_index) GETSTRUCT(heapTuple); - - Oid indexId = indexForm->indexrelid; - - bool indexImpliedByConstraint = IndexImpliedByAConstraint(indexForm); - - /* - * Skip the indexes that are not implied by explicitly executing - * a CREATE INDEX command. - */ - if (!indexImpliedByConstraint) - { - indexOidList = lappend_oid(indexOidList, indexId); - } - - heapTuple = systable_getnext(scanDescriptor); + *explicitIndexIdList = lappend_oid(*explicitIndexIdList, indexForm->indexrelid); } - - systable_endscan(scanDescriptor); - table_close(pgIndex, NoLock); - - /* revert back to original search_path */ - PopOverrideSearchPath(); - - return indexOidList; } From 4b03195c06b05c8fe38b3bd47fad1ea91776077f Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 18 Aug 2021 01:28:33 +0300 Subject: [PATCH 4/5] Use RelationGetStatExtList instead of GetExplicitStatisticsIdList --- .../citus_add_local_table_to_metadata.c | 5 +- src/backend/distributed/commands/statistics.c | 50 ++----------------- src/include/distributed/commands.h | 1 - 3 files changed, 9 insertions(+), 47 deletions(-) diff --git a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c index ed02e55db..6dfe6cbc9 100644 --- a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c +++ b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c @@ -687,7 +687,10 @@ GetRenameShardIndexCommand(Oid indexOid, uint64 shardId) static void RenameShardRelationStatistics(Oid shardRelationId, uint64 shardId) { - List *statsOidList = GetExplicitStatisticsIdList(shardRelationId); + Relation shardRelation = RelationIdGetRelation(shardRelationId); + List *statsOidList = RelationGetStatExtList(shardRelation); + RelationClose(shardRelation); + List *statsCommandList = GetRenameStatsCommandList(statsOidList, shardId); char *command = NULL; diff --git a/src/backend/distributed/commands/statistics.c b/src/backend/distributed/commands/statistics.c index 3eed687d9..9b0323442 100644 --- a/src/backend/distributed/commands/statistics.c +++ b/src/backend/distributed/commands/statistics.c @@ -425,16 +425,18 @@ PreprocessAlterStatisticsOwnerStmt(Node *node, const char *queryString, * GetExplicitStatisticsCommandList returns the list of DDL commands to create * or alter statistics that are explicitly created for the table with relationId. * This function gets called when distributing the table with relationId. - * See comment of GetExplicitStatisticsIdList function. */ List * GetExplicitStatisticsCommandList(Oid relationId) { List *explicitStatisticsCommandList = NIL; - PushOverrideEmptySearchPath(CurrentMemoryContext); + Relation relation = RelationIdGetRelation(relationId); + List *statisticsIdList = RelationGetStatExtList(relation); + RelationClose(relation); - List *statisticsIdList = GetExplicitStatisticsIdList(relationId); + /* generate fully-qualified names */ + PushOverrideEmptySearchPath(CurrentMemoryContext); Oid statisticsId = InvalidOid; foreach_oid(statisticsId, statisticsIdList) @@ -567,48 +569,6 @@ GetAlterIndexStatisticsCommands(Oid indexOid) } -/* - * GetExplicitStatisticsIdList returns a list of OIDs corresponding to the statistics - * that are explicitly created on the relation with relationId. That means, - * this function discards internal statistics implicitly created by postgres. - */ -List * -GetExplicitStatisticsIdList(Oid relationId) -{ - List *statisticsIdList = NIL; - - Relation pgStatistics = table_open(StatisticExtRelationId, AccessShareLock); - - int scanKeyCount = 1; - ScanKeyData scanKey[1]; - - ScanKeyInit(&scanKey[0], Anum_pg_statistic_ext_stxrelid, - BTEqualStrategyNumber, F_OIDEQ, relationId); - - bool useIndex = true; - SysScanDesc scanDescriptor = systable_beginscan(pgStatistics, - StatisticExtRelidIndexId, - useIndex, NULL, scanKeyCount, - scanKey); - - HeapTuple heapTuple = systable_getnext(scanDescriptor); - while (HeapTupleIsValid(heapTuple)) - { - FormData_pg_statistic_ext *statisticsForm = - (FormData_pg_statistic_ext *) GETSTRUCT(heapTuple); - Oid statisticsId = statisticsForm->oid; - statisticsIdList = lappend_oid(statisticsIdList, statisticsId); - - heapTuple = systable_getnext(scanDescriptor); - } - - systable_endscan(scanDescriptor); - table_close(pgStatistics, NoLock); - - return statisticsIdList; -} - - /* * GenerateAlterIndexColumnSetStatsCommand returns a string in form of 'ALTER INDEX .. * ALTER COLUMN .. SET STATISTICS ..' which will be used to create a DDL command to diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 178014669..7accc064e 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -388,7 +388,6 @@ extern List * PreprocessAlterStatisticsOwnerStmt(Node *node, const char *querySt extern List * GetExplicitStatisticsCommandList(Oid relationId); extern List * GetExplicitStatisticsSchemaIdList(Oid relationId); extern List * GetAlterIndexStatisticsCommands(Oid indexOid); -extern List * GetExplicitStatisticsIdList(Oid relationId); /* subscription.c - forward declarations */ extern Node * ProcessCreateSubscriptionStmt(CreateSubscriptionStmt *createSubStmt); From 4e1201a33319c0dde87a63bac254c6bfd6a78cc2 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 18 Aug 2021 14:34:42 +0300 Subject: [PATCH 5/5] Use RelationGetStatExtList instead of scanning pg_stats_ext --- src/backend/distributed/commands/statistics.c | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/backend/distributed/commands/statistics.c b/src/backend/distributed/commands/statistics.c index 9b0323442..fb5406786 100644 --- a/src/backend/distributed/commands/statistics.c +++ b/src/backend/distributed/commands/statistics.c @@ -490,23 +490,19 @@ GetExplicitStatisticsSchemaIdList(Oid relationId) { List *schemaIdList = NIL; - Relation pgStatistics = table_open(StatisticExtRelationId, AccessShareLock); + Relation relation = RelationIdGetRelation(relationId); + List *statsIdList = RelationGetStatExtList(relation); + RelationClose(relation); - int scanKeyCount = 1; - ScanKeyData scanKey[1]; - - ScanKeyInit(&scanKey[0], Anum_pg_statistic_ext_stxrelid, - BTEqualStrategyNumber, F_OIDEQ, relationId); - - bool useIndex = true; - SysScanDesc scanDescriptor = systable_beginscan(pgStatistics, - StatisticExtRelidIndexId, - useIndex, NULL, scanKeyCount, - scanKey); - - HeapTuple heapTuple = systable_getnext(scanDescriptor); - while (HeapTupleIsValid(heapTuple)) + Oid statsId = InvalidOid; + foreach_oid(statsId, statsIdList) { + HeapTuple heapTuple = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(statsId)); + if (!HeapTupleIsValid(heapTuple)) + { + ereport(ERROR, (errmsg("cache lookup failed for statistics " + "object with oid %u", statsId))); + } FormData_pg_statistic_ext *statisticsForm = (FormData_pg_statistic_ext *) GETSTRUCT(heapTuple); @@ -515,12 +511,9 @@ GetExplicitStatisticsSchemaIdList(Oid relationId) { schemaIdList = lappend_oid(schemaIdList, schemaId); } - - heapTuple = systable_getnext(scanDescriptor); + ReleaseSysCache(heapTuple); } - systable_endscan(scanDescriptor); - table_close(pgStatistics, NoLock); return schemaIdList; }