public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v7 2/3] Remove "dead" flag from catcache tuple
4+ messages / 3 participants
[nested] [flat]
* [PATCH v7 2/3] Remove "dead" flag from catcache tuple
@ 2020-11-18 07:57 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Kyotaro Horiguchi @ 2020-11-18 07:57 UTC (permalink / raw)
---
src/backend/utils/cache/catcache.c | 43 +++++++++++++-----------------
src/include/utils/catcache.h | 10 -------
2 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 644d92dd9a..611b65168d 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -480,6 +480,13 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct)
Assert(ct->refcount == 0);
Assert(ct->my_cache == cache);
+ /* delink from linked list if not yet */
+ if (ct->cache_elem.prev)
+ {
+ dlist_delete(&ct->cache_elem);
+ ct->cache_elem.prev = NULL;
+ }
+
if (ct->c_list)
{
/*
@@ -487,14 +494,10 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct)
* which will recurse back to me, and the recursive call will do the
* work. Set the "dead" flag to make sure it does recurse.
*/
- ct->dead = true;
CatCacheRemoveCList(cache, ct->c_list);
return; /* nothing left to do */
}
- /* delink from linked list */
- dlist_delete(&ct->cache_elem);
-
/*
* Free keys when we're dealing with a negative entry, normal entries just
* point into tuple, allocated together with the CatCTup.
@@ -534,7 +537,7 @@ CatCacheRemoveCList(CatCache *cache, CatCList *cl)
/* if the member is dead and now has no references, remove it */
if (
#ifndef CATCACHE_FORCE_RELEASE
- ct->dead &&
+ ct->cache_elem.prev == NULL &&
#endif
ct->refcount == 0)
CatCacheRemoveCTup(cache, ct);
@@ -609,7 +612,9 @@ CatCacheInvalidate(CatCache *cache, uint32 hashValue)
if (ct->refcount > 0 ||
(ct->c_list && ct->c_list->refcount > 0))
{
- ct->dead = true;
+ dlist_delete(&ct->cache_elem);
+ ct->cache_elem.prev = NULL;
+
/* list, if any, was marked dead above */
Assert(ct->c_list == NULL || ct->c_list->dead);
}
@@ -688,7 +693,8 @@ ResetCatalogCache(CatCache *cache)
if (ct->refcount > 0 ||
(ct->c_list && ct->c_list->refcount > 0))
{
- ct->dead = true;
+ dlist_delete(&ct->cache_elem);
+ ct->cache_elem.prev = NULL;
/* list, if any, was marked dead above */
Assert(ct->c_list == NULL || ct->c_list->dead);
}
@@ -1268,9 +1274,6 @@ SearchCatCacheInternal(CatCache *cache,
{
ct = dlist_container(CatCTup, cache_elem, iter.cur);
- if (ct->dead)
- continue; /* ignore dead entries */
-
if (ct->hash_value != hashValue)
continue; /* quickly skip entry if wrong hash val */
@@ -1522,7 +1525,6 @@ ReleaseCatCache(HeapTuple tuple)
offsetof(CatCTup, tuple));
/* Safety checks to ensure we were handed a cache entry */
- Assert(ct->ct_magic == CT_MAGIC);
Assert(ct->refcount > 0);
ct->refcount--;
@@ -1530,7 +1532,7 @@ ReleaseCatCache(HeapTuple tuple)
if (
#ifndef CATCACHE_FORCE_RELEASE
- ct->dead &&
+ ct->cache_elem.prev == NULL &&
#endif
ct->refcount == 0 &&
(ct->c_list == NULL || ct->c_list->refcount == 0))
@@ -1737,8 +1739,8 @@ SearchCatCacheList(CatCache *cache,
{
ct = dlist_container(CatCTup, cache_elem, iter.cur);
- if (ct->dead || ct->negative)
- continue; /* ignore dead and negative entries */
+ if (ct->negative)
+ continue; /* ignore negative entries */
if (ct->hash_value != hashValue)
continue; /* quickly skip entry if wrong hash val */
@@ -1799,14 +1801,13 @@ SearchCatCacheList(CatCache *cache,
{
foreach(ctlist_item, ctlist)
{
+ Assert (ct->cache_elem.prev != NULL);
+
ct = (CatCTup *) lfirst(ctlist_item);
Assert(ct->c_list == NULL);
Assert(ct->refcount > 0);
ct->refcount--;
if (
-#ifndef CATCACHE_FORCE_RELEASE
- ct->dead &&
-#endif
ct->refcount == 0 &&
(ct->c_list == NULL || ct->c_list->refcount == 0))
CatCacheRemoveCTup(cache, ct);
@@ -1834,9 +1835,6 @@ SearchCatCacheList(CatCache *cache,
/* release the temporary refcount on the member */
Assert(ct->refcount > 0);
ct->refcount--;
- /* mark list dead if any members already dead */
- if (ct->dead)
- cl->dead = true;
}
Assert(i == nmembers);
@@ -1960,11 +1958,9 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
* Finish initializing the CatCTup header, and add it to the cache's
* linked list and counts.
*/
- ct->ct_magic = CT_MAGIC;
ct->my_cache = cache;
ct->c_list = NULL;
ct->refcount = 0; /* for the moment */
- ct->dead = false;
ct->negative = negative;
ct->hash_value = hashValue;
ct->lastaccess = catcacheclock;
@@ -2158,9 +2154,6 @@ PrintCatCacheLeakWarning(HeapTuple tuple)
CatCTup *ct = (CatCTup *) (((char *) tuple) -
offsetof(CatCTup, tuple));
- /* Safety check to ensure we were handed a cache entry */
- Assert(ct->ct_magic == CT_MAGIC);
-
elog(WARNING, "cache reference leak: cache %s (%d), tuple %u/%u has count %d",
ct->my_cache->cc_relname, ct->my_cache->id,
ItemPointerGetBlockNumber(&(tuple->t_self)),
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index 291e857e38..53b0bf31eb 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -87,9 +87,6 @@ typedef struct catcache
typedef struct catctup
{
- int ct_magic; /* for identifying CatCTup entries */
-#define CT_MAGIC 0x57261502
-
uint32 hash_value; /* hash value for this tuple's keys */
/*
@@ -106,19 +103,12 @@ typedef struct catctup
dlist_node cache_elem; /* list member of per-bucket list */
/*
- * A tuple marked "dead" must not be returned by subsequent searches.
- * However, it won't be physically deleted from the cache until its
- * refcount goes to zero. (If it's a member of a CatCList, the list's
- * refcount must go to zero, too; also, remember to mark the list dead at
- * the same time the tuple is marked.)
- *
* A negative cache entry is an assertion that there is no tuple matching
* a particular key. This is just as useful as a normal entry so far as
* avoiding catalog searches is concerned. Management of positive and
* negative entries is identical.
*/
int refcount; /* number of active references */
- bool dead; /* dead but not yet removed? */
bool negative; /* negative cache entry? */
HeapTupleData tuple; /* tuple management header */
uint64 lastaccess; /* timestamp in us of the last usage */
--
2.27.0
----Next_Part(Wed_Jan_27_10_13_08_2021_482)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="v7-0003-catcachebench.patch"
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Ability to reference other extensions by schema in extension scripts
@ 2023-02-23 18:39 Sandro Santilli <[email protected]>
2023-02-25 20:40 ` RE: Ability to reference other extensions by schema in extension scripts Regina Obe <[email protected]>
2023-02-26 06:39 ` RE: Ability to reference other extensions by schema in extension scripts Regina Obe <[email protected]>
0 siblings, 2 replies; 4+ messages in thread
From: Sandro Santilli @ 2023-02-23 18:39 UTC (permalink / raw)
To: Regina Obe <[email protected]>; +Cc: 'Tom Lane' <[email protected]>; pgsql-hackers
On Mon, Feb 06, 2023 at 05:19:39AM -0500, Regina Obe wrote:
>
> Attached is a revised version of the original patch. It is revised to
> prevent
>
> ALTER EXTENSION .. SET SCHEMA if there is a dependent extension that
> references the extension in their scripts using @extschema:extensionname@
> It also adds additional tests to verify that new feature.
>
> In going thru the code base, I was tempted to add a new dependency type
> instead of using the existing DEPENDENCY_AUTO. I think this would be
> cleaner, but I felt that was overstepping the area a bit, since it requires
> making changes to dependency.h and dependency.c
>
> My main concern with using DEPENDENCY_AUTO is because it was designed for
> cases where an object can be dropped without need for CASCADE. In this
> case, we don't want a dependent extension to be dropped if it's required is
> dropped. However since there will already exist
> a DEPENDENCY_NORMAL between the 2 extensions, I figure we are protected
> against that issue already.
I was thinking: how about using the "refobjsubid" to encode the
"level" of dependency on an extension ? Right now "refobjsubid" is
always 0 when the referenced object is an extension.
Could we consider subid=1 to mean the dependency is not only
on the extension but ALSO on it's schema location ?
Also: should we really allow extensions to rely on other extension
w/out fully-qualifying calls to their functions ? Or should it be
discouraged and thus forbidden ? If we wanted to forbid it we then
would not need to encode any additional dependency but rather always
forbid `ALTER EXTENSION .. SET SCHEMA` whenever the extension is
a dependency of any other extension.
On the code in the patch itself, I tried with this simple use case:
- ext1, relocatable, exposes an ext1log(text) function
- ext2, relocatable, exposes an ext2log(text) function
calling @extschema:[email protected]()
What is not good:
- Drop of ext1 automatically cascades to drop of ext2 without even a notice:
test=# create extension ext2 cascade;
NOTICE: installing required extension "ext1"
CREATE EXTENSION
test=# drop extension ext1;
DROP EXTENSION -- no WARNING, no NOTICE, ext2 is gone
What is good:
- ext1 cannot be relocated while ext2 is loaded:
test=# create extension ext2 cascade;
NOTICE: installing required extension "ext1"
CREATE EXTENSION
test=# alter extension ext1 set schema n1;
ERROR: Extension can not be relocated because dependent extension references it's location
test=# drop extension ext2;
DROP EXTENSION
test=# alter extension ext1 set schema n1;
ALTER EXTENSION
--strk;
Libre GIS consultant/developer
https://strk.kbt.io/services.html
^ permalink raw reply [nested|flat] 4+ messages in thread
* RE: Ability to reference other extensions by schema in extension scripts
2023-02-23 18:39 Re: Ability to reference other extensions by schema in extension scripts Sandro Santilli <[email protected]>
@ 2023-02-25 20:40 ` Regina Obe <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: Regina Obe @ 2023-02-25 20:40 UTC (permalink / raw)
To: [email protected]; +Cc: 'Tom Lane' <[email protected]>; pgsql-hackers
> On Mon, Feb 06, 2023 at 05:19:39AM -0500, Regina Obe wrote:
> >
> > Attached is a revised version of the original patch. It is revised to
> > prevent
> >
> > ALTER EXTENSION .. SET SCHEMA if there is a dependent extension that
> > references the extension in their scripts using
> > @extschema:extensionname@ It also adds additional tests to verify that
> new feature.
> >
> > In going thru the code base, I was tempted to add a new dependency
> > type instead of using the existing DEPENDENCY_AUTO. I think this
> > would be cleaner, but I felt that was overstepping the area a bit,
> > since it requires making changes to dependency.h and dependency.c
> >
> > My main concern with using DEPENDENCY_AUTO is because it was designed
> > for cases where an object can be dropped without need for CASCADE. In
> > this case, we don't want a dependent extension to be dropped if it's
> > required is dropped. However since there will already exist a
> > DEPENDENCY_NORMAL between the 2 extensions, I figure we are protected
> > against that issue already.
>
> I was thinking: how about using the "refobjsubid" to encode the "level" of
> dependency on an extension ? Right now "refobjsubid" is always 0 when the
> referenced object is an extension.
> Could we consider subid=1 to mean the dependency is not only on the
> extension but ALSO on it's schema location ?
>
I like that idea. It's only been ever used for tables I think, but I don't
see why it wouldn't apply in this case as the concept is kinda the same.
Only concern if other parts rely on this being 0.
The other question, should this just update the existing DEPENDENCY_NORMAL
extension or add a new DEPENDENCY_NORMAL between the extensions with
subid=1?
> Also: should we really allow extensions to rely on other extension w/out
fully-
> qualifying calls to their functions ? Or should it be discouraged and thus
> forbidden ? If we wanted to forbid it we then would not need to encode any
> additional dependency but rather always forbid `ALTER EXTENSION .. SET
> SCHEMA` whenever the extension is a dependency of any other extension.
>
> On the code in the patch itself, I tried with this simple use case:
>
> - ext1, relocatable, exposes an ext1log(text) function
>
> - ext2, relocatable, exposes an ext2log(text) function
> calling @extschema:[email protected]()
>
This would be an okay solution to me too if everyone is okay with it.
> What is not good:
>
> - Drop of ext1 automatically cascades to drop of ext2 without even a
> notice:
>
> test=# create extension ext2 cascade;
> NOTICE: installing required extension "ext1"
> CREATE EXTENSION
> test=# drop extension ext1;
> DROP EXTENSION -- no WARNING, no NOTICE, ext2 is gone
>
Oops. I don't know why I thought the normal dependency would protect
against this. I should have tested that. So DEPENDENCY_AUTO is not an
option to use and creating a new type of dependency seems like over stepping
the bounds of this patch.
> What is good:
>
> - ext1 cannot be relocated while ext2 is loaded:
>
> test=# create extension ext2 cascade;
> NOTICE: installing required extension "ext1"
> CREATE EXTENSION
> test=# alter extension ext1 set schema n1;
> ERROR: Extension can not be relocated because dependent
> extension references it's location
> test=# drop extension ext2;
> DROP EXTENSION
> test=# alter extension ext1 set schema n1;
> ALTER EXTENSION
>
> --strk;
>
> Libre GIS consultant/developer
> https://strk.kbt.io/services.html
So in conclusion we have 3 possible paths to go with this
1) Just don't allow any extensions referenced by other extensions to be
relocatable.
It will show a message something like
"SET SCHEMA not allowed because other extensions depend on it"
Given that if you don't specify relocatable in you .control file, the assume
is relocatable = false , this isn't too far off from standard protocol.
2) Use objsubid=1 to denote that another extension explicitly references the
schema of another extension so setting schema of other extension is not
okay. So instead of introducing another dependency, we'd update the
DEPENDENCY_NORMAL one between the two schemas with objsubid=1 instead of 0.
This has 2 approaches:
a) Update the existing DEPENDENCY_NORMAL between the two extensions setting
the objsubid=1
or
b) Create a new DEPEDENCY_NORMAL between the two extensions with objsubid=1
I'm not sure if either has implications in backup / restore . I suspect b
would be safer since I suspect objsubid might be checked and this
dependency only needs checking during SET SCHEMA time.
3) Create a whole new DEPENDENCY type, perhaps calling it something like
DEPENDENCY_EXTENSION_SCHEMA
4) Just don't allow @extschema:<reqextension>@ syntax to be used unless the
<reqextension> is marked as relocatable=false. This one I don't like
because it doesn't solve my fundamental issue of
postgis_tiger_geocoder relying on fuzzystrmatch, which is marked as
relocatable.
The main issue I was trying to solve is my extension references
fuzzystrmatch functions in a function used for functional indexes, and this
fails restore of table indexes because I can't schema qualify the
fuzzystrmatch extension in the backing function.
If no one has any opinion, I'll go with option 1 which is the one that strk
had actually proposed before and seems least programmatically invasive, but
perhaps more annoying user facing.
My preferred would be #2
Thanks,
Regina
^ permalink raw reply [nested|flat] 4+ messages in thread
* RE: Ability to reference other extensions by schema in extension scripts
2023-02-23 18:39 Re: Ability to reference other extensions by schema in extension scripts Sandro Santilli <[email protected]>
@ 2023-02-26 06:39 ` Regina Obe <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: Regina Obe @ 2023-02-26 06:39 UTC (permalink / raw)
To: [email protected]; +Cc: 'Tom Lane' <[email protected]>; pgsql-hackers
> So in conclusion we have 3 possible paths to go with this
>
> 1) Just don't allow any extensions referenced by other extensions to be
> relocatable.
> It will show a message something like
> "SET SCHEMA not allowed because other extensions depend on it"
> Given that if you don't specify relocatable in you .control file, the
assume is
> relocatable = false , this isn't too far off from standard protocol.
>
> 2) Use objsubid=1 to denote that another extension explicitly references
the
> schema of another extension so setting schema of other extension is not
okay.
> So instead of introducing another dependency, we'd update the
> DEPENDENCY_NORMAL one between the two schemas with objsubid=1
> instead of 0.
>
> This has 2 approaches:
>
> a) Update the existing DEPENDENCY_NORMAL between the two extensions
> setting the objsubid=1
>
> or
> b) Create a new DEPEDENCY_NORMAL between the two extensions with
> objsubid=1
>
> I'm not sure if either has implications in backup / restore . I suspect b
would
> be safer since I suspect objsubid might be checked and this dependency
only
> needs checking during SET SCHEMA time.
>
> 3) Create a whole new DEPENDENCY type, perhaps calling it something like
> DEPENDENCY_EXTENSION_SCHEMA
>
> 4) Just don't allow @extschema:<reqextension>@ syntax to be used unless
> the <reqextension> is marked as relocatable=false. This one I don't like
> because it doesn't solve my fundamental issue of
>
> postgis_tiger_geocoder relying on fuzzystrmatch, which is marked as
> relocatable.
>
> The main issue I was trying to solve is my extension references
fuzzystrmatch
> functions in a function used for functional indexes, and this fails
restore of
> table indexes because I can't schema qualify the fuzzystrmatch extension
in
> the backing function.
>
>
> If no one has any opinion, I'll go with option 1 which is the one that
strk had
> actually proposed before and seems least programmatically invasive, but
> perhaps more annoying user facing.
>
> My preferred would be #2
>
> Thanks,
> Regina
Attached is my revision 3 patch, which follows the proposed #1.
Don't allow schema relocation of an extension if another extension requires
it.
Attachments:
[application/octet-stream] 0003-Allow-use-of-extschema-reqextname-to-reference.patch (15.2K, ../../[email protected]/2-0003-Allow-use-of-extschema-reqextname-to-reference.patch)
download | inline diff:
From d25ae7c6795922c82921fb9abfd29719fcc8bdc0 Mon Sep 17 00:00:00 2001
From: Regina Obe <[email protected]>
Date: Sun, 5 Feb 2023 01:35:29 -0500
Subject: [PATCH 3/3] Allow @extschema:<extname>@ in extension scripts
Changes to extension execute logic to allow referencing required
extension schemas by variable
This feature is needed in cases such as
when another extension packages a function
which references an object from one of required extensions
and this function is used to back
indexes, constraints, and materialized views
Without this feature, because pg_restore removes the paths,
these indexes, constraints, materialized views are not restored
because when the function is called, the function call fails
because the dependent item is not schema qualified.
- Extension tests both Makefile and meson.build
- Documentation of the new feature
- Prevent an extension from being relocated if
another extension requires it
Discussion: https://postgr.es/m/000801d94959%2463a9f520%242afddf60%24%40pcorp.us
---
doc/src/sgml/extend.sgml | 17 ++++++
src/backend/commands/extension.c | 55 +++++++++++++++++++
src/test/modules/test_extensions/Makefile | 9 ++-
.../expected/test_extensions.out | 40 ++++++++++++++
src/test/modules/test_extensions/meson.build | 7 +++
.../test_extensions/sql/test_extensions.sql | 14 +++++
.../test_ext_req_schema1--1.0.sql | 6 ++
.../test_ext_req_schema1.control | 3 +
.../test_ext_req_schema2--1.0--2.0.sql | 7 +++
.../test_ext_req_schema2--1.0.sql | 9 +++
.../test_ext_req_schema2.control | 4 ++
.../test_ext_req_schema3--1.0.sql | 13 +++++
.../test_ext_req_schema3.control | 4 ++
13 files changed, 186 insertions(+), 2 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema1.control
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema2.control
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_req_schema3.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index b70cbe83ae..6c93bddb36 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -908,6 +908,23 @@ RETURNS anycompatible AS ...
</para>
</listitem>
+ <listitem>
+ <para>
+ An extension might depend on other extensions.
+ It is useful to schema qualify calls to dependent extension to minimize reliance on search_path.
+ This is critical for cases such as functions used in indexes, materialized views, or check constraints.
+ To reference a required extension's schema, you must first have <literal>requires</literal>
+ variable specifying the list of extensions your extension requires.
+ In your extension sql scripts,
+ you can reference a required extension's schema with syntax
+ <literal>@extschema:reqextname@</literal> where <literal>reqextname</literal>
+ is the name of an extension in your <literal>requires</literal> list.
+ All occurrences of this string will be
+ replaced by the schema the required extension is installed in before the script is
+ executed.
+ </para>
+ </listitem>
+
<listitem>
<para>
If the extension does not support relocation at all, set
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index b1509cc505..57a21ba41e 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -854,6 +854,7 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
const char *schemaName, Oid schemaOid)
{
bool switch_to_superuser = false;
+ char *origSchemaName = (char *) schemaName;
char *filename;
Oid save_userid = 0;
int save_sec_context = 0;
@@ -1030,6 +1031,46 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
CStringGetTextDatum(qSchemaName));
}
+ /*
+ * If this extension requires other extensions
+ * Check each required extension to see if it's schema
+ * is referenced by @extschema:reqextname@ syntax
+ */
+ if (control->requires)
+ {
+ foreach(lc, control->requires)
+ {
+ char *curreq;
+ curreq = (char *) lfirst(lc);
+ Oid reqext;
+ Oid reqschema;
+ reqext = get_required_extension(curreq,
+ control->name,
+ origSchemaName,
+ false,
+ NIL,
+ false);
+ reqschema = get_extension_schema(reqext);
+ char *reqname;
+ reqname = get_namespace_name(reqschema);
+ StringInfoData rToken;
+ initStringInfo(&rToken);
+ appendStringInfo(&rToken, "%s%s%s", "@extschema:", curreq, "@");
+ if (strstr(c_sql,rToken.data) != NULL){
+ /*
+ * If the required extension's schema is referenced
+ * by variable name,
+ * Replace each occurence of @extschema:reqextname@
+ * with the required extension's schema
+ */
+ t_sql = DirectFunctionCall3Coll(replace_text,
+ C_COLLATION_OID,
+ t_sql,
+ CStringGetTextDatum(rToken.data),
+ CStringGetTextDatum(quote_identifier(reqname)));
+ }
+ }
+ }
/*
* If module_pathname was set in the control file, substitute its
* value for occurrences of MODULE_PATHNAME.
@@ -2816,6 +2857,20 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
ObjectAddress dep;
Oid dep_oldNspOid;
+ /* If an extension requires this extension
+ * do not allow relocation */
+ if (pg_depend->deptype == DEPENDENCY_NORMAL && pg_depend->classid == ExtensionRelationId){
+ dep.classId = pg_depend->classid;
+ dep.objectId = pg_depend->objid;
+ dep.objectSubId = pg_depend->objsubid;
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot SET SCHEMA of extension %s because other extensions require it",
+ NameStr(extForm->extname)),
+ errdetail("%s requires extension %s",
+ getObjectDescription(&dep, false), NameStr(extForm->extname))));
+
+ }
/*
* Ignore non-membership dependencies. (Currently, the only other
* case we could see here is a normal dependency from another
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index c3139ab0fc..c073df963c 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -6,14 +6,19 @@ PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext7 test_ext8 test_ext_cine test_ext_cor \
test_ext_cyclic1 test_ext_cyclic2 \
- test_ext_evttrig
+ test_ext_evttrig \
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
+
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
test_ext_cor--1.0.sql \
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
- test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
+ test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+ test_ext_req_schema1--1.0.sql \
+ test_ext_req_schema2--1.0.sql test_ext_req_schema2--1.0--2.0.sql \
+ test_ext_req_schema3--1.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 821fed38d1..21dfd05982 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -312,3 +312,43 @@ Objects in extension "test_ext_cine"
table ext_cine_tab3
(9 rows)
+CREATE SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema1 SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema3 CASCADE;
+NOTICE: installing required extension "test_ext_req_schema2"
+SELECT dep_req();
+ dep_req
+---------
+ 1032w
+(1 row)
+
+SELECT dep_req2();
+ dep_req2
+----------
+ 1032w
+(1 row)
+
+SELECT dep_req3();
+ dep_req3
+----------
+ 2032w
+(1 row)
+
+ALTER EXTENSION test_ext_req_schema2 UPDATE TO '2.0';
+CREATE SCHEMA test_s_dep2;
+ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2;
+ERROR: cannot SET SCHEMA of extension test_ext_req_schema1 because other extensions require it
+DETAIL: extension test_ext_req_schema3 requires extension test_ext_req_schema1
+SELECT dep_req();
+ dep_req
+---------
+ 1update
+(1 row)
+
+DROP EXTENSION test_ext_req_schema1;
+ERROR: cannot drop extension test_ext_req_schema1 because other objects depend on it
+DETAIL: extension test_ext_req_schema2 depends on extension test_ext_req_schema1
+extension test_ext_req_schema3 depends on extension test_ext_req_schema1
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP EXTENSION test_ext_req_schema3;
+DROP EXTENSION test_ext_req_schema2;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index 45597ddc23..7519f6e28e 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -31,6 +31,13 @@ install_data(
'test_ext_evttrig--1.0--2.0.sql',
'test_ext_evttrig--1.0.sql',
'test_ext_evttrig.control',
+ 'test_ext_req_schema1--1.0.sql',
+ 'test_ext_req_schema1.control',
+ 'test_ext_req_schema2--1.0.sql',
+ 'test_ext_req_schema2.control',
+ 'test_ext_req_schema2--1.0--2.0.sql',
+ 'test_ext_req_schema3.control',
+ 'test_ext_req_schema3--1.0.sql',
kwargs: contrib_data_args,
)
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 41b6cddf0b..81ecd13736 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -209,3 +209,17 @@ CREATE EXTENSION test_ext_cine;
ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
\dx+ test_ext_cine
+
+CREATE SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema1 SCHEMA test_s_dep;
+CREATE EXTENSION test_ext_req_schema3 CASCADE;
+SELECT dep_req();
+SELECT dep_req2();
+SELECT dep_req3();
+ALTER EXTENSION test_ext_req_schema2 UPDATE TO '2.0';
+CREATE SCHEMA test_s_dep2;
+ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2;
+SELECT dep_req();
+DROP EXTENSION test_ext_req_schema1;
+DROP EXTENSION test_ext_req_schema3;
+DROP EXTENSION test_ext_req_schema2;
\ No newline at end of file
diff --git a/src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql b/src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql
new file mode 100644
index 0000000000..462fb52145
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql
@@ -0,0 +1,6 @@
+/* src/test/modules/test_extensions/test_ext_req_schema1--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema1" to load this file. \quit
+
+CREATE DOMAIN req AS text
+ CONSTRAINT starts_with_1 check(pg_catalog.left(value,1) OPERATOR(pg_catalog.=) '1');
diff --git a/src/test/modules/test_extensions/test_ext_req_schema1.control b/src/test/modules/test_extensions/test_ext_req_schema1.control
new file mode 100644
index 0000000000..9ea4558a90
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema1.control
@@ -0,0 +1,3 @@
+comment = 'Create required extension to be referenced'
+default_version = '1.0'
+relocatable = true
diff --git a/src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql b/src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql
new file mode 100644
index 0000000000..73a44a25e5
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema2--1.0--2.0.sql
@@ -0,0 +1,7 @@
+/* src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema2" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION dep_req() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''1update''::@extschema:[email protected]';
diff --git a/src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql b/src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql
new file mode 100644
index 0000000000..9fe25d48a1
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql
@@ -0,0 +1,9 @@
+/* src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema2" to load this file. \quit
+CREATE DOMAIN dreq AS text
+ CONSTRAINT starts_with_2 check(pg_catalog.left(value,1) OPERATOR(pg_catalog.=) '2');
+
+CREATE FUNCTION dep_req() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''1032w''::@extschema:[email protected]';
diff --git a/src/test/modules/test_extensions/test_ext_req_schema2.control b/src/test/modules/test_extensions/test_ext_req_schema2.control
new file mode 100644
index 0000000000..d2ba5add97
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema2.control
@@ -0,0 +1,4 @@
+comment = 'Test schema referencing of required extensions'
+default_version = '1.0'
+relocatable = true
+requires = 'test_ext_req_schema1'
diff --git a/src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql b/src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql
new file mode 100644
index 0000000000..bd05669097
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema3--1.0.sql
@@ -0,0 +1,13 @@
+/* src/test/modules/test_extensions/test_ext_req_schema2--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_req_schema2" to load this file. \quit
+CREATE DOMAIN req2 AS text
+ CONSTRAINT starts_with_2 check(pg_catalog.left(value,1) OPERATOR(pg_catalog.=) '2');
+
+CREATE FUNCTION dep_req2() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''1032w''::@extschema:[email protected]';
+
+CREATE FUNCTION dep_req3() RETURNS @extschema:[email protected]
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+AS 'SELECT ''2032w''::@extschema:[email protected]';
diff --git a/src/test/modules/test_extensions/test_ext_req_schema3.control b/src/test/modules/test_extensions/test_ext_req_schema3.control
new file mode 100644
index 0000000000..b052fad785
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_req_schema3.control
@@ -0,0 +1,4 @@
+comment = 'Test schema referencing of 2 required extensions'
+default_version = '1.0'
+relocatable = true
+requires = 'test_ext_req_schema1,test_ext_req_schema2'
--
2.21.0.windows.1
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2023-02-26 06:39 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-18 07:57 [PATCH v7 2/3] Remove "dead" flag from catcache tuple Kyotaro Horiguchi <[email protected]>
2023-02-23 18:39 Re: Ability to reference other extensions by schema in extension scripts Sandro Santilli <[email protected]>
2023-02-25 20:40 ` RE: Ability to reference other extensions by schema in extension scripts Regina Obe <[email protected]>
2023-02-26 06:39 ` RE: Ability to reference other extensions by schema in extension scripts Regina Obe <[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