This is an automated email from the ASF dual-hosted git repository. maxyang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit f83cb86feacf7bc125e275a481cbc63ff7cdc9a4 Author: Tom Lane <[email protected]> AuthorDate: Mon Aug 8 11:12:31 2022 -0400 In extensions, don't replace objects not belonging to the extension. Previously, if an extension script did CREATE OR REPLACE and there was an existing object not belonging to the extension, it would overwrite the object and adopt it into the extension. This is problematic, first because the overwrite is probably unintentional, and second because we didn't change the object's ownership. Thus a hostile user could create an object in advance of an expected CREATE EXTENSION command, and would then have ownership rights on an extension object, which could be modified for trojan-horse-type attacks. Hence, forbid CREATE OR REPLACE of an existing object unless it already belongs to the extension. (Note that we've always forbidden replacing an object that belongs to some other extension; only the behavior for previously-free-standing objects changes here.) For the same reason, also fail CREATE IF NOT EXISTS when there is an existing object that doesn't belong to the extension. Our thanks to Sven Klemm for reporting this problem. Security: CVE-2022-2625 --- doc/src/sgml/extend.sgml | 11 -- src/backend/catalog/pg_collation.c | 49 ++++-- src/backend/catalog/pg_depend.c | 79 ++++++++-- src/backend/catalog/pg_operator.c | 2 +- src/backend/catalog/pg_type.c | 17 +-- src/backend/commands/createas.c | 34 ++++- src/backend/commands/foreigncmds.c | 19 ++- src/backend/commands/schemacmds.c | 25 +++- src/backend/commands/sequence.c | 8 + src/backend/commands/statscmds.c | 4 + src/backend/commands/view.c | 16 +- src/backend/parser/parse_utilcmd.c | 10 ++ src/include/catalog/dependency.h | 2 + src/test/modules/test_extensions/Makefile | 12 +- .../test_extensions/expected/test_extensions.out | 165 ++++++++++++++++++++- .../test_extensions/sql/test_extensions.sql | 115 +++++++++++++- .../test_extensions/test_ext_cine--1.0--1.1.sql | 26 ++++ .../modules/test_extensions/test_ext_cine--1.0.sql | 25 ++++ .../modules/test_extensions/test_ext_cine.control | 3 + .../modules/test_extensions/test_ext_cor--1.0.sql | 20 +++ .../modules/test_extensions/test_ext_cor.control | 3 + 21 files changed, 568 insertions(+), 77 deletions(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index e928894726..bb0b2679bb 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -1319,17 +1319,6 @@ SELECT * FROM pg_extension_update_paths('<replaceable>extension_name</replaceabl trusted if it depends on another one, unless that other one is always installed in <literal>pg_catalog</literal>. </para> - - <para> - Do <emphasis>not</emphasis> use <command>CREATE OR REPLACE - FUNCTION</command>, except in an update script that must change the - definition of a function that is known to be an extension member - already. (Likewise for other <literal>OR REPLACE</literal> options.) - Using <literal>OR REPLACE</literal> unnecessarily not only has a risk - of accidentally overwriting someone else's function, but it creates a - security hazard since the overwritten function would still be owned by - its original owner, who could modify it. - </para> </sect3> </sect2> diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c index 6aaebac25c..7643afc166 100644 --- a/src/backend/catalog/pg_collation.c +++ b/src/backend/catalog/pg_collation.c @@ -80,15 +80,25 @@ CollationCreate(const char *collname, Oid collnamespace, * friendlier error message. The unique index provides a backstop against * race conditions. */ - if (SearchSysCacheExists3(COLLNAMEENCNSP, - PointerGetDatum(collname), - Int32GetDatum(collencoding), - ObjectIdGetDatum(collnamespace))) + oid = GetSysCacheOid3(COLLNAMEENCNSP, + Anum_pg_collation_oid, + PointerGetDatum(collname), + Int32GetDatum(collencoding), + ObjectIdGetDatum(collnamespace)); + if (OidIsValid(oid)) { if (quiet) return InvalidOid; else if (if_not_exists) { + /* + * If we are in an extension script, insist that the pre-existing + * object be a member of the extension, to avoid security risks. + */ + ObjectAddressSet(myself, CollationRelationId, oid); + checkMembershipInCurrentExtension(&myself); + + /* OK to skip */ ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), collencoding == -1 @@ -118,16 +128,19 @@ CollationCreate(const char *collname, Oid collnamespace, * so we take a ShareRowExclusiveLock earlier, to protect against * concurrent changes fooling this check. */ - if ((collencoding == -1 && - SearchSysCacheExists3(COLLNAMEENCNSP, - PointerGetDatum(collname), - Int32GetDatum(GetDatabaseEncoding()), - ObjectIdGetDatum(collnamespace))) || - (collencoding != -1 && - SearchSysCacheExists3(COLLNAMEENCNSP, - PointerGetDatum(collname), - Int32GetDatum(-1), - ObjectIdGetDatum(collnamespace)))) + if (collencoding == -1) + oid = GetSysCacheOid3(COLLNAMEENCNSP, + Anum_pg_collation_oid, + PointerGetDatum(collname), + Int32GetDatum(GetDatabaseEncoding()), + ObjectIdGetDatum(collnamespace)); + else + oid = GetSysCacheOid3(COLLNAMEENCNSP, + Anum_pg_collation_oid, + PointerGetDatum(collname), + Int32GetDatum(-1), + ObjectIdGetDatum(collnamespace)); + if (OidIsValid(oid)) { if (quiet) { @@ -136,6 +149,14 @@ CollationCreate(const char *collname, Oid collnamespace, } else if (if_not_exists) { + /* + * If we are in an extension script, insist that the pre-existing + * object be a member of the extension, to avoid security risks. + */ + ObjectAddressSet(myself, CollationRelationId, oid); + checkMembershipInCurrentExtension(&myself); + + /* OK to skip */ table_close(rel, NoLock); ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index ae16d2fefa..62cbf7a5ba 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -166,22 +166,23 @@ recordMultipleDependencies(const ObjectAddress *depender, /* * If we are executing a CREATE EXTENSION operation, mark the given object - * as being a member of the extension. Otherwise, do nothing. + * as being a member of the extension, or check that it already is one. + * Otherwise, do nothing. * * This must be called during creation of any user-definable object type * that could be a member of an extension. * - * If isReplace is true, the object already existed (or might have already - * existed), so we must check for a pre-existing extension membership entry. - * Passing false is a guarantee that the object is newly created, and so - * could not already be a member of any extension. + * isReplace must be true if the object already existed, and false if it is + * newly created. In the former case we insist that it already be a member + * of the current extension. In the latter case we can skip checking whether + * it is already a member of any extension. * - * Note: isReplace = true is typically used when updating an object in - * CREATE OR REPLACE and similar commands. The net effect is that if an - * extension script uses such a command on a pre-existing free-standing - * object, the object will be absorbed into the extension. If the object - * is already a member of some other extension, the command will fail. - * This behavior is desirable for cases such as replacing a shell type. + * Note: isReplace = true is typically used when updating a object in + * CREATE OR REPLACE and similar commands. We used to allow the target + * object to not already be an extension member, instead silently absorbing + * it into the current extension. However, this was both error-prone + * (extensions might accidentally overwrite free-standing objects) and + * a security hazard (since the object would retain its previous ownership). */ void recordDependencyOnCurrentExtension(const ObjectAddress *object, @@ -199,6 +200,12 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object, { Oid oldext; + /* + * Side note: these catalog lookups are safe only because the + * object is a pre-existing one. In the not-isReplace case, the + * caller has most likely not yet done a CommandCounterIncrement + * that would make the new object visible. + */ oldext = getExtensionOfObject(object->classId, object->objectId); if (OidIsValid(oldext)) { @@ -212,6 +219,13 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object, getObjectDescription(object, false), get_extension_name(oldext)))); } + /* It's a free-standing object, so reject */ + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("%s is not a member of extension \"%s\"", + getObjectDescription(object, false), + get_extension_name(CurrentExtensionObject)), + errdetail("An extension is not allowed to replace an object that it does not own."))); } /* OK, record it as a member of CurrentExtensionObject */ @@ -223,6 +237,49 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object, } } +/* + * If we are executing a CREATE EXTENSION operation, check that the given + * object is a member of the extension, and throw an error if it isn't. + * Otherwise, do nothing. + * + * This must be called whenever a CREATE IF NOT EXISTS operation (for an + * object type that can be an extension member) has found that an object of + * the desired name already exists. It is insecure for an extension to use + * IF NOT EXISTS except when the conflicting object is already an extension + * member; otherwise a hostile user could substitute an object with arbitrary + * properties. + */ +void +checkMembershipInCurrentExtension(const ObjectAddress *object) +{ + /* + * This is actually the same condition tested in + * recordDependencyOnCurrentExtension; but we want to issue a + * differently-worded error, and anyway it would be pretty confusing to + * call recordDependencyOnCurrentExtension in these circumstances. + */ + + /* Only whole objects can be extension members */ + Assert(object->objectSubId == 0); + + if (creating_extension) + { + Oid oldext; + + oldext = getExtensionOfObject(object->classId, object->objectId); + /* If already a member of this extension, OK */ + if (oldext == CurrentExtensionObject) + return; + /* Else complain */ + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("%s is not a member of extension \"%s\"", + getObjectDescription(object, false), + get_extension_name(CurrentExtensionObject)), + errdetail("An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns."))); + } +} + /* * deleteDependencyRecordsFor -- delete all records with given depender * classId/objectId. Returns the number of records deleted. diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index 24cba9e60f..aed985136a 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -871,7 +871,7 @@ makeOperatorDependencies(HeapTuple tuple, /* Dependency on extension */ if (makeExtensionDep) - recordDependencyOnCurrentExtension(&myself, true); + recordDependencyOnCurrentExtension(&myself, isUpdate); return myself; } diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c index b7cb5f5e4b..6d25414609 100644 --- a/src/backend/catalog/pg_type.c +++ b/src/backend/catalog/pg_type.c @@ -686,17 +686,12 @@ TypeCreate(Oid newTypeOid, * isDependentType is true if this is an implicit array or relation rowtype; * that means it doesn't need its own dependencies on owner etc. * - * We make an extension-membership dependency if we're in an extension - * script and makeExtensionDep is true (and isDependentType isn't true). - * makeExtensionDep should be true when creating a new type or replacing a - * shell type, but not for ALTER TYPE on an existing type. Passing false - * causes the type's extension membership to be left alone. - * - * rebuild should be true if this is a pre-existing type. We will remove - * existing dependencies and rebuild them from scratch. This is needed for - * ALTER TYPE, and also when replacing a shell type. We don't remove any - * existing extension dependency, though (hence, if makeExtensionDep is also - * true and the type belongs to some other extension, an error will occur). + * If rebuild is true, we remove existing dependencies and rebuild them + * from scratch. This is needed for ALTER TYPE, and also when replacing + * a shell type. We don't remove an existing extension dependency, though. + * That means an extension can't absorb a shell type that is free-standing + * or belongs to another extension, nor ALTER a type that is free-standing or + * belongs to another extension. */ void GenerateTypeDependencies(HeapTuple typeTuple, diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 6483985b40..37f4bac98d 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -346,8 +346,38 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt, Assert(Gp_role != GP_ROLE_EXECUTE); /* Check if the relation exists or not */ - if (CreateTableAsRelExists(stmt)) - return InvalidObjectAddress; + { + Oid nspid; + Oid oldrelid; + + nspid = RangeVarGetCreationNamespace(into->rel); + + oldrelid = get_relname_relid(into->rel->relname, nspid); + if (OidIsValid(oldrelid)) + { + if (!stmt->if_not_exists) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_TABLE), + errmsg("relation \"%s\" already exists", + into->rel->relname))); + + /* + * The relation exists and IF NOT EXISTS has been specified. + * + * If we are in an extension script, insist that the pre-existing + * object be a member of the extension, to avoid security risks. + */ + ObjectAddressSet(address, RelationRelationId, oldrelid); + checkMembershipInCurrentExtension(&address); + + /* OK to skip */ + ereport(NOTICE, + (errcode(ERRCODE_DUPLICATE_TABLE), + errmsg("relation \"%s\" already exists, skipping", + into->rel->relname))); + return InvalidObjectAddress; + } + } /* * Create the tuple receiver object and insert info it will need diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c index dc5f502aab..5cc7a8e5c2 100644 --- a/src/backend/commands/foreigncmds.c +++ b/src/backend/commands/foreigncmds.c @@ -892,13 +892,22 @@ CreateForeignServer(CreateForeignServerStmt *stmt) ownerId = GetUserId(); /* - * Check that there is no other foreign server by this name. Do nothing if - * IF NOT EXISTS was enforced. + * Check that there is no other foreign server by this name. If there is + * one, do nothing if IF NOT EXISTS was specified. */ - if (GetForeignServerByName(stmt->servername, true) != NULL) + srvId = get_foreign_server_oid(stmt->servername, true); + if (OidIsValid(srvId)) { if (stmt->if_not_exists) { + /* + * If we are in an extension script, insist that the pre-existing + * object be a member of the extension, to avoid security risks. + */ + ObjectAddressSet(myself, ForeignServerRelationId, srvId); + checkMembershipInCurrentExtension(&myself); + + /* OK to skip */ ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), errmsg("server \"%s\" already exists, skipping", @@ -1180,6 +1189,10 @@ CreateUserMapping(CreateUserMappingStmt *stmt) { if (stmt->if_not_exists) { + /* + * Since user mappings aren't members of extensions (see comments + * below), no need for checkMembershipInCurrentExtension here. + */ ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), errmsg("user mapping for \"%s\" already exists for server \"%s\", skipping", diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index a179bac952..93bc2ace73 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -157,14 +157,25 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString, * the permissions checks, but since CREATE TABLE IF NOT EXISTS makes its * creation-permission check first, we do likewise. */ - if (stmt->if_not_exists && - SearchSysCacheExists1(NAMESPACENAME, PointerGetDatum(schemaName))) + if (stmt->if_not_exists) { - ereport(NOTICE, - (errcode(ERRCODE_DUPLICATE_SCHEMA), - errmsg("schema \"%s\" already exists, skipping", - schemaName))); - return InvalidOid; + namespaceId = get_namespace_oid(schemaName, true); + if (OidIsValid(namespaceId)) + { + /* + * If we are in an extension script, insist that the pre-existing + * object be a member of the extension, to avoid security risks. + */ + ObjectAddressSet(address, NamespaceRelationId, namespaceId); + checkMembershipInCurrentExtension(&address); + + /* OK to skip */ + ereport(NOTICE, + (errcode(ERRCODE_DUPLICATE_SCHEMA), + errmsg("schema \"%s\" already exists, skipping", + schemaName))); + return InvalidOid; + } } /* Create the schema's namespace */ diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 825988bdb8..e989a7db05 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -187,6 +187,14 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq) RangeVarGetAndCheckCreationNamespace(seq->sequence, NoLock, &seqoid); if (OidIsValid(seqoid)) { + /* + * If we are in an extension script, insist that the pre-existing + * object be a member of the extension, to avoid security risks. + */ + ObjectAddressSet(address, RelationRelationId, seqoid); + checkMembershipInCurrentExtension(&address); + + /* OK to skip */ ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_TABLE), errmsg("relation \"%s\" already exists, skipping", diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index 00869b1211..5bf1c8c393 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -189,6 +189,10 @@ CreateStatistics(CreateStatsStmt *stmt) { if (stmt->if_not_exists) { + /* + * Since stats objects aren't members of extensions (see comments + * below), no need for checkMembershipInCurrentExtension here. + */ ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), errmsg("statistics object \"%s\" already exists, skipping", diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index 0b070e3c69..98c3264563 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -202,7 +202,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace, CommandCounterIncrement(); /* - * Finally update the view options. + * Update the view's options. * * The new options list replaces the existing options list, even if * it's empty. @@ -215,8 +215,22 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace, /* EventTriggerAlterTableStart called by ProcessUtilitySlow */ AlterTableInternal(viewOid, atcmds, true); + /* + * There is very little to do here to update the view's dependencies. + * Most view-level dependency relationships, such as those on the + * owner, schema, and associated composite type, aren't changing. + * Because we don't allow changing type or collation of an existing + * view column, those dependencies of the existing columns don't + * change either, while the AT_AddColumnToView machinery took care of + * adding such dependencies for new view columns. The dependencies of + * the view's query could have changed arbitrarily, but that was dealt + * with inside StoreViewQuery. What remains is only to check that + * view replacement is allowed when we're creating an extension. + */ ObjectAddressSet(address, RelationRelationId, viewOid); + recordDependencyOnCurrentExtension(&address, true); + /* * Seems okay, so return the OID of the pre-existing view. */ diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 16d43c82a9..faefa49e45 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -244,6 +244,16 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) */ if (stmt->if_not_exists && OidIsValid(existing_relid)) { + /* + * If we are in an extension script, insist that the pre-existing + * object be a member of the extension, to avoid security risks. + */ + ObjectAddress address; + + ObjectAddressSet(address, RelationRelationId, existing_relid); + checkMembershipInCurrentExtension(&address); + + /* OK to skip */ ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_TABLE), errmsg("relation \"%s\" already exists, skipping", diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h index 3b57044e51..5983008015 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -225,6 +225,8 @@ extern void recordMultipleDependencies(const ObjectAddress *depender, extern void recordDependencyOnCurrentExtension(const ObjectAddress *object, bool isReplace); +extern void checkMembershipInCurrentExtension(const ObjectAddress *object); + extern long deleteDependencyRecordsFor(Oid classId, Oid objectId, bool skipExtensionDeps); diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile index 77ee4d5d9e..d210d4196f 100644 --- a/src/test/modules/test_extensions/Makefile +++ b/src/test/modules/test_extensions/Makefile @@ -4,15 +4,17 @@ MODULE = test_extensions 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_cyclic1 test_ext_cyclic2 \ - test_ext_evttrig + test_ext7 test_ext8 test_ext_cine test_ext_cor \ + test_ext_cyclic1 test_ext_cyclic2 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_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_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 -REGRESS = test_extensions test_extdepend + +REGRESS = test_extensions #test_extdepend TODO: enable this test after https://github.com/greenplum-db/gpdb/issues/14532 ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out index 30ae621d05..69ef3077a4 100644 --- a/src/test/modules/test_extensions/expected/test_extensions.out +++ b/src/test/modules/test_extensions/expected/test_extensions.out @@ -148,14 +148,167 @@ SELECT create_extension_with_temp_schema(); (1 row) PREPARE TRANSACTION 'twophase_extension'; -ERROR: cannot PREPARE a transaction that has operated on temporary objects +ERROR: PREPARE TRANSACTION is not yet supported in Cloudberry Database +ROLLBACK; -- Clean up DROP TABLE test_ext4_tab; DROP FUNCTION create_extension_with_temp_schema(); RESET client_min_messages; \unset SHOW_CONTEXT --- Test case of an event trigger run in an extension upgrade script. --- See: https://postgr.es/m/20200902193715.6e0269d4@firost -CREATE EXTENSION test_ext_evttrig; -ALTER EXTENSION test_ext_evttrig UPDATE TO '2.0'; -DROP EXTENSION test_ext_evttrig; +-- It's generally bad style to use CREATE OR REPLACE unnecessarily. +-- Test what happens if an extension does it anyway. +-- Replacing a shell type or operator is sort of like CREATE OR REPLACE; +-- check that too. +CREATE FUNCTION ext_cor_func() RETURNS text + AS $$ SELECT 'ext_cor_func: original'::text $$ LANGUAGE sql; +CREATE EXTENSION test_ext_cor; -- fail +ERROR: function ext_cor_func() is not a member of extension "test_ext_cor" +DETAIL: An extension is not allowed to replace an object that it does not own. +SELECT ext_cor_func(); + ext_cor_func +------------------------ + ext_cor_func: original +(1 row) + +DROP FUNCTION ext_cor_func(); +CREATE VIEW ext_cor_view AS + SELECT 'ext_cor_view: original'::text AS col; +CREATE EXTENSION test_ext_cor; -- fail +ERROR: view ext_cor_view is not a member of extension "test_ext_cor" +DETAIL: An extension is not allowed to replace an object that it does not own. +SELECT ext_cor_func(); +ERROR: function ext_cor_func() does not exist +LINE 1: SELECT ext_cor_func(); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +SELECT * FROM ext_cor_view; + col +------------------------ + ext_cor_view: original +(1 row) + +DROP VIEW ext_cor_view; +CREATE TYPE test_ext_type; +CREATE EXTENSION test_ext_cor; -- fail +ERROR: type test_ext_type is not a member of extension "test_ext_cor" +DETAIL: An extension is not allowed to replace an object that it does not own. +DROP TYPE test_ext_type; +-- this makes a shell "point <<@@ polygon" operator too +CREATE OPERATOR @@>> ( PROCEDURE = poly_contain_pt, + LEFTARG = polygon, RIGHTARG = point, + COMMUTATOR = <<@@ ); +CREATE EXTENSION test_ext_cor; -- fail +ERROR: operator <<@@(point,polygon) is not a member of extension "test_ext_cor" +DETAIL: An extension is not allowed to replace an object that it does not own. +DROP OPERATOR <<@@ (point, polygon); +CREATE EXTENSION test_ext_cor; -- now it should work +SELECT ext_cor_func(); + ext_cor_func +------------------------------ + ext_cor_func: from extension +(1 row) + +SELECT * FROM ext_cor_view; + col +------------------------------ + ext_cor_view: from extension +(1 row) + +SELECT 'x'::test_ext_type; + test_ext_type +--------------- + x +(1 row) + +SELECT point(0,0) <<@@ polygon(circle(point(0,0),1)); + ?column? +---------- + t +(1 row) + +\dx+ test_ext_cor +Objects in extension "test_ext_cor" + Object description +------------------------------ + function ext_cor_func() + operator <<@@(point,polygon) + type test_ext_type + view ext_cor_view +(4 rows) + +-- +-- CREATE IF NOT EXISTS is an entirely unsound thing for an extension +-- to be doing, but let's at least plug the major security hole in it. +-- +CREATE COLLATION ext_cine_coll + ( LC_COLLATE = "C", LC_CTYPE = "C" ); +CREATE EXTENSION test_ext_cine; -- fail +ERROR: collation ext_cine_coll is not a member of extension "test_ext_cine" +DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns. +DROP COLLATION ext_cine_coll; +CREATE MATERIALIZED VIEW ext_cine_mv AS SELECT 11 AS f1; +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause. Creating a NULL policy entry. +CREATE EXTENSION test_ext_cine; -- fail +ERROR: materialized view ext_cine_mv is not a member of extension "test_ext_cine" +DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns. +DROP MATERIALIZED VIEW ext_cine_mv; +CREATE FOREIGN DATA WRAPPER dummy; +CREATE SERVER ext_cine_srv FOREIGN DATA WRAPPER dummy; +CREATE EXTENSION test_ext_cine; -- fail +ERROR: server ext_cine_srv is not a member of extension "test_ext_cine" +DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns. +DROP SERVER ext_cine_srv; +CREATE SCHEMA ext_cine_schema; +CREATE EXTENSION test_ext_cine; -- fail +ERROR: schema ext_cine_schema is not a member of extension "test_ext_cine" +DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns. +DROP SCHEMA ext_cine_schema; +CREATE SEQUENCE ext_cine_seq; +CREATE EXTENSION test_ext_cine; -- fail +ERROR: sequence ext_cine_seq is not a member of extension "test_ext_cine" +DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns. +DROP SEQUENCE ext_cine_seq; +CREATE TABLE ext_cine_tab1 (x int); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'x' as the Cloudberry Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +CREATE EXTENSION test_ext_cine; -- fail +ERROR: table ext_cine_tab1 is not a member of extension "test_ext_cine" +DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns. +DROP TABLE ext_cine_tab1; +CREATE TABLE ext_cine_tab2 AS SELECT 42 AS y; +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause. Creating a NULL policy entry. +CREATE EXTENSION test_ext_cine; -- fail +ERROR: table ext_cine_tab2 is not a member of extension "test_ext_cine" +DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns. +DROP TABLE ext_cine_tab2; +CREATE EXTENSION test_ext_cine; +\dx+ test_ext_cine +Objects in extension "test_ext_cine" + Object description +----------------------------------- + collation ext_cine_coll + foreign-data wrapper ext_cine_fdw + materialized view ext_cine_mv + schema ext_cine_schema + sequence ext_cine_seq + server ext_cine_srv + table ext_cine_tab1 + table ext_cine_tab2 +(8 rows) + +ALTER EXTENSION test_ext_cine UPDATE TO '1.1'; +\dx+ test_ext_cine +Objects in extension "test_ext_cine" + Object description +----------------------------------- + collation ext_cine_coll + foreign-data wrapper ext_cine_fdw + materialized view ext_cine_mv + schema ext_cine_schema + sequence ext_cine_seq + server ext_cine_srv + table ext_cine_tab1 + table ext_cine_tab2 + table ext_cine_tab3 +(9 rows) + diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql index c16fd36da8..946c8f25a8 100644 --- a/src/test/modules/test_extensions/sql/test_extensions.sql +++ b/src/test/modules/test_extensions/sql/test_extensions.sql @@ -88,14 +88,119 @@ CREATE OR REPLACE FUNCTION create_extension_with_temp_schema() BEGIN; SELECT create_extension_with_temp_schema(); PREPARE TRANSACTION 'twophase_extension'; +ROLLBACK; -- Clean up DROP TABLE test_ext4_tab; DROP FUNCTION create_extension_with_temp_schema(); RESET client_min_messages; \unset SHOW_CONTEXT --- Test case of an event trigger run in an extension upgrade script. --- See: https://postgr.es/m/20200902193715.6e0269d4@firost -CREATE EXTENSION test_ext_evttrig; -ALTER EXTENSION test_ext_evttrig UPDATE TO '2.0'; -DROP EXTENSION test_ext_evttrig; +-- It's generally bad style to use CREATE OR REPLACE unnecessarily. +-- Test what happens if an extension does it anyway. +-- Replacing a shell type or operator is sort of like CREATE OR REPLACE; +-- check that too. + +CREATE FUNCTION ext_cor_func() RETURNS text + AS $$ SELECT 'ext_cor_func: original'::text $$ LANGUAGE sql; + +CREATE EXTENSION test_ext_cor; -- fail + +SELECT ext_cor_func(); + +DROP FUNCTION ext_cor_func(); + +CREATE VIEW ext_cor_view AS + SELECT 'ext_cor_view: original'::text AS col; + +CREATE EXTENSION test_ext_cor; -- fail + +SELECT ext_cor_func(); + +SELECT * FROM ext_cor_view; + +DROP VIEW ext_cor_view; + +CREATE TYPE test_ext_type; + +CREATE EXTENSION test_ext_cor; -- fail + +DROP TYPE test_ext_type; + +-- this makes a shell "point <<@@ polygon" operator too +CREATE OPERATOR @@>> ( PROCEDURE = poly_contain_pt, + LEFTARG = polygon, RIGHTARG = point, + COMMUTATOR = <<@@ ); + +CREATE EXTENSION test_ext_cor; -- fail + +DROP OPERATOR <<@@ (point, polygon); + +CREATE EXTENSION test_ext_cor; -- now it should work + +SELECT ext_cor_func(); + +SELECT * FROM ext_cor_view; + +SELECT 'x'::test_ext_type; + +SELECT point(0,0) <<@@ polygon(circle(point(0,0),1)); + +\dx+ test_ext_cor + +-- +-- CREATE IF NOT EXISTS is an entirely unsound thing for an extension +-- to be doing, but let's at least plug the major security hole in it. +-- + +CREATE COLLATION ext_cine_coll + ( LC_COLLATE = "C", LC_CTYPE = "C" ); + +CREATE EXTENSION test_ext_cine; -- fail + +DROP COLLATION ext_cine_coll; + +CREATE MATERIALIZED VIEW ext_cine_mv AS SELECT 11 AS f1; + +CREATE EXTENSION test_ext_cine; -- fail + +DROP MATERIALIZED VIEW ext_cine_mv; + +CREATE FOREIGN DATA WRAPPER dummy; + +CREATE SERVER ext_cine_srv FOREIGN DATA WRAPPER dummy; + +CREATE EXTENSION test_ext_cine; -- fail + +DROP SERVER ext_cine_srv; + +CREATE SCHEMA ext_cine_schema; + +CREATE EXTENSION test_ext_cine; -- fail + +DROP SCHEMA ext_cine_schema; + +CREATE SEQUENCE ext_cine_seq; + +CREATE EXTENSION test_ext_cine; -- fail + +DROP SEQUENCE ext_cine_seq; + +CREATE TABLE ext_cine_tab1 (x int); + +CREATE EXTENSION test_ext_cine; -- fail + +DROP TABLE ext_cine_tab1; + +CREATE TABLE ext_cine_tab2 AS SELECT 42 AS y; + +CREATE EXTENSION test_ext_cine; -- fail + +DROP TABLE ext_cine_tab2; + +CREATE EXTENSION test_ext_cine; + +\dx+ test_ext_cine + +ALTER EXTENSION test_ext_cine UPDATE TO '1.1'; + +\dx+ test_ext_cine diff --git a/src/test/modules/test_extensions/test_ext_cine--1.0--1.1.sql b/src/test/modules/test_extensions/test_ext_cine--1.0--1.1.sql new file mode 100644 index 0000000000..6dadfd2416 --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_cine--1.0--1.1.sql @@ -0,0 +1,26 @@ +/* src/test/modules/test_extensions/test_ext_cine--1.0--1.1.sql */ +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION test_ext_cine UPDATE TO '1.1'" to load this file. \quit + +-- +-- These are the same commands as in the 1.0 script; we expect them +-- to do nothing. +-- + +CREATE COLLATION IF NOT EXISTS ext_cine_coll + ( LC_COLLATE = "POSIX", LC_CTYPE = "POSIX" ); + +CREATE MATERIALIZED VIEW IF NOT EXISTS ext_cine_mv AS SELECT 42 AS f1; + +CREATE SERVER IF NOT EXISTS ext_cine_srv FOREIGN DATA WRAPPER ext_cine_fdw; + +CREATE SCHEMA IF NOT EXISTS ext_cine_schema; + +CREATE SEQUENCE IF NOT EXISTS ext_cine_seq; + +CREATE TABLE IF NOT EXISTS ext_cine_tab1 (x int); + +CREATE TABLE IF NOT EXISTS ext_cine_tab2 AS SELECT 42 AS y; + +-- just to verify the script ran +CREATE TABLE ext_cine_tab3 (z int); diff --git a/src/test/modules/test_extensions/test_ext_cine--1.0.sql b/src/test/modules/test_extensions/test_ext_cine--1.0.sql new file mode 100644 index 0000000000..01408ffeb0 --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_cine--1.0.sql @@ -0,0 +1,25 @@ +/* src/test/modules/test_extensions/test_ext_cine--1.0.sql */ +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION test_ext_cine" to load this file. \quit + +-- +-- CREATE IF NOT EXISTS is an entirely unsound thing for an extension +-- to be doing, but let's at least plug the major security hole in it. +-- + +CREATE COLLATION IF NOT EXISTS ext_cine_coll + ( LC_COLLATE = "POSIX", LC_CTYPE = "POSIX" ); + +CREATE MATERIALIZED VIEW IF NOT EXISTS ext_cine_mv AS SELECT 42 AS f1; + +CREATE FOREIGN DATA WRAPPER ext_cine_fdw; + +CREATE SERVER IF NOT EXISTS ext_cine_srv FOREIGN DATA WRAPPER ext_cine_fdw; + +CREATE SCHEMA IF NOT EXISTS ext_cine_schema; + +CREATE SEQUENCE IF NOT EXISTS ext_cine_seq; + +CREATE TABLE IF NOT EXISTS ext_cine_tab1 (x int); + +CREATE TABLE IF NOT EXISTS ext_cine_tab2 AS SELECT 42 AS y; diff --git a/src/test/modules/test_extensions/test_ext_cine.control b/src/test/modules/test_extensions/test_ext_cine.control new file mode 100644 index 0000000000..ced713bace --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_cine.control @@ -0,0 +1,3 @@ +comment = 'Test extension using CREATE IF NOT EXISTS' +default_version = '1.0' +relocatable = true diff --git a/src/test/modules/test_extensions/test_ext_cor--1.0.sql b/src/test/modules/test_extensions/test_ext_cor--1.0.sql new file mode 100644 index 0000000000..2e8d89cecc --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_cor--1.0.sql @@ -0,0 +1,20 @@ +/* src/test/modules/test_extensions/test_ext_cor--1.0.sql */ +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION test_ext_cor" to load this file. \quit + +-- It's generally bad style to use CREATE OR REPLACE unnecessarily. +-- Test what happens if an extension does it anyway. + +CREATE OR REPLACE FUNCTION ext_cor_func() RETURNS text + AS $$ SELECT 'ext_cor_func: from extension'::text $$ LANGUAGE sql; + +CREATE OR REPLACE VIEW ext_cor_view AS + SELECT 'ext_cor_view: from extension'::text AS col; + +-- These are for testing replacement of a shell type/operator, which works +-- enough like an implicit OR REPLACE to be important to check. + +CREATE TYPE test_ext_type AS ENUM('x', 'y'); + +CREATE OPERATOR <<@@ ( PROCEDURE = pt_contained_poly, + LEFTARG = point, RIGHTARG = polygon ); diff --git a/src/test/modules/test_extensions/test_ext_cor.control b/src/test/modules/test_extensions/test_ext_cor.control new file mode 100644 index 0000000000..0e972e5d1b --- /dev/null +++ b/src/test/modules/test_extensions/test_ext_cor.control @@ -0,0 +1,3 @@ +comment = 'Test extension using CREATE OR REPLACE' +default_version = '1.0' +relocatable = true --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
