Commit 0dab5ef3 authored by Tom Lane's avatar Tom Lane

Fix ALTER OPERATOR to update dependencies properly.

Fix an oversight in commit 321eed5f: 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.
parent e5d06f2b
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "catalog/objectaccess.h" #include "catalog/objectaccess.h"
#include "catalog/pg_namespace.h" #include "catalog/pg_namespace.h"
#include "catalog/pg_operator.h" #include "catalog/pg_operator.h"
#include "catalog/pg_operator_fn.h"
#include "catalog/pg_proc.h" #include "catalog/pg_proc.h"
#include "catalog/pg_type.h" #include "catalog/pg_type.h"
#include "miscadmin.h" #include "miscadmin.h"
...@@ -61,8 +62,6 @@ static Oid get_other_operator(List *otherOp, ...@@ -61,8 +62,6 @@ static Oid get_other_operator(List *otherOp,
Oid leftTypeId, Oid rightTypeId, Oid leftTypeId, Oid rightTypeId,
bool isCommutator); bool isCommutator);
static ObjectAddress makeOperatorDependencies(HeapTuple tuple);
/* /*
* Check whether a proposed operator name is legal * Check whether a proposed operator name is legal
...@@ -270,7 +269,7 @@ OperatorShellMake(const char *operatorName, ...@@ -270,7 +269,7 @@ OperatorShellMake(const char *operatorName,
CatalogUpdateIndexes(pg_operator_desc, tup); CatalogUpdateIndexes(pg_operator_desc, tup);
/* Add dependencies for the entry */ /* Add dependencies for the entry */
makeOperatorDependencies(tup); makeOperatorDependencies(tup, false);
heap_freetuple(tup); heap_freetuple(tup);
...@@ -340,6 +339,7 @@ OperatorCreate(const char *operatorName, ...@@ -340,6 +339,7 @@ OperatorCreate(const char *operatorName,
{ {
Relation pg_operator_desc; Relation pg_operator_desc;
HeapTuple tup; HeapTuple tup;
bool isUpdate;
bool nulls[Natts_pg_operator]; bool nulls[Natts_pg_operator];
bool replaces[Natts_pg_operator]; bool replaces[Natts_pg_operator];
Datum values[Natts_pg_operator]; Datum values[Natts_pg_operator];
...@@ -350,7 +350,6 @@ OperatorCreate(const char *operatorName, ...@@ -350,7 +350,6 @@ OperatorCreate(const char *operatorName,
negatorId; negatorId;
bool selfCommutator = false; bool selfCommutator = false;
NameData oname; NameData oname;
TupleDesc tupDesc;
int i; int i;
ObjectAddress address; ObjectAddress address;
...@@ -515,6 +514,8 @@ OperatorCreate(const char *operatorName, ...@@ -515,6 +514,8 @@ OperatorCreate(const char *operatorName,
*/ */
if (operatorObjectId) if (operatorObjectId)
{ {
isUpdate = true;
tup = SearchSysCacheCopy1(OPEROID, tup = SearchSysCacheCopy1(OPEROID,
ObjectIdGetDatum(operatorObjectId)); ObjectIdGetDatum(operatorObjectId));
if (!HeapTupleIsValid(tup)) if (!HeapTupleIsValid(tup))
...@@ -531,8 +532,10 @@ OperatorCreate(const char *operatorName, ...@@ -531,8 +532,10 @@ OperatorCreate(const char *operatorName,
} }
else else
{ {
tupDesc = pg_operator_desc->rd_att; isUpdate = false;
tup = heap_form_tuple(tupDesc, values, nulls);
tup = heap_form_tuple(RelationGetDescr(pg_operator_desc),
values, nulls);
operatorObjectId = simple_heap_insert(pg_operator_desc, tup); operatorObjectId = simple_heap_insert(pg_operator_desc, tup);
} }
...@@ -541,7 +544,7 @@ OperatorCreate(const char *operatorName, ...@@ -541,7 +544,7 @@ OperatorCreate(const char *operatorName,
CatalogUpdateIndexes(pg_operator_desc, tup); CatalogUpdateIndexes(pg_operator_desc, tup);
/* Add dependencies for the entry */ /* Add dependencies for the entry */
address = makeOperatorDependencies(tup); address = makeOperatorDependencies(tup, isUpdate);
/* Post creation hook for new operator */ /* Post creation hook for new operator */
InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0); InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0);
...@@ -759,14 +762,15 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId) ...@@ -759,14 +762,15 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId)
} }
/* /*
* Create dependencies for a new operator (either a freshly inserted * Create dependencies for an operator (either a freshly inserted
* complete operator, a new shell operator, or a just-updated shell). * 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 * NB: the OidIsValid tests in this routine are necessary, in case
* the given operator is a shell. * the given operator is a shell.
*/ */
static ObjectAddress ObjectAddress
makeOperatorDependencies(HeapTuple tuple) makeOperatorDependencies(HeapTuple tuple, bool isUpdate)
{ {
Form_pg_operator oper = (Form_pg_operator) GETSTRUCT(tuple); Form_pg_operator oper = (Form_pg_operator) GETSTRUCT(tuple);
ObjectAddress myself, ObjectAddress myself,
...@@ -777,11 +781,14 @@ makeOperatorDependencies(HeapTuple tuple) ...@@ -777,11 +781,14 @@ makeOperatorDependencies(HeapTuple tuple)
myself.objectSubId = 0; 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. * for extension membership which should remain the same.
*/ */
if (isUpdate)
{
deleteDependencyRecordsFor(myself.classId, myself.objectId, true); deleteDependencyRecordsFor(myself.classId, myself.objectId, true);
deleteSharedDependencyRecordsFor(myself.classId, myself.objectId, 0); deleteSharedDependencyRecordsFor(myself.classId, myself.objectId, 0);
}
/* Dependency on namespace */ /* Dependency on namespace */
if (OidIsValid(oper->oprnamespace)) if (OidIsValid(oper->oprnamespace))
......
...@@ -40,6 +40,7 @@ ...@@ -40,6 +40,7 @@
#include "catalog/indexing.h" #include "catalog/indexing.h"
#include "catalog/objectaccess.h" #include "catalog/objectaccess.h"
#include "catalog/pg_operator.h" #include "catalog/pg_operator.h"
#include "catalog/pg_operator_fn.h"
#include "catalog/pg_type.h" #include "catalog/pg_type.h"
#include "commands/alter.h" #include "commands/alter.h"
#include "commands/defrem.h" #include "commands/defrem.h"
...@@ -500,9 +501,9 @@ AlterOperator(AlterOperatorStmt *stmt) ...@@ -500,9 +501,9 @@ AlterOperator(AlterOperatorStmt *stmt)
simple_heap_update(catalog, &tup->t_self, tup); simple_heap_update(catalog, &tup->t_self, tup);
CatalogUpdateIndexes(catalog, tup); CatalogUpdateIndexes(catalog, tup);
InvokeObjectPostAlterHook(OperatorRelationId, oprId, 0); address = makeOperatorDependencies(tup, true);
ObjectAddressSet(address, OperatorRelationId, oprId); InvokeObjectPostAlterHook(OperatorRelationId, oprId, 0);
heap_close(catalog, NoLock); heap_close(catalog, NoLock);
......
...@@ -23,8 +23,6 @@ ...@@ -23,8 +23,6 @@
#define PG_OPERATOR_H #define PG_OPERATOR_H
#include "catalog/genbki.h" #include "catalog/genbki.h"
#include "catalog/objectaddress.h"
#include "nodes/pg_list.h"
/* ---------------- /* ----------------
* pg_operator definition. cpp turns this into * pg_operator definition. cpp turns this into
...@@ -1826,19 +1824,4 @@ DESCR("delete array element"); ...@@ -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 - - )); DATA(insert OID = 3287 ( "#-" PGNSP PGUID b f f 3802 1009 3802 0 0 jsonb_delete_path - - ));
DESCR("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 */ #endif /* PG_OPERATOR_H */
/*-------------------------------------------------------------------------
*
* 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 */
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; 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 === ( CREATE OPERATOR === (
LEFTARG = boolean, LEFTARG = boolean,
RIGHTARG = boolean, RIGHTARG = boolean,
PROCEDURE = alter_op_test_fn, PROCEDURE = alter_op_test_fn,
COMMUTATOR = ===, COMMUTATOR = ===,
NEGATOR = !==, NEGATOR = !==,
RESTRICT = contsel, RESTRICT = customcontsel,
JOIN = contjoinsel, JOIN = contjoinsel,
HASHES, MERGES 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 -- Reset and set params
-- --
...@@ -22,6 +36,17 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' ...@@ -22,6 +36,17 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
- | - - | -
(1 row) (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 (RESTRICT = contsel);
ALTER OPERATOR === (boolean, boolean) SET (JOIN = contjoinsel); ALTER OPERATOR === (boolean, boolean) SET (JOIN = contjoinsel);
SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
...@@ -31,6 +56,17 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' ...@@ -31,6 +56,17 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
contsel | contjoinsel contsel | contjoinsel
(1 row) (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); ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE, JOIN = NONE);
SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype;
...@@ -39,14 +75,37 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' ...@@ -39,14 +75,37 @@ SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
- | - - | -
(1 row) (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 = '===' SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype;
oprrest | oprjoin oprrest | oprjoin
---------+------------- ---------------+-------------
contsel | contjoinsel customcontsel | contjoinsel
(1 row) (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. -- Test invalid options.
-- --
...@@ -73,3 +132,5 @@ ERROR: must be owner of operator === ...@@ -73,3 +132,5 @@ ERROR: must be owner of operator ===
RESET SESSION AUTHORIZATION; RESET SESSION AUTHORIZATION;
DROP USER regtest_alter_user; DROP USER regtest_alter_user;
DROP OPERATOR === (boolean, boolean); DROP OPERATOR === (boolean, boolean);
DROP FUNCTION customcontsel(internal, oid, internal, integer);
DROP FUNCTION alter_op_test_fn(boolean, boolean);
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; 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 === ( CREATE OPERATOR === (
LEFTARG = boolean, LEFTARG = boolean,
RIGHTARG = boolean, RIGHTARG = boolean,
PROCEDURE = alter_op_test_fn, PROCEDURE = alter_op_test_fn,
COMMUTATOR = ===, COMMUTATOR = ===,
NEGATOR = !==, NEGATOR = !==,
RESTRICT = contsel, RESTRICT = customcontsel,
JOIN = contjoinsel, JOIN = contjoinsel,
HASHES, MERGES 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 -- Reset and set params
-- --
...@@ -22,22 +31,46 @@ ALTER OPERATOR === (boolean, boolean) SET (JOIN = NONE); ...@@ -22,22 +31,46 @@ ALTER OPERATOR === (boolean, boolean) SET (JOIN = NONE);
SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; 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 (RESTRICT = contsel);
ALTER OPERATOR === (boolean, boolean) SET (JOIN = contjoinsel); ALTER OPERATOR === (boolean, boolean) SET (JOIN = contjoinsel);
SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; 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); ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE, JOIN = NONE);
SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '===' SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; 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 = '===' SELECT oprrest, oprjoin FROM pg_operator WHERE oprname = '==='
AND oprleft = 'boolean'::regtype AND oprright = 'boolean'::regtype; 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. -- Test invalid options.
-- --
...@@ -60,3 +93,5 @@ ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE); ...@@ -60,3 +93,5 @@ ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE);
RESET SESSION AUTHORIZATION; RESET SESSION AUTHORIZATION;
DROP USER regtest_alter_user; DROP USER regtest_alter_user;
DROP OPERATOR === (boolean, boolean); DROP OPERATOR === (boolean, boolean);
DROP FUNCTION customcontsel(internal, oid, internal, integer);
DROP FUNCTION alter_op_test_fn(boolean, boolean);
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