Update leakproofness markings on some btree comparison functions.

Mark pg_lsn and oidvector comparison functions as leakproof.  Per
discussion, these clearly are leakproof so we might as well mark them so.

On the other hand, remove leakproof markings from name comparison
functions other than equal/not-equal.  Now that these depend on
varstr_cmp, they can't be considered leakproof if text comparison isn't.
(This was my error in commit 586b98fdf.)

While at it, add some opr_sanity queries to catch cases where related
functions do not have the same volatility and leakproof markings.
This would clearly be bogus for commutator or negator pairs.  In the
domain of btree comparison functions, we do have some exceptions,
because text equality is leakproof but inequality comparisons are not.
That's odd on first glance but is reasonable (for now anyway) given
the much greater complexity of the inequality code paths.

Discussion: https://postgr.es/m/20181231172551.GA206480@gust.leadboat.com

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 588a110093a6d85569462f059a7ba0ddd6f633a8..257eb1bd7725a343cfa9bd8369b088832ae66250 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     201812301
+#define CATALOG_VERSION_NO     201812311
 
 #endif

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6e1e1dfad71c64e55de4a52fe12537a9548d8eeb..9cf8e73852593601cda3d03b9e9e8b5fa71da52c 100644 (file)
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -677,44 +677,44 @@
   proname => 'nameeqtext', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'name text', prosrc => 'nameeqtext' },
 { oid => '241',
-  proname => 'namelttext', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name text', prosrc => 'namelttext' },
+  proname => 'namelttext', prorettype => 'bool', proargtypes => 'name text',
+  prosrc => 'namelttext' },
 { oid => '242',
-  proname => 'nameletext', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name text', prosrc => 'nameletext' },
+  proname => 'nameletext', prorettype => 'bool', proargtypes => 'name text',
+  prosrc => 'nameletext' },
 { oid => '243',
-  proname => 'namegetext', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name text', prosrc => 'namegetext' },
+  proname => 'namegetext', prorettype => 'bool', proargtypes => 'name text',
+  prosrc => 'namegetext' },
 { oid => '244',
-  proname => 'namegttext', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name text', prosrc => 'namegttext' },
+  proname => 'namegttext', prorettype => 'bool', proargtypes => 'name text',
+  prosrc => 'namegttext' },
 { oid => '245',
   proname => 'namenetext', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'name text', prosrc => 'namenetext' },
 { oid => '246', descr => 'less-equal-greater',
-  proname => 'btnametextcmp', proleakproof => 't', prorettype => 'int4',
-  proargtypes => 'name text', prosrc => 'btnametextcmp' },
+  proname => 'btnametextcmp', prorettype => 'int4', proargtypes => 'name text',
+  prosrc => 'btnametextcmp' },
 { oid => '247',
   proname => 'texteqname', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text name', prosrc => 'texteqname' },
 { oid => '248',
-  proname => 'textltname', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'text name', prosrc => 'textltname' },
+  proname => 'textltname', prorettype => 'bool', proargtypes => 'text name',
+  prosrc => 'textltname' },
 { oid => '249',
-  proname => 'textlename', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'text name', prosrc => 'textlename' },
+  proname => 'textlename', prorettype => 'bool', proargtypes => 'text name',
+  prosrc => 'textlename' },
 { oid => '250',
-  proname => 'textgename', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'text name', prosrc => 'textgename' },
+  proname => 'textgename', prorettype => 'bool', proargtypes => 'text name',
+  prosrc => 'textgename' },
 { oid => '251',
-  proname => 'textgtname', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'text name', prosrc => 'textgtname' },
+  proname => 'textgtname', prorettype => 'bool', proargtypes => 'text name',
+  prosrc => 'textgtname' },
 { oid => '252',
   proname => 'textnename', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text name', prosrc => 'textnename' },
 { oid => '253', descr => 'less-equal-greater',
-  proname => 'bttextnamecmp', proleakproof => 't', prorettype => 'int4',
-  proargtypes => 'text name', prosrc => 'bttextnamecmp' },
+  proname => 'bttextnamecmp', prorettype => 'int4', proargtypes => 'text name',
+  prosrc => 'bttextnamecmp' },
 
 { oid => '266', descr => 'concatenate name and oid',
   proname => 'nameconcatoid', prorettype => 'name', proargtypes => 'name oid',
@@ -982,14 +982,14 @@
   proname => 'btoidsortsupport', prorettype => 'void',
   proargtypes => 'internal', prosrc => 'btoidsortsupport' },
 { oid => '404', descr => 'less-equal-greater',
-  proname => 'btoidvectorcmp', prorettype => 'int4',
+  proname => 'btoidvectorcmp', proleakproof => 't', prorettype => 'int4',
   proargtypes => 'oidvector oidvector', prosrc => 'btoidvectorcmp' },
 { oid => '358', descr => 'less-equal-greater',
   proname => 'btcharcmp', proleakproof => 't', prorettype => 'int4',
   proargtypes => 'char char', prosrc => 'btcharcmp' },
 { oid => '359', descr => 'less-equal-greater',
-  proname => 'btnamecmp', proleakproof => 't', prorettype => 'int4',
-  proargtypes => 'name name', prosrc => 'btnamecmp' },
+  proname => 'btnamecmp', prorettype => 'int4', proargtypes => 'name name',
+  prosrc => 'btnamecmp' },
 { oid => '3135', descr => 'sort support',
   proname => 'btnamesortsupport', prorettype => 'void',
   proargtypes => 'internal', prosrc => 'btnamesortsupport' },
@@ -1308,17 +1308,17 @@
   prosrc => 'int28' },
 
 { oid => '655',
-  proname => 'namelt', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name name', prosrc => 'namelt' },
+  proname => 'namelt', prorettype => 'bool', proargtypes => 'name name',
+  prosrc => 'namelt' },
 { oid => '656',
-  proname => 'namele', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name name', prosrc => 'namele' },
+  proname => 'namele', prorettype => 'bool', proargtypes => 'name name',
+  prosrc => 'namele' },
 { oid => '657',
-  proname => 'namegt', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name name', prosrc => 'namegt' },
+  proname => 'namegt', prorettype => 'bool', proargtypes => 'name name',
+  prosrc => 'namegt' },
 { oid => '658',
-  proname => 'namege', proleakproof => 't', prorettype => 'bool',
-  proargtypes => 'name name', prosrc => 'namege' },
+  proname => 'namege', prorettype => 'bool', proargtypes => 'name name',
+  prosrc => 'namege' },
 { oid => '659',
   proname => 'namene', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'name name', prosrc => 'namene' },
@@ -1335,22 +1335,22 @@
   prosrc => 'varchar' },
 
 { oid => '619',
-  proname => 'oidvectorne', prorettype => 'bool',
+  proname => 'oidvectorne', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'oidvector oidvector', prosrc => 'oidvectorne' },
 { oid => '677',
-  proname => 'oidvectorlt', prorettype => 'bool',
+  proname => 'oidvectorlt', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'oidvector oidvector', prosrc => 'oidvectorlt' },
 { oid => '678',
-  proname => 'oidvectorle', prorettype => 'bool',
+  proname => 'oidvectorle', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'oidvector oidvector', prosrc => 'oidvectorle' },
 { oid => '679',
-  proname => 'oidvectoreq', prorettype => 'bool',
+  proname => 'oidvectoreq', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'oidvector oidvector', prosrc => 'oidvectoreq' },
 { oid => '680',
-  proname => 'oidvectorge', prorettype => 'bool',
+  proname => 'oidvectorge', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'oidvector oidvector', prosrc => 'oidvectorge' },
 { oid => '681',
-  proname => 'oidvectorgt', prorettype => 'bool',
+  proname => 'oidvectorgt', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'oidvector oidvector', prosrc => 'oidvectorgt' },
 
 # OIDS 700 - 799
@@ -8269,23 +8269,23 @@
   proname => 'pg_lsn_out', prorettype => 'cstring', proargtypes => 'pg_lsn',
   prosrc => 'pg_lsn_out' },
 { oid => '3231',
-  proname => 'pg_lsn_lt', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
-  prosrc => 'pg_lsn_lt' },
+  proname => 'pg_lsn_lt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_lt' },
 { oid => '3232',
-  proname => 'pg_lsn_le', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
-  prosrc => 'pg_lsn_le' },
+  proname => 'pg_lsn_le', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_le' },
 { oid => '3233',
-  proname => 'pg_lsn_eq', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
-  prosrc => 'pg_lsn_eq' },
+  proname => 'pg_lsn_eq', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_eq' },
 { oid => '3234',
-  proname => 'pg_lsn_ge', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
-  prosrc => 'pg_lsn_ge' },
+  proname => 'pg_lsn_ge', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_ge' },
 { oid => '3235',
-  proname => 'pg_lsn_gt', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
-  prosrc => 'pg_lsn_gt' },
+  proname => 'pg_lsn_gt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_gt' },
 { oid => '3236',
-  proname => 'pg_lsn_ne', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
-  prosrc => 'pg_lsn_ne' },
+  proname => 'pg_lsn_ne', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_ne' },
 { oid => '3237',
   proname => 'pg_lsn_mi', prorettype => 'numeric',
   proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_mi' },
@@ -8296,8 +8296,8 @@
   proname => 'pg_lsn_send', prorettype => 'bytea', proargtypes => 'pg_lsn',
   prosrc => 'pg_lsn_send' },
 { oid => '3251', descr => 'less-equal-greater',
-  proname => 'pg_lsn_cmp', prorettype => 'int4', proargtypes => 'pg_lsn pg_lsn',
-  prosrc => 'pg_lsn_cmp' },
+  proname => 'pg_lsn_cmp', proleakproof => 't', prorettype => 'int4',
+  proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_cmp' },
 { oid => '3252', descr => 'hash',
   proname => 'pg_lsn_hash', prorettype => 'int4', proargtypes => 'pg_lsn',
   prosrc => 'pg_lsn_hash' },

diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 214ad2d619c00a9ce2f38050ffea74d360849e4d..7328095b6fef6562cc3131f261a956f993ec4ee4 100644 (file)
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -526,19 +526,9 @@ int42ge(integer,smallint)
 oideq(oid,oid)
 oidne(oid,oid)
 nameeqtext(name,text)
-namelttext(name,text)
-nameletext(name,text)
-namegetext(name,text)
-namegttext(name,text)
 namenetext(name,text)
-btnametextcmp(name,text)
 texteqname(text,name)
-textltname(text,name)
-textlename(text,name)
-textgename(text,name)
-textgtname(text,name)
 textnename(text,name)
-bttextnamecmp(text,name)
 float4eq(real,real)
 float4ne(real,real)
 float4lt(real,real)
@@ -569,8 +559,8 @@ btfloat4cmp(real,real)
 btfloat8cmp(double precision,double precision)
 btoidcmp(oid,oid)
 btcharcmp("char","char")
-btnamecmp(name,name)
 cash_cmp(money,money)
+btoidvectorcmp(oidvector,oidvector)
 int8eq(bigint,bigint)
 int8ne(bigint,bigint)
 int8lt(bigint,bigint)
@@ -583,11 +573,13 @@ int84lt(bigint,integer)
 int84gt(bigint,integer)
 int84le(bigint,integer)
 int84ge(bigint,integer)
-namelt(name,name)
-namele(name,name)
-namegt(name,name)
-namege(name,name)
+oidvectorne(oidvector,oidvector)
 namene(name,name)
+oidvectorlt(oidvector,oidvector)
+oidvectorle(oidvector,oidvector)
+oidvectoreq(oidvector,oidvector)
+oidvectorge(oidvector,oidvector)
+oidvectorgt(oidvector,oidvector)
 oidlt(oid,oid)
 oidle(oid,oid)
 macaddr_eq(macaddr,macaddr)
@@ -737,6 +729,13 @@ uuid_ge(uuid,uuid)
 uuid_gt(uuid,uuid)
 uuid_ne(uuid,uuid)
 uuid_cmp(uuid,uuid)
+pg_lsn_lt(pg_lsn,pg_lsn)
+pg_lsn_le(pg_lsn,pg_lsn)
+pg_lsn_eq(pg_lsn,pg_lsn)
+pg_lsn_ge(pg_lsn,pg_lsn)
+pg_lsn_gt(pg_lsn,pg_lsn)
+pg_lsn_ne(pg_lsn,pg_lsn)
+pg_lsn_cmp(pg_lsn,pg_lsn)
 xidneq(xid,xid)
 xidneqint4(xid,integer)
 sha224(bytea)
@@ -1298,6 +1297,54 @@ ORDER BY 1;
   3953 | json_extract_path_text  | get value from json as text with path elements
 (9 rows)
 
+-- Operators that are commutator pairs should have identical volatility
+-- and leakproofness markings on their implementation functions.
+SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
+FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
+WHERE o1.oprcom = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
+    (p1.provolatile != p2.provolatile OR
+     p1.proleakproof != p2.proleakproof);
+ oid | oprcode | oid | oprcode 
+-----+---------+-----+---------
+(0 rows)
+
+-- Likewise for negator pairs.
+SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
+FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
+WHERE o1.oprnegate = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
+    (p1.provolatile != p2.provolatile OR
+     p1.proleakproof != p2.proleakproof);
+ oid | oprcode | oid | oprcode 
+-----+---------+-----+---------
+(0 rows)
+
+-- Btree comparison operators' functions should have the same volatility
+-- and leakproofness markings as the associated comparison support function.
+-- As of Postgres 12, the only exceptions are that textual equality functions
+-- are marked leakproof but textual comparison/inequality functions are not.
+SELECT pp.oid::regprocedure as proc, pp.provolatile as vp, pp.proleakproof as lp,
+       po.oid::regprocedure as opr, po.provolatile as vo, po.proleakproof as lo
+FROM pg_proc pp, pg_proc po, pg_operator o, pg_amproc ap, pg_amop ao
+WHERE pp.oid = ap.amproc AND po.oid = o.oprcode AND o.oid = ao.amopopr AND
+    ao.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
+    ao.amopfamily = ap.amprocfamily AND
+    ao.amoplefttype = ap.amproclefttype AND
+    ao.amoprighttype = ap.amprocrighttype AND
+    ap.amprocnum = 1 AND
+    (pp.provolatile != po.provolatile OR
+     pp.proleakproof != po.proleakproof)
+ORDER BY 1;
+                   proc                    | vp | lp |              opr              | vo | lo 
+-------------------------------------------+----+----+-------------------------------+----+----
+ btnametextcmp(name,text)                  | i  | f  | nameeqtext(name,text)         | i  | t
+ bttextnamecmp(text,name)                  | i  | f  | texteqname(text,name)         | i  | t
+ btnamecmp(name,name)                      | i  | f  | nameeq(name,name)             | i  | t
+ bttextcmp(text,text)                      | i  | f  | texteq(text,text)             | i  | t
+ bpcharcmp(character,character)            | i  | f  | bpchareq(character,character) | i  | t
+ bttext_pattern_cmp(text,text)             | i  | f  | texteq(text,text)             | i  | t
+ btbpchar_pattern_cmp(character,character) | i  | f  | bpchareq(character,character) | i  | t
+(7 rows)
+
 -- **************** pg_aggregate ****************
 -- Look for illegal values in pg_aggregate fields.
 SELECT ctid, aggfnoid::oid

diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 64eca7e91538c2d9c70c5dfdaf9d52f0b454ca23..8544cbe62e8327cad7a8394e9b66072e7d669d4b 100644 (file)
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -789,6 +789,38 @@ SELECT p_oid, proname, prodesc FROM funcdescs
     AND oprdesc NOT LIKE 'deprecated%'
 ORDER BY 1;
 
+-- Operators that are commutator pairs should have identical volatility
+-- and leakproofness markings on their implementation functions.
+SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
+FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
+WHERE o1.oprcom = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
+    (p1.provolatile != p2.provolatile OR
+     p1.proleakproof != p2.proleakproof);
+
+-- Likewise for negator pairs.
+SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
+FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
+WHERE o1.oprnegate = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
+    (p1.provolatile != p2.provolatile OR
+     p1.proleakproof != p2.proleakproof);
+
+-- Btree comparison operators' functions should have the same volatility
+-- and leakproofness markings as the associated comparison support function.
+-- As of Postgres 12, the only exceptions are that textual equality functions
+-- are marked leakproof but textual comparison/inequality functions are not.
+SELECT pp.oid::regprocedure as proc, pp.provolatile as vp, pp.proleakproof as lp,
+       po.oid::regprocedure as opr, po.provolatile as vo, po.proleakproof as lo
+FROM pg_proc pp, pg_proc po, pg_operator o, pg_amproc ap, pg_amop ao
+WHERE pp.oid = ap.amproc AND po.oid = o.oprcode AND o.oid = ao.amopopr AND
+    ao.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
+    ao.amopfamily = ap.amprocfamily AND
+    ao.amoplefttype = ap.amproclefttype AND
+    ao.amoprighttype = ap.amprocrighttype AND
+    ap.amprocnum = 1 AND
+    (pp.provolatile != po.provolatile OR
+     pp.proleakproof != po.proleakproof)
+ORDER BY 1;
+
 
 -- **************** pg_aggregate ****************