From 0dab5ef39b3d9d86e45bbbb2f6ea60b4f5517d9a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 31 Dec 2015 17:37:31 -0500 Subject: [PATCH] Fix ALTER OPERATOR to update dependencies properly. Fix an oversight in commit 321eed5f0f7563a0: replacing an operator's selectivity functions needs to result in a corresponding update in pg_depend. We have a function that can handle that, but it was not called by AlterOperator(). To fix this without enlarging pg_operator.h's #include list beyond what clients can safely include, split off the function definitions into a new file pg_operator_fn.h, similarly to what we've done for some other catalog header files. It's not entirely clear whether any client-side code needs to include pg_operator.h, but it seems prudent to assume that there is some such code somewhere. --- src/backend/catalog/pg_operator.c | 35 ++++++---- src/backend/commands/operatorcmds.c | 5 +- src/include/catalog/pg_operator.h | 17 ----- src/include/catalog/pg_operator_fn.h | 34 +++++++++ src/test/regress/expected/alter_operator.out | 73 ++++++++++++++++++-- src/test/regress/sql/alter_operator.sql | 41 ++++++++++- 6 files changed, 163 insertions(+), 42 deletions(-) create mode 100644 src/include/catalog/pg_operator_fn.h diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index 072f530d98..d7333b00d3 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -26,6 +26,7 @@ #include "catalog/objectaccess.h" #include "catalog/pg_namespace.h" #include "catalog/pg_operator.h" +#include "catalog/pg_operator_fn.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "miscadmin.h" @@ -61,8 +62,6 @@ static Oid get_other_operator(List *otherOp, Oid leftTypeId, Oid rightTypeId, bool isCommutator); -static ObjectAddress makeOperatorDependencies(HeapTuple tuple); - /* * Check whether a proposed operator name is legal @@ -270,7 +269,7 @@ OperatorShellMake(const char *operatorName, CatalogUpdateIndexes(pg_operator_desc, tup); /* Add dependencies for the entry */ - makeOperatorDependencies(tup); + makeOperatorDependencies(tup, false); heap_freetuple(tup); @@ -340,6 +339,7 @@ OperatorCreate(const char *operatorName, { Relation pg_operator_desc; HeapTuple tup; + bool isUpdate; bool nulls[Natts_pg_operator]; bool replaces[Natts_pg_operator]; Datum values[Natts_pg_operator]; @@ -350,7 +350,6 @@ OperatorCreate(const char *operatorName, negatorId; bool selfCommutator = false; NameData oname; - TupleDesc tupDesc; int i; ObjectAddress address; @@ -515,6 +514,8 @@ OperatorCreate(const char *operatorName, */ if (operatorObjectId) { + isUpdate = true; + tup = SearchSysCacheCopy1(OPEROID, ObjectIdGetDatum(operatorObjectId)); if (!HeapTupleIsValid(tup)) @@ -531,8 +532,10 @@ OperatorCreate(const char *operatorName, } else { - tupDesc = pg_operator_desc->rd_att; - tup = heap_form_tuple(tupDesc, values, nulls); + isUpdate = false; + + tup = heap_form_tuple(RelationGetDescr(pg_operator_desc), + values, nulls); operatorObjectId = simple_heap_insert(pg_operator_desc, tup); } @@ -541,7 +544,7 @@ OperatorCreate(const char *operatorName, CatalogUpdateIndexes(pg_operator_desc, tup); /* Add dependencies for the entry */ - address = makeOperatorDependencies(tup); + address = makeOperatorDependencies(tup, isUpdate); /* Post creation hook for new operator */ InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0); @@ -759,14 +762,15 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId) } /* - * Create dependencies for a new operator (either a freshly inserted - * complete operator, a new shell operator, or a just-updated shell). + * Create dependencies for an operator (either a freshly inserted + * complete operator, a new shell operator, a just-updated shell, + * or an operator that's being modified by ALTER OPERATOR). * * NB: the OidIsValid tests in this routine are necessary, in case * the given operator is a shell. */ -static ObjectAddress -makeOperatorDependencies(HeapTuple tuple) +ObjectAddress +makeOperatorDependencies(HeapTuple tuple, bool isUpdate) { Form_pg_operator oper = (Form_pg_operator) GETSTRUCT(tuple); ObjectAddress myself, @@ -777,11 +781,14 @@ makeOperatorDependencies(HeapTuple tuple) myself.objectSubId = 0; /* - * In case we are updating a shell, delete any existing entries, except + * If we are updating the operator, delete any existing entries, except * for extension membership which should remain the same. */ - deleteDependencyRecordsFor(myself.classId, myself.objectId, true); - deleteSharedDependencyRecordsFor(myself.classId, myself.objectId, 0); + if (isUpdate) + { + deleteDependencyRecordsFor(myself.classId, myself.objectId, true); + deleteSharedDependencyRecordsFor(myself.classId, myself.objectId, 0); + } /* Dependency on namespace */ if (OidIsValid(oper->oprnamespace)) diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c index 0bb9743ea3..67d266cd74 100644 --- a/src/backend/commands/operatorcmds.c +++ b/src/backend/commands/operatorcmds.c @@ -40,6 +40,7 @@ #include "catalog/indexing.h" #include "catalog/objectaccess.h" #include "catalog/pg_operator.h" +#include "catalog/pg_operator_fn.h" #include "catalog/pg_type.h" #include "commands/alter.h" #include "commands/defrem.h" @@ -500,9 +501,9 @@ AlterOperator(AlterOperatorStmt *stmt) simple_heap_update(catalog, &tup->t_self, tup); CatalogUpdateIndexes(catalog, tup); - InvokeObjectPostAlterHook(OperatorRelationId, oprId, 0); + address = makeOperatorDependencies(tup, true); - ObjectAddressSet(address, OperatorRelationId, oprId); + InvokeObjectPostAlterHook(OperatorRelationId, oprId, 0); heap_close(catalog, NoLock); diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h index e79ce57f6e..facef0f335 100644 --- a/src/include/catalog/pg_operator.h +++ b/src/include/catalog/pg_operator.h @@ -23,8 +23,6 @@ #define PG_OPERATOR_H #include "catalog/genbki.h" -#include "catalog/objectaddress.h" -#include "nodes/pg_list.h" /* ---------------- * pg_operator definition. cpp turns this into @@ -1826,19 +1824,4 @@ DESCR("delete array element"); DATA(insert OID = 3287 ( "#-" PGNSP PGUID b f f 3802 1009 3802 0 0 jsonb_delete_path - - )); DESCR("delete path"); -/* - * function prototypes - */ -extern ObjectAddress OperatorCreate(const char *operatorName, - Oid operatorNamespace, - Oid leftTypeId, - Oid rightTypeId, - Oid procedureId, - List *commutatorName, - List *negatorName, - Oid restrictionId, - Oid joinId, - bool canMerge, - bool canHash); - #endif /* PG_OPERATOR_H */ diff --git a/src/include/catalog/pg_operator_fn.h b/src/include/catalog/pg_operator_fn.h new file mode 100644 index 0000000000..bf236d6d74 --- /dev/null +++ b/src/include/catalog/pg_operator_fn.h @@ -0,0 +1,34 @@ +/*------------------------------------------------------------------------- + * + * pg_operator_fn.h +* prototypes for functions in catalog/pg_operator.c + * + * + * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/catalog/pg_operator_fn.h + * + *------------------------------------------------------------------------- + */ +#ifndef PG_OPERATOR_FN_H +#define PG_OPERATOR_FN_H + +#include "catalog/objectaddress.h" +#include "nodes/pg_list.h" + +extern ObjectAddress OperatorCreate(const char *operatorName, + Oid operatorNamespace, + Oid leftTypeId, + Oid rightTypeId, + Oid procedureId, + List *commutatorName, + List *negatorName, + Oid restrictionId, + Oid joinId, + bool canMerge, + bool canHash); + +extern ObjectAddress makeOperatorDependencies(HeapTuple tuple, bool isUpdate); + +#endif /* PG_OPERATOR_FN_H */ diff --git a/src/test/regress/expected/alter_operator.out b/src/test/regress/expected/alter_operator.out index ce8366a139..449ed53f8b 100644 --- a/src/test/regress/expected/alter_operator.out +++ b/src/test/regress/expected/alter_operator.out @@ -1,15 +1,29 @@ -CREATE OR REPLACE FUNCTION alter_op_test_fn(boolean, boolean) +CREATE FUNCTION alter_op_test_fn(boolean, boolean) RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE; +CREATE FUNCTION customcontsel(internal, oid, internal, integer) +RETURNS float8 AS 'contsel' LANGUAGE internal STABLE STRICT; CREATE OPERATOR === ( LEFTARG = boolean, RIGHTARG = boolean, PROCEDURE = alter_op_test_fn, COMMUTATOR = ===, NEGATOR = !==, - RESTRICT = contsel, + RESTRICT = customcontsel, JOIN = contjoinsel, HASHES, MERGES ); +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + ref | deptype +-------------------------------------------------------+--------- + function alter_op_test_fn(boolean,boolean) | n + function customcontsel(internal,oid,internal,integer) | n + schema public | n +(3 rows) + -- -- Reset and set params -- @@ -22,6 +36,17 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' - | - (1 row) +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + ref | deptype +--------------------------------------------+--------- + function alter_op_test_fn(boolean,boolean) | n + schema public | n +(2 rows) + ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = contsel); ALTER OPERATOR === (boolean, boolean) SET (JOIN = contjoinsel); SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' @@ -31,6 +56,17 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' contsel | contjoinsel (1 row) +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + ref | deptype +--------------------------------------------+--------- + function alter_op_test_fn(boolean,boolean) | n + schema public | n +(2 rows) + ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE, JOIN = NONE); SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; @@ -39,14 +75,37 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' - | - (1 row) -ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = contsel, JOIN = contjoinsel); +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + ref | deptype +--------------------------------------------+--------- + function alter_op_test_fn(boolean,boolean) | n + schema public | n +(2 rows) + +ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = customcontsel, JOIN = contjoinsel); SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; - oprrest | oprjoin ----------+------------- - contsel | contjoinsel + oprrest | oprjoin +---------------+------------- + customcontsel | contjoinsel (1 row) +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + ref | deptype +-------------------------------------------------------+--------- + function alter_op_test_fn(boolean,boolean) | n + function customcontsel(internal,oid,internal,integer) | n + schema public | n +(3 rows) + -- -- Test invalid options. -- @@ -73,3 +132,5 @@ ERROR: must be owner of operator === RESET SESSION AUTHORIZATION; DROP USER regtest_alter_user; DROP OPERATOR === (boolean, boolean); +DROP FUNCTION customcontsel(internal, oid, internal, integer); +DROP FUNCTION alter_op_test_fn(boolean, boolean); diff --git a/src/test/regress/sql/alter_operator.sql b/src/test/regress/sql/alter_operator.sql index a7e1988682..dfabec6175 100644 --- a/src/test/regress/sql/alter_operator.sql +++ b/src/test/regress/sql/alter_operator.sql @@ -1,17 +1,26 @@ -CREATE OR REPLACE FUNCTION alter_op_test_fn(boolean, boolean) +CREATE FUNCTION alter_op_test_fn(boolean, boolean) RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE; +CREATE FUNCTION customcontsel(internal, oid, internal, integer) +RETURNS float8 AS 'contsel' LANGUAGE internal STABLE STRICT; + CREATE OPERATOR === ( LEFTARG = boolean, RIGHTARG = boolean, PROCEDURE = alter_op_test_fn, COMMUTATOR = ===, NEGATOR = !==, - RESTRICT = contsel, + RESTRICT = customcontsel, JOIN = contjoinsel, HASHES, MERGES ); +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + -- -- Reset and set params -- @@ -22,22 +31,46 @@ ALTER OPERATOR === (boolean, boolean) SET (JOIN = NONE); SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = contsel); ALTER OPERATOR === (boolean, boolean) SET (JOIN = contjoinsel); SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE, JOIN = NONE); SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; -ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = contsel, JOIN = contjoinsel); +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + +ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = customcontsel, JOIN = contjoinsel); SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; +SELECT pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype +FROM pg_depend +WHERE classid = 'pg_operator'::regclass AND + objid = '===(bool,bool)'::regoperator +ORDER BY 1; + -- -- Test invalid options. -- @@ -60,3 +93,5 @@ ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE); RESET SESSION AUTHORIZATION; DROP USER regtest_alter_user; DROP OPERATOR === (boolean, boolean); +DROP FUNCTION customcontsel(internal, oid, internal, integer); +DROP FUNCTION alter_op_test_fn(boolean, boolean);