Numeric is not leakproof

Started by Konstantin Knizhnikalmost 7 years ago4 messagesbugs
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliestests failedCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t69624
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 20, 2026 at 12:29 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t69624_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t69624_1 && git checkout t69624_1

Patchset v1 (message #1) is on t69624_1

Jump to latest
#1Konstantin Knizhnik
k.knizhnik@postgrespro.ru

Numeric functions are not marked as leakproof in pg_proc.dat
It cause unexpected behavior in case of using row-level security:

create user tester login;
create role readers;
create table document(id numeric primary key, is_deleted boolean);
create index on document(is_deleted);
ALTER TABLE document ENABLE ROW LEVEL SECURITY;
insert into document values (generate_series(1,100000));
CREATE POLICY read_all_docs ON document FOR SELECT TO readers USING (NOT
IS_DELETED);
grant readers to tester;
grant select on document to readers;
analyze document;

set role tester;
explain select * from document where id=1001;

                                       QUERY PLAN
----------------------------------------------------------------------------------------
 Index Scan using document_is_deleted_idx on document (cost=0.29..8.31
rows=1 width=7)
   Index Cond: (is_deleted = false)
   Filter: (id = '1001'::numeric)
(3 rows)

So we are no using index in "id" just because comparison function for
numeric type is  not leakproof and we can not call it before checking
RLS constraint.
The attached simple patch fixes the problem for numeric type. With this
patch query plan is normal:

                                  QUERY PLAN
------------------------------------------------------------------------------
 Index Scan using document_pkey on document  (cost=0.29..8.31 rows=1
width=7)
   Index Cond: (id = '1001'::numeric)
   Filter: (NOT is_deleted)
(3 rows)

I have not checked all other builtin type.
But it seems to me that it may be reasonable to mark ALL builtin
functions (described in pg_proc.dat) as leekprof by default.

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

t69624_1
numeric-leakproof.patchtext/x-patch; name=numeric-leakproof.patchDownload+6-6
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Konstantin Knizhnik (#1)
Re: Numeric is not leakproof

Konstantin Knizhnik <k.knizhnik@postgrespro.ru> writes:

Numeric functions are not marked as leakproof in pg_proc.dat

Indeed. Nobody has done the analysis needed to decide that it'd be safe
to do so. For comparison, see the rather considerable discussion that
occurred before marking the text comparison functions leakproof.

But it seems to me that it may be reasonable to mark ALL builtin
functions (described in pg_proc.dat) as leekprof by default.

This proposal is risible. But if you actually need a counterexample,
here's one:

regression=# select 'abc' ~ '(foo';
ERROR: invalid regular expression: parentheses () not balanced

regards, tom lane

#3Stephen Frost
sfrost@snowman.net
In reply to: Konstantin Knizhnik (#1)
Re: Numeric is not leakproof

Greetings,

* Konstantin Knizhnik (k.knizhnik@postgrespro.ru) wrote:

Numeric functions are not marked as leakproof in pg_proc.dat
It cause unexpected behavior in case of using row-level security:

The behavior you're getting is *entirely* expected, just to be clear.
Perhaps unfortunate and not as performant as you were hoping, but
definitely not unexpected.

As Tom noted downthread, you can't just mark things 'leakproof' because
you want them to be able to be used in an index- you need to actually
show that they're leakproof.

I have not checked all other builtin type.
But it seems to me that it may be reasonable to mark ALL builtin functions
(described in pg_proc.dat) as leekprof by default.

Absolutely not without careful verification of each and every one.
There's nothing that guarantees builtins are leakproof (and indeed,
there's no shortage of ones that are clearly *not* leakproof today).

I'd love it for someone to go through and fix them all to actually be
leakproof (or at least all of the ones that might be used with an index)
but that clearly hasn't been done here.

Thanks,

Stephen

#4Konstantin Knizhnik
k.knizhnik@postgrespro.ru
In reply to: Stephen Frost (#3)
Re: Numeric is not leakproof

On 03.12.2019 23:43, Stephen Frost wrote:

Greetings,

* Konstantin Knizhnik (k.knizhnik@postgrespro.ru) wrote:

Numeric functions are not marked as leakproof in pg_proc.dat
It cause unexpected behavior in case of using row-level security:

The behavior you're getting is *entirely* expected, just to be clear.
Perhaps unfortunate and not as performant as you were hoping, but
definitely not unexpected.

As Tom noted downthread, you can't just mark things 'leakproof' because
you want them to be able to be used in an index- you need to actually
show that they're leakproof.

I have not checked all other builtin type.
But it seems to me that it may be reasonable to mark ALL builtin functions
(described in pg_proc.dat) as leekprof by default.

Absolutely not without careful verification of each and every one.
There's nothing that guarantees builtins are leakproof (and indeed,
there's no shortage of ones that are clearly *not* leakproof today).

I'd love it for someone to go through and fix them all to actually be
leakproof (or at least all of the ones that might be used with an index)
but that clearly hasn't been done here.

Ok, I understand that it is not possible just to mark all built-in
functions as leak proof, but what about
marking as leakproof just comparison functions for the numeric type as I
proposed in the attached patch?
I have checked that cmp_numerics can not report any errors and so it can
be considered as leekprof as far as comparison functions for most of
other builtin types
which are already marked as leakproof.

After applying this patch opr_sanity test is failed (just because list
of leakproof functions is extended, so expected result for this test
should also be updated).

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company