From 343c2a865bc6c0a03358709df854ce1eac52ca45 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 20 Dec 2012 16:30:59 -0500 Subject: [PATCH] Fix pg_extension_config_dump() to handle update cases more sanely. If pg_extension_config_dump() is executed again for a table already listed in the extension's extconfig, the code was blindly making a new array entry. This does not seem useful. Fix it to replace the existing array entry instead, so that it's possible for extension update scripts to alter the filter conditions for configuration tables. In addition, teach ALTER EXTENSION DROP TABLE to check for an extconfig entry for the target table, and remove it if present. This is not a 100% solution because it's allowed for an extension update script to just summarily DROP a member table, and that code path doesn't go through ExecAlterExtensionContentsStmt. We could probably make that case clean things up if we had to, but it would involve sticking a very ugly wart somewhere in the guts of dependency.c. Since on the whole it seems quite unlikely that extension updates would want to remove pre-existing configuration tables, making the case possible with an explicit command seems sufficient. Per bug #7756 from Regina Obe. Back-patch to 9.1 where extensions were introduced. --- doc/src/sgml/extend.sgml | 12 ++ src/backend/commands/extension.c | 237 +++++++++++++++++++++++++++++-- 2 files changed, 239 insertions(+), 10 deletions(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 8d5b9d0c83..672d0df13c 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -665,6 +665,10 @@ SET LOCAL search_path TO @extschema@; and reload. + + pg_extension_config_dump + + To solve this problem, an extension's script file can mark a table it has created as a configuration table, which will cause @@ -703,6 +707,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr be modified by users, can be handled by creating triggers on the configuration table to ensure that modified rows are marked correctly. + + + You can alter the filter condition associated with a configuration table + by calling pg_extension_config_dump again. (This would + typically be useful in an extension update script.) The only way to mark + a table as no longer a configuration table is to dissociate it from the + extension with ALTER EXTENSION ... DROP TABLE. + diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 47631beb77..3b8e22425c 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -2050,6 +2050,7 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) HeapTuple extTup; Datum arrayDatum; Datum elementDatum; + int arrayLength; int arrayIndex; bool isnull; Datum repl_val[Natts_pg_extension]; @@ -2069,8 +2070,8 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) /* * Check that the table exists and is a member of the extension being - * created. This ensures that we don't need to register a dependency to - * protect the extconfig entry. + * created. This ensures that we don't need to register an additional + * dependency to protect the extconfig entry. */ tablename = get_rel_name(tableoid); if (tablename == NULL) @@ -2087,6 +2088,9 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) /* * Add the table OID and WHERE condition to the extension's extconfig and * extcondition arrays. + * + * If the table is already in extconfig, treat this as an update of the + * WHERE condition. */ /* Find the pg_extension tuple */ @@ -2117,18 +2121,41 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) RelationGetDescr(extRel), &isnull); if (isnull) { + /* Previously empty extconfig, so build 1-element array */ + arrayLength = 0; + arrayIndex = 1; + a = construct_array(&elementDatum, 1, OIDOID, sizeof(Oid), true, 'i'); } else { + /* Modify or extend existing extconfig array */ + Oid *arrayData; + int i; + a = DatumGetArrayTypeP(arrayDatum); - Assert(ARR_ELEMTYPE(a) == OIDOID); - Assert(ARR_NDIM(a) == 1); - Assert(ARR_LBOUND(a)[0] == 1); - arrayIndex = ARR_DIMS(a)[0] + 1; /* add after end */ + arrayLength = ARR_DIMS(a)[0]; + if (ARR_NDIM(a) != 1 || + ARR_LBOUND(a)[0] != 1 || + arrayLength < 0 || + ARR_HASNULL(a) || + ARR_ELEMTYPE(a) != OIDOID) + elog(ERROR, "extconfig is not a 1-D Oid array"); + arrayData = (Oid *) ARR_DATA_PTR(a); + + arrayIndex = arrayLength + 1; /* set up to add after end */ + + for (i = 0; i < arrayLength; i++) + { + if (arrayData[i] == tableoid) + { + arrayIndex = i + 1; /* replace this element instead */ + break; + } + } a = array_set(a, 1, &arrayIndex, elementDatum, @@ -2148,6 +2175,9 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) RelationGetDescr(extRel), &isnull); if (isnull) { + if (arrayLength != 0) + elog(ERROR, "extconfig and extcondition arrays do not match"); + a = construct_array(&elementDatum, 1, TEXTOID, -1, false, 'i'); @@ -2155,12 +2185,16 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) else { a = DatumGetArrayTypeP(arrayDatum); - Assert(ARR_ELEMTYPE(a) == TEXTOID); - Assert(ARR_NDIM(a) == 1); - Assert(ARR_LBOUND(a)[0] == 1); - arrayIndex = ARR_DIMS(a)[0] + 1; /* add after end */ + if (ARR_NDIM(a) != 1 || + ARR_LBOUND(a)[0] != 1 || + ARR_HASNULL(a) || + ARR_ELEMTYPE(a) != TEXTOID) + elog(ERROR, "extcondition is not a 1-D text array"); + if (ARR_DIMS(a)[0] != arrayLength) + elog(ERROR, "extconfig and extcondition arrays do not match"); + /* Add or replace at same index as in extconfig */ a = array_set(a, 1, &arrayIndex, elementDatum, false, @@ -2185,6 +2219,182 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* + * extension_config_remove + * + * Remove the specified table OID from extension's extconfig, if present. + * This is not currently exposed as a function, but it could be; + * for now, we just invoke it from ALTER EXTENSION DROP. + */ +static void +extension_config_remove(Oid extensionoid, Oid tableoid) +{ + Relation extRel; + ScanKeyData key[1]; + SysScanDesc extScan; + HeapTuple extTup; + Datum arrayDatum; + int arrayLength; + int arrayIndex; + bool isnull; + Datum repl_val[Natts_pg_extension]; + bool repl_null[Natts_pg_extension]; + bool repl_repl[Natts_pg_extension]; + ArrayType *a; + + /* Find the pg_extension tuple */ + extRel = heap_open(ExtensionRelationId, RowExclusiveLock); + + ScanKeyInit(&key[0], + ObjectIdAttributeNumber, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(extensionoid)); + + extScan = systable_beginscan(extRel, ExtensionOidIndexId, true, + SnapshotNow, 1, key); + + extTup = systable_getnext(extScan); + + if (!HeapTupleIsValid(extTup)) /* should not happen */ + elog(ERROR, "extension with oid %u does not exist", + extensionoid); + + /* Search extconfig for the tableoid */ + arrayDatum = heap_getattr(extTup, Anum_pg_extension_extconfig, + RelationGetDescr(extRel), &isnull); + if (isnull) + { + /* nothing to do */ + a = NULL; + arrayLength = 0; + arrayIndex = -1; + } + else + { + Oid *arrayData; + int i; + + a = DatumGetArrayTypeP(arrayDatum); + + arrayLength = ARR_DIMS(a)[0]; + if (ARR_NDIM(a) != 1 || + ARR_LBOUND(a)[0] != 1 || + arrayLength < 0 || + ARR_HASNULL(a) || + ARR_ELEMTYPE(a) != OIDOID) + elog(ERROR, "extconfig is not a 1-D Oid array"); + arrayData = (Oid *) ARR_DATA_PTR(a); + + arrayIndex = -1; /* flag for no deletion needed */ + + for (i = 0; i < arrayLength; i++) + { + if (arrayData[i] == tableoid) + { + arrayIndex = i; /* index to remove */ + break; + } + } + } + + /* If tableoid is not in extconfig, nothing to do */ + if (arrayIndex < 0) + { + systable_endscan(extScan); + heap_close(extRel, RowExclusiveLock); + return; + } + + /* Modify or delete the extconfig value */ + memset(repl_val, 0, sizeof(repl_val)); + memset(repl_null, false, sizeof(repl_null)); + memset(repl_repl, false, sizeof(repl_repl)); + + if (arrayLength <= 1) + { + /* removing only element, just set array to null */ + repl_null[Anum_pg_extension_extconfig - 1] = true; + } + else + { + /* squeeze out the target element */ + Datum *dvalues; + bool *dnulls; + int nelems; + int i; + + deconstruct_array(a, OIDOID, sizeof(Oid), true, 'i', + &dvalues, &dnulls, &nelems); + + /* We already checked there are no nulls, so ignore dnulls */ + for (i = arrayIndex; i < arrayLength - 1; i++) + dvalues[i] = dvalues[i + 1]; + + a = construct_array(dvalues, arrayLength - 1, + OIDOID, sizeof(Oid), true, 'i'); + + repl_val[Anum_pg_extension_extconfig - 1] = PointerGetDatum(a); + } + repl_repl[Anum_pg_extension_extconfig - 1] = true; + + /* Modify or delete the extcondition value */ + arrayDatum = heap_getattr(extTup, Anum_pg_extension_extcondition, + RelationGetDescr(extRel), &isnull); + if (isnull) + { + elog(ERROR, "extconfig and extcondition arrays do not match"); + } + else + { + a = DatumGetArrayTypeP(arrayDatum); + + if (ARR_NDIM(a) != 1 || + ARR_LBOUND(a)[0] != 1 || + ARR_HASNULL(a) || + ARR_ELEMTYPE(a) != TEXTOID) + elog(ERROR, "extcondition is not a 1-D text array"); + if (ARR_DIMS(a)[0] != arrayLength) + elog(ERROR, "extconfig and extcondition arrays do not match"); + } + + if (arrayLength <= 1) + { + /* removing only element, just set array to null */ + repl_null[Anum_pg_extension_extcondition - 1] = true; + } + else + { + /* squeeze out the target element */ + Datum *dvalues; + bool *dnulls; + int nelems; + int i; + + deconstruct_array(a, TEXTOID, -1, false, 'i', + &dvalues, &dnulls, &nelems); + + /* We already checked there are no nulls, so ignore dnulls */ + for (i = arrayIndex; i < arrayLength - 1; i++) + dvalues[i] = dvalues[i + 1]; + + a = construct_array(dvalues, arrayLength - 1, + TEXTOID, -1, false, 'i'); + + repl_val[Anum_pg_extension_extcondition - 1] = PointerGetDatum(a); + } + repl_repl[Anum_pg_extension_extcondition - 1] = true; + + extTup = heap_modify_tuple(extTup, RelationGetDescr(extRel), + repl_val, repl_null, repl_repl); + + simple_heap_update(extRel, &extTup->t_self, extTup); + CatalogUpdateIndexes(extRel, extTup); + + systable_endscan(extScan); + + heap_close(extRel, RowExclusiveLock); +} + /* * Execute ALTER EXTENSION SET SCHEMA */ @@ -2745,6 +2955,13 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt) ExtensionRelationId, DEPENDENCY_EXTENSION) != 1) elog(ERROR, "unexpected number of extension dependency records"); + + /* + * If it's a relation, it might have an entry in the extension's + * extconfig array, which we must remove. + */ + if (object.classId == RelationRelationId) + extension_config_remove(extension.objectId, object.objectId); } /* -- 2.40.0