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.

diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 072f530d988e945e26e7395814a5e44a50168f82..d7333b00d3fb1ce0523957d615589fb45f115825 100644 (file)
--- 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 0bb9743ea3622040dc03112bb8484d5da7ea3900..67d266cd7455dbfbb8c0d94ec4b1e9dd3623cd7c 100644 (file)
--- 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 e79ce57f6ebfa0bc3b5873ae160f9e5ce2042b70..facef0f335fa2599978202ca1c84914e684cbfa4 100644 (file)
--- 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 (file)
index 0000000..bf236d6
--- /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 ce8366a1397fb0b6ff3f7f2fede7a79356103287..449ed53f8bb517e0401f37356b9d920a201c79ca 100644 (file)
--- 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 a7e1988682fe12dcaf63191d8abe9cf7bfafb1b0..dfabec61752259fc20867a442e026f36c772bfe8 100644 (file)
--- 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);