Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit d01e75d

Browse files
committed
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 586b98f.) 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
1 parent e439c6f commit d01e75d

File tree

4 files changed

+146
-67
lines changed

4 files changed

+146
-67
lines changed

src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/* yyyymmddN */
56-
#define CATALOG_VERSION_NO 201812301
56+
#define CATALOG_VERSION_NO 201812311
5757

5858
#endif

src/include/catalog/pg_proc.dat

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -677,44 +677,44 @@
677677
proname => 'nameeqtext', proleakproof => 't', prorettype => 'bool',
678678
proargtypes => 'name text', prosrc => 'nameeqtext' },
679679
{ oid => '241',
680-
proname => 'namelttext', proleakproof => 't', prorettype => 'bool',
681-
proargtypes => 'name text', prosrc => 'namelttext' },
680+
proname => 'namelttext', prorettype => 'bool', proargtypes => 'name text',
681+
prosrc => 'namelttext' },
682682
{ oid => '242',
683-
proname => 'nameletext', proleakproof => 't', prorettype => 'bool',
684-
proargtypes => 'name text', prosrc => 'nameletext' },
683+
proname => 'nameletext', prorettype => 'bool', proargtypes => 'name text',
684+
prosrc => 'nameletext' },
685685
{ oid => '243',
686-
proname => 'namegetext', proleakproof => 't', prorettype => 'bool',
687-
proargtypes => 'name text', prosrc => 'namegetext' },
686+
proname => 'namegetext', prorettype => 'bool', proargtypes => 'name text',
687+
prosrc => 'namegetext' },
688688
{ oid => '244',
689-
proname => 'namegttext', proleakproof => 't', prorettype => 'bool',
690-
proargtypes => 'name text', prosrc => 'namegttext' },
689+
proname => 'namegttext', prorettype => 'bool', proargtypes => 'name text',
690+
prosrc => 'namegttext' },
691691
{ oid => '245',
692692
proname => 'namenetext', proleakproof => 't', prorettype => 'bool',
693693
proargtypes => 'name text', prosrc => 'namenetext' },
694694
{ oid => '246', descr => 'less-equal-greater',
695-
proname => 'btnametextcmp', proleakproof => 't', prorettype => 'int4',
696-
proargtypes => 'name text', prosrc => 'btnametextcmp' },
695+
proname => 'btnametextcmp', prorettype => 'int4', proargtypes => 'name text',
696+
prosrc => 'btnametextcmp' },
697697
{ oid => '247',
698698
proname => 'texteqname', proleakproof => 't', prorettype => 'bool',
699699
proargtypes => 'text name', prosrc => 'texteqname' },
700700
{ oid => '248',
701-
proname => 'textltname', proleakproof => 't', prorettype => 'bool',
702-
proargtypes => 'text name', prosrc => 'textltname' },
701+
proname => 'textltname', prorettype => 'bool', proargtypes => 'text name',
702+
prosrc => 'textltname' },
703703
{ oid => '249',
704-
proname => 'textlename', proleakproof => 't', prorettype => 'bool',
705-
proargtypes => 'text name', prosrc => 'textlename' },
704+
proname => 'textlename', prorettype => 'bool', proargtypes => 'text name',
705+
prosrc => 'textlename' },
706706
{ oid => '250',
707-
proname => 'textgename', proleakproof => 't', prorettype => 'bool',
708-
proargtypes => 'text name', prosrc => 'textgename' },
707+
proname => 'textgename', prorettype => 'bool', proargtypes => 'text name',
708+
prosrc => 'textgename' },
709709
{ oid => '251',
710-
proname => 'textgtname', proleakproof => 't', prorettype => 'bool',
711-
proargtypes => 'text name', prosrc => 'textgtname' },
710+
proname => 'textgtname', prorettype => 'bool', proargtypes => 'text name',
711+
prosrc => 'textgtname' },
712712
{ oid => '252',
713713
proname => 'textnename', proleakproof => 't', prorettype => 'bool',
714714
proargtypes => 'text name', prosrc => 'textnename' },
715715
{ oid => '253', descr => 'less-equal-greater',
716-
proname => 'bttextnamecmp', proleakproof => 't', prorettype => 'int4',
717-
proargtypes => 'text name', prosrc => 'bttextnamecmp' },
716+
proname => 'bttextnamecmp', prorettype => 'int4', proargtypes => 'text name',
717+
prosrc => 'bttextnamecmp' },
718718

719719
{ oid => '266', descr => 'concatenate name and oid',
720720
proname => 'nameconcatoid', prorettype => 'name', proargtypes => 'name oid',
@@ -982,14 +982,14 @@
982982
proname => 'btoidsortsupport', prorettype => 'void',
983983
proargtypes => 'internal', prosrc => 'btoidsortsupport' },
984984
{ oid => '404', descr => 'less-equal-greater',
985-
proname => 'btoidvectorcmp', prorettype => 'int4',
985+
proname => 'btoidvectorcmp', proleakproof => 't', prorettype => 'int4',
986986
proargtypes => 'oidvector oidvector', prosrc => 'btoidvectorcmp' },
987987
{ oid => '358', descr => 'less-equal-greater',
988988
proname => 'btcharcmp', proleakproof => 't', prorettype => 'int4',
989989
proargtypes => 'char char', prosrc => 'btcharcmp' },
990990
{ oid => '359', descr => 'less-equal-greater',
991-
proname => 'btnamecmp', proleakproof => 't', prorettype => 'int4',
992-
proargtypes => 'name name', prosrc => 'btnamecmp' },
991+
proname => 'btnamecmp', prorettype => 'int4', proargtypes => 'name name',
992+
prosrc => 'btnamecmp' },
993993
{ oid => '3135', descr => 'sort support',
994994
proname => 'btnamesortsupport', prorettype => 'void',
995995
proargtypes => 'internal', prosrc => 'btnamesortsupport' },
@@ -1308,17 +1308,17 @@
13081308
prosrc => 'int28' },
13091309

13101310
{ oid => '655',
1311-
proname => 'namelt', proleakproof => 't', prorettype => 'bool',
1312-
proargtypes => 'name name', prosrc => 'namelt' },
1311+
proname => 'namelt', prorettype => 'bool', proargtypes => 'name name',
1312+
prosrc => 'namelt' },
13131313
{ oid => '656',
1314-
proname => 'namele', proleakproof => 't', prorettype => 'bool',
1315-
proargtypes => 'name name', prosrc => 'namele' },
1314+
proname => 'namele', prorettype => 'bool', proargtypes => 'name name',
1315+
prosrc => 'namele' },
13161316
{ oid => '657',
1317-
proname => 'namegt', proleakproof => 't', prorettype => 'bool',
1318-
proargtypes => 'name name', prosrc => 'namegt' },
1317+
proname => 'namegt', prorettype => 'bool', proargtypes => 'name name',
1318+
prosrc => 'namegt' },
13191319
{ oid => '658',
1320-
proname => 'namege', proleakproof => 't', prorettype => 'bool',
1321-
proargtypes => 'name name', prosrc => 'namege' },
1320+
proname => 'namege', prorettype => 'bool', proargtypes => 'name name',
1321+
prosrc => 'namege' },
13221322
{ oid => '659',
13231323
proname => 'namene', proleakproof => 't', prorettype => 'bool',
13241324
proargtypes => 'name name', prosrc => 'namene' },
@@ -1335,22 +1335,22 @@
13351335
prosrc => 'varchar' },
13361336

13371337
{ oid => '619',
1338-
proname => 'oidvectorne', prorettype => 'bool',
1338+
proname => 'oidvectorne', proleakproof => 't', prorettype => 'bool',
13391339
proargtypes => 'oidvector oidvector', prosrc => 'oidvectorne' },
13401340
{ oid => '677',
1341-
proname => 'oidvectorlt', prorettype => 'bool',
1341+
proname => 'oidvectorlt', proleakproof => 't', prorettype => 'bool',
13421342
proargtypes => 'oidvector oidvector', prosrc => 'oidvectorlt' },
13431343
{ oid => '678',
1344-
proname => 'oidvectorle', prorettype => 'bool',
1344+
proname => 'oidvectorle', proleakproof => 't', prorettype => 'bool',
13451345
proargtypes => 'oidvector oidvector', prosrc => 'oidvectorle' },
13461346
{ oid => '679',
1347-
proname => 'oidvectoreq', prorettype => 'bool',
1347+
proname => 'oidvectoreq', proleakproof => 't', prorettype => 'bool',
13481348
proargtypes => 'oidvector oidvector', prosrc => 'oidvectoreq' },
13491349
{ oid => '680',
1350-
proname => 'oidvectorge', prorettype => 'bool',
1350+
proname => 'oidvectorge', proleakproof => 't', prorettype => 'bool',
13511351
proargtypes => 'oidvector oidvector', prosrc => 'oidvectorge' },
13521352
{ oid => '681',
1353-
proname => 'oidvectorgt', prorettype => 'bool',
1353+
proname => 'oidvectorgt', proleakproof => 't', prorettype => 'bool',
13541354
proargtypes => 'oidvector oidvector', prosrc => 'oidvectorgt' },
13551355

13561356
# OIDS 700 - 799
@@ -8269,23 +8269,23 @@
82698269
proname => 'pg_lsn_out', prorettype => 'cstring', proargtypes => 'pg_lsn',
82708270
prosrc => 'pg_lsn_out' },
82718271
{ oid => '3231',
8272-
proname => 'pg_lsn_lt', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
8273-
prosrc => 'pg_lsn_lt' },
8272+
proname => 'pg_lsn_lt', proleakproof => 't', prorettype => 'bool',
8273+
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_lt' },
82748274
{ oid => '3232',
8275-
proname => 'pg_lsn_le', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
8276-
prosrc => 'pg_lsn_le' },
8275+
proname => 'pg_lsn_le', proleakproof => 't', prorettype => 'bool',
8276+
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_le' },
82778277
{ oid => '3233',
8278-
proname => 'pg_lsn_eq', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
8279-
prosrc => 'pg_lsn_eq' },
8278+
proname => 'pg_lsn_eq', proleakproof => 't', prorettype => 'bool',
8279+
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_eq' },
82808280
{ oid => '3234',
8281-
proname => 'pg_lsn_ge', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
8282-
prosrc => 'pg_lsn_ge' },
8281+
proname => 'pg_lsn_ge', proleakproof => 't', prorettype => 'bool',
8282+
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_ge' },
82838283
{ oid => '3235',
8284-
proname => 'pg_lsn_gt', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
8285-
prosrc => 'pg_lsn_gt' },
8284+
proname => 'pg_lsn_gt', proleakproof => 't', prorettype => 'bool',
8285+
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_gt' },
82868286
{ oid => '3236',
8287-
proname => 'pg_lsn_ne', prorettype => 'bool', proargtypes => 'pg_lsn pg_lsn',
8288-
prosrc => 'pg_lsn_ne' },
8287+
proname => 'pg_lsn_ne', proleakproof => 't', prorettype => 'bool',
8288+
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_ne' },
82898289
{ oid => '3237',
82908290
proname => 'pg_lsn_mi', prorettype => 'numeric',
82918291
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_mi' },
@@ -8296,8 +8296,8 @@
82968296
proname => 'pg_lsn_send', prorettype => 'bytea', proargtypes => 'pg_lsn',
82978297
prosrc => 'pg_lsn_send' },
82988298
{ oid => '3251', descr => 'less-equal-greater',
8299-
proname => 'pg_lsn_cmp', prorettype => 'int4', proargtypes => 'pg_lsn pg_lsn',
8300-
prosrc => 'pg_lsn_cmp' },
8299+
proname => 'pg_lsn_cmp', proleakproof => 't', prorettype => 'int4',
8300+
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_cmp' },
83018301
{ oid => '3252', descr => 'hash',
83028302
proname => 'pg_lsn_hash', prorettype => 'int4', proargtypes => 'pg_lsn',
83038303
prosrc => 'pg_lsn_hash' },

src/test/regress/expected/opr_sanity.out

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -526,19 +526,9 @@ int42ge(integer,smallint)
526526
oideq(oid,oid)
527527
oidne(oid,oid)
528528
nameeqtext(name,text)
529-
namelttext(name,text)
530-
nameletext(name,text)
531-
namegetext(name,text)
532-
namegttext(name,text)
533529
namenetext(name,text)
534-
btnametextcmp(name,text)
535530
texteqname(text,name)
536-
textltname(text,name)
537-
textlename(text,name)
538-
textgename(text,name)
539-
textgtname(text,name)
540531
textnename(text,name)
541-
bttextnamecmp(text,name)
542532
float4eq(real,real)
543533
float4ne(real,real)
544534
float4lt(real,real)
@@ -569,8 +559,8 @@ btfloat4cmp(real,real)
569559
btfloat8cmp(double precision,double precision)
570560
btoidcmp(oid,oid)
571561
btcharcmp("char","char")
572-
btnamecmp(name,name)
573562
cash_cmp(money,money)
563+
btoidvectorcmp(oidvector,oidvector)
574564
int8eq(bigint,bigint)
575565
int8ne(bigint,bigint)
576566
int8lt(bigint,bigint)
@@ -583,11 +573,13 @@ int84lt(bigint,integer)
583573
int84gt(bigint,integer)
584574
int84le(bigint,integer)
585575
int84ge(bigint,integer)
586-
namelt(name,name)
587-
namele(name,name)
588-
namegt(name,name)
589-
namege(name,name)
576+
oidvectorne(oidvector,oidvector)
590577
namene(name,name)
578+
oidvectorlt(oidvector,oidvector)
579+
oidvectorle(oidvector,oidvector)
580+
oidvectoreq(oidvector,oidvector)
581+
oidvectorge(oidvector,oidvector)
582+
oidvectorgt(oidvector,oidvector)
591583
oidlt(oid,oid)
592584
oidle(oid,oid)
593585
macaddr_eq(macaddr,macaddr)
@@ -737,6 +729,13 @@ uuid_ge(uuid,uuid)
737729
uuid_gt(uuid,uuid)
738730
uuid_ne(uuid,uuid)
739731
uuid_cmp(uuid,uuid)
732+
pg_lsn_lt(pg_lsn,pg_lsn)
733+
pg_lsn_le(pg_lsn,pg_lsn)
734+
pg_lsn_eq(pg_lsn,pg_lsn)
735+
pg_lsn_ge(pg_lsn,pg_lsn)
736+
pg_lsn_gt(pg_lsn,pg_lsn)
737+
pg_lsn_ne(pg_lsn,pg_lsn)
738+
pg_lsn_cmp(pg_lsn,pg_lsn)
740739
xidneq(xid,xid)
741740
xidneqint4(xid,integer)
742741
sha224(bytea)
@@ -1298,6 +1297,54 @@ ORDER BY 1;
12981297
3953 | json_extract_path_text | get value from json as text with path elements
12991298
(9 rows)
13001299

1300+
-- Operators that are commutator pairs should have identical volatility
1301+
-- and leakproofness markings on their implementation functions.
1302+
SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
1303+
FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
1304+
WHERE o1.oprcom = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
1305+
(p1.provolatile != p2.provolatile OR
1306+
p1.proleakproof != p2.proleakproof);
1307+
oid | oprcode | oid | oprcode
1308+
-----+---------+-----+---------
1309+
(0 rows)
1310+
1311+
-- Likewise for negator pairs.
1312+
SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
1313+
FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
1314+
WHERE o1.oprnegate = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
1315+
(p1.provolatile != p2.provolatile OR
1316+
p1.proleakproof != p2.proleakproof);
1317+
oid | oprcode | oid | oprcode
1318+
-----+---------+-----+---------
1319+
(0 rows)
1320+
1321+
-- Btree comparison operators' functions should have the same volatility
1322+
-- and leakproofness markings as the associated comparison support function.
1323+
-- As of Postgres 12, the only exceptions are that textual equality functions
1324+
-- are marked leakproof but textual comparison/inequality functions are not.
1325+
SELECT pp.oid::regprocedure as proc, pp.provolatile as vp, pp.proleakproof as lp,
1326+
po.oid::regprocedure as opr, po.provolatile as vo, po.proleakproof as lo
1327+
FROM pg_proc pp, pg_proc po, pg_operator o, pg_amproc ap, pg_amop ao
1328+
WHERE pp.oid = ap.amproc AND po.oid = o.oprcode AND o.oid = ao.amopopr AND
1329+
ao.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
1330+
ao.amopfamily = ap.amprocfamily AND
1331+
ao.amoplefttype = ap.amproclefttype AND
1332+
ao.amoprighttype = ap.amprocrighttype AND
1333+
ap.amprocnum = 1 AND
1334+
(pp.provolatile != po.provolatile OR
1335+
pp.proleakproof != po.proleakproof)
1336+
ORDER BY 1;
1337+
proc | vp | lp | opr | vo | lo
1338+
-------------------------------------------+----+----+-------------------------------+----+----
1339+
btnametextcmp(name,text) | i | f | nameeqtext(name,text) | i | t
1340+
bttextnamecmp(text,name) | i | f | texteqname(text,name) | i | t
1341+
btnamecmp(name,name) | i | f | nameeq(name,name) | i | t
1342+
bttextcmp(text,text) | i | f | texteq(text,text) | i | t
1343+
bpcharcmp(character,character) | i | f | bpchareq(character,character) | i | t
1344+
bttext_pattern_cmp(text,text) | i | f | texteq(text,text) | i | t
1345+
btbpchar_pattern_cmp(character,character) | i | f | bpchareq(character,character) | i | t
1346+
(7 rows)
1347+
13011348
-- **************** pg_aggregate ****************
13021349
-- Look for illegal values in pg_aggregate fields.
13031350
SELECT ctid, aggfnoid::oid

src/test/regress/sql/opr_sanity.sql

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,38 @@ SELECT p_oid, proname, prodesc FROM funcdescs
789789
AND oprdesc NOT LIKE 'deprecated%'
790790
ORDER BY 1;
791791

792+
-- Operators that are commutator pairs should have identical volatility
793+
-- and leakproofness markings on their implementation functions.
794+
SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
795+
FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
796+
WHERE o1.oprcom = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
797+
(p1.provolatile != p2.provolatile OR
798+
p1.proleakproof != p2.proleakproof);
799+
800+
-- Likewise for negator pairs.
801+
SELECT o1.oid, o1.oprcode, o2.oid, o2.oprcode
802+
FROM pg_operator AS o1, pg_operator AS o2, pg_proc AS p1, pg_proc AS p2
803+
WHERE o1.oprnegate = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND
804+
(p1.provolatile != p2.provolatile OR
805+
p1.proleakproof != p2.proleakproof);
806+
807+
-- Btree comparison operators' functions should have the same volatility
808+
-- and leakproofness markings as the associated comparison support function.
809+
-- As of Postgres 12, the only exceptions are that textual equality functions
810+
-- are marked leakproof but textual comparison/inequality functions are not.
811+
SELECT pp.oid::regprocedure as proc, pp.provolatile as vp, pp.proleakproof as lp,
812+
po.oid::regprocedure as opr, po.provolatile as vo, po.proleakproof as lo
813+
FROM pg_proc pp, pg_proc po, pg_operator o, pg_amproc ap, pg_amop ao
814+
WHERE pp.oid = ap.amproc AND po.oid = o.oprcode AND o.oid = ao.amopopr AND
815+
ao.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
816+
ao.amopfamily = ap.amprocfamily AND
817+
ao.amoplefttype = ap.amproclefttype AND
818+
ao.amoprighttype = ap.amprocrighttype AND
819+
ap.amprocnum = 1 AND
820+
(pp.provolatile != po.provolatile OR
821+
pp.proleakproof != po.proleakproof)
822+
ORDER BY 1;
823+
792824

793825
-- **************** pg_aggregate ****************
794826

0 commit comments

Comments
 (0)