Commit 343c2a86 authored by Tom Lane's avatar Tom Lane

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.
parent 343ee00b
...@@ -665,6 +665,10 @@ SET LOCAL search_path TO @extschema@; ...@@ -665,6 +665,10 @@ SET LOCAL search_path TO @extschema@;
and reload. and reload.
</para> </para>
<indexterm>
<primary>pg_extension_config_dump</primary>
</indexterm>
<para> <para>
To solve this problem, an extension's script file can mark a table To solve this problem, an extension's script file can mark a table
it has created as a configuration table, which will cause 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 ...@@ -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 be modified by users, can be handled by creating triggers on the
configuration table to ensure that modified rows are marked correctly. configuration table to ensure that modified rows are marked correctly.
</para> </para>
<para>
You can alter the filter condition associated with a configuration table
by calling <function>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 <command>ALTER EXTENSION ... DROP TABLE</>.
</para>
</sect2> </sect2>
<sect2> <sect2>
......
...@@ -2050,6 +2050,7 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) ...@@ -2050,6 +2050,7 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
HeapTuple extTup; HeapTuple extTup;
Datum arrayDatum; Datum arrayDatum;
Datum elementDatum; Datum elementDatum;
int arrayLength;
int arrayIndex; int arrayIndex;
bool isnull; bool isnull;
Datum repl_val[Natts_pg_extension]; Datum repl_val[Natts_pg_extension];
...@@ -2069,8 +2070,8 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) ...@@ -2069,8 +2070,8 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
/* /*
* Check that the table exists and is a member of the extension being * 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 * created. This ensures that we don't need to register an additional
* protect the extconfig entry. * dependency to protect the extconfig entry.
*/ */
tablename = get_rel_name(tableoid); tablename = get_rel_name(tableoid);
if (tablename == NULL) if (tablename == NULL)
...@@ -2087,6 +2088,9 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) ...@@ -2087,6 +2088,9 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
/* /*
* Add the table OID and WHERE condition to the extension's extconfig and * Add the table OID and WHERE condition to the extension's extconfig and
* extcondition arrays. * extcondition arrays.
*
* If the table is already in extconfig, treat this as an update of the
* WHERE condition.
*/ */
/* Find the pg_extension tuple */ /* Find the pg_extension tuple */
...@@ -2117,18 +2121,41 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) ...@@ -2117,18 +2121,41 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
RelationGetDescr(extRel), &isnull); RelationGetDescr(extRel), &isnull);
if (isnull) if (isnull)
{ {
/* Previously empty extconfig, so build 1-element array */
arrayLength = 0;
arrayIndex = 1;
a = construct_array(&elementDatum, 1, a = construct_array(&elementDatum, 1,
OIDOID, OIDOID,
sizeof(Oid), true, 'i'); sizeof(Oid), true, 'i');
} }
else else
{ {
/* Modify or extend existing extconfig array */
Oid *arrayData;
int i;
a = DatumGetArrayTypeP(arrayDatum); 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, a = array_set(a, 1, &arrayIndex,
elementDatum, elementDatum,
...@@ -2148,6 +2175,9 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) ...@@ -2148,6 +2175,9 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
RelationGetDescr(extRel), &isnull); RelationGetDescr(extRel), &isnull);
if (isnull) if (isnull)
{ {
if (arrayLength != 0)
elog(ERROR, "extconfig and extcondition arrays do not match");
a = construct_array(&elementDatum, 1, a = construct_array(&elementDatum, 1,
TEXTOID, TEXTOID,
-1, false, 'i'); -1, false, 'i');
...@@ -2155,12 +2185,16 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) ...@@ -2155,12 +2185,16 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
else else
{ {
a = DatumGetArrayTypeP(arrayDatum); 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, a = array_set(a, 1, &arrayIndex,
elementDatum, elementDatum,
false, false,
...@@ -2185,6 +2219,182 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) ...@@ -2185,6 +2219,182 @@ pg_extension_config_dump(PG_FUNCTION_ARGS)
PG_RETURN_VOID(); 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 * Execute ALTER EXTENSION SET SCHEMA
*/ */
...@@ -2745,6 +2955,13 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt) ...@@ -2745,6 +2955,13 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt)
ExtensionRelationId, ExtensionRelationId,
DEPENDENCY_EXTENSION) != 1) DEPENDENCY_EXTENSION) != 1)
elog(ERROR, "unexpected number of extension dependency records"); 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);
} }
/* /*
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment