From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: Stephen Frost <sfrost@snowman.net>
Subject: Is MinMaxExpr really leakproof?
Date: Sun, 30 Dec 2018 13:24:02 -0500
Message-ID: <31042.1546194242@sss.pgh.pa.us> (raw)
contain_leaked_vars_walker asserts the following about MinMaxExpr:
...
case T_MinMaxExpr:
...
/*
* We know these node types don't contain function calls; but
* something further down in the node tree might.
*/
break;
Now, the idea that it "doesn't contain a function call" is nonsense,
because the node will invoke the btree comparison function for the
datatype of its arguments. So this coding amounts to an undocumented
assumption that every non-cross-type btree comparison function is
leakproof.
A quick catalog query finds 15 counterexamples just among the
built-in datatypes:
select p.oid::regprocedure from pg_proc p, pg_amproc a, pg_opfamily f
where p.oid=a.amproc and f.oid=a.amprocfamily and opfmethod=403 and not proleakproof and amproclefttype=amprocrighttype and amprocnum=1;
bpcharcmp(character,character)
btarraycmp(anyarray,anyarray)
btbpchar_pattern_cmp(character,character)
btoidvectorcmp(oidvector,oidvector)
btrecordcmp(record,record)
btrecordimagecmp(record,record)
bttext_pattern_cmp(text,text)
bttextcmp(text,text)
enum_cmp(anyenum,anyenum)
jsonb_cmp(jsonb,jsonb)
numeric_cmp(numeric,numeric)
pg_lsn_cmp(pg_lsn,pg_lsn)
range_cmp(anyrange,anyrange)
tsquery_cmp(tsquery,tsquery)
tsvector_cmp(tsvector,tsvector)
so this assumption is, on its face, wrong.
In practice it might be all right, because it's hard to see a reason why
a btree comparison function would ever throw an error except for internal
failures, which are probably outside the scope of leakproofness guarantees
anyway. Nonetheless, if we didn't mark these functions as leakproof,
why not?
I think that we should either change contain_leaked_vars_walker to
explicitly verify leakproofness of the comparison function, or decide
that it's project policy that btree comparison functions are leakproof,
and change the markings on those (and their associated operators).
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, sfrost@snowman.net
Subject: Re: Is MinMaxExpr really leakproof?
In-Reply-To: <31042.1546194242@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox