Hi,
On Tue, 30 Jul 2024 01:36:55 +0200
Erik Wienhold <e...@ewie.name> wrote:
> On 2024-07-01 15:08 +0200, Yugo NAGATA wrote:
> > I would like to propose to add a new field to psql's \dAo+ meta-command
> > to show whether the underlying function of an operator is leak-proof.
>
> +1 for making that info easily accessible.
>
> > This idea is inspired from [1] that claims some indexes uses non-LEAKPROOF
> > functions under the associated operators, as a result, it can not be
> > selected
> > for queries with security_barrier views or row-level security policies.
> > The original proposal was to add a query over system catalogs for looking up
> > non-leakproof operators to the documentation, but I thought it is useful
> > to improve \dAo results rather than putting such query to the doc.
> >
> > The attached patch adds the field to \dAo+ and also a description that
> > explains the relation between indexes and security quals with referencing
> > \dAo+ meta-command.
> >
> > [1]
> > https://www.postgresql.org/message-id/raw/5af3bf0c-5e0c-4128-81dc-084c5258b1af%40code406.com
>
> \dAo+ output looks good.
Thank you for looking into this.
I attached a patch updated with your suggestions.
>
> But this patch fails regression tests in src/test/regress/sql/psql.sql
> (\dAo+ btree float_ops) because of the new leak-proof column. I think
> this could even be changed to "\dAo+ btree array_ops|float_ops" to also
> cover operators that are not leak-proof.
Thank you for pointing out this. I fixed it with you suggestion to cover
non leak-proof operators, too.
> +<para>
> + For example, an index scan can not be selected for queries with
>
> I check the docs and "cannot" is more commonly used than "can not".
Fixed.
>
> + <literal>security_barrier</literal> views or row-level security policies
> if an
> + operator used in the <literal>WHERE</literal> clause is associated with
> the
> + operator family of the index, but its underlying function is not marked
> + <literal>LEAKPROOF</literal>. The <xref linkend="app-psql"/> program's
> + <command>\dAo+</command> meta-command is useful for listing the operators
> + with associated operator families and whether it is leak-proof.
> +</para>
>
> I think the last sentence can be improved. How about: "Use psql's \dAo+
> command to list operator families and tell which of their operators are
> marked as leak-proof."? Should something similar be added to [1] which
> also talks about leak-proof operators?
I agree, so I fixed the sentence as your suggestion and also add the
same description to the planner-stats-security doc.
> The rest is just formatting nitpicks:
>
> + ", ofs.opfname AS \"%s\"\n,"
>
> The trailing comma should come before the newline.
>
> + " CASE\n"
> + " WHEN p.proleakproof THEN '%s'\n"
> + " ELSE '%s'\n"
> + " END AS \"%s\"\n",
>
> WHEN/ELSE/END should be intended with one additional space to be
> consistent with the other CASE expressions in this query.
Fixed both.
Regards,
Yugo Nagata
>
> [1] https://www.postgresql.org/docs/devel/planner-stats-security.html
>
> --
> Erik
--
Yugo NAGATA <nag...@sraoss.co.jp>
>From ca41705da15ca588d55f3c2cc2106284911e53a1 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Mon, 1 Jul 2024 16:16:39 +0900
Subject: [PATCH v2] psql: Add leakproof field to \dAo+ meta-command results
This adds a field that shows whether the underlying function of an
operator associated with operator families is leak-proof.
It is useful for checking an index can be used with security_barrier
views or row-level security policies when the query's WHERE
clause contains an operator which is associated with the index.
---
doc/src/sgml/planstats.sgml | 2 ++
doc/src/sgml/ref/psql-ref.sgml | 3 +-
doc/src/sgml/rules.sgml | 10 ++++++
src/bin/psql/describe.c | 17 ++++++---
src/test/regress/expected/psql.out | 55 ++++++++++++++++--------------
src/test/regress/sql/psql.sql | 2 +-
6 files changed, 58 insertions(+), 31 deletions(-)
diff --git a/doc/src/sgml/planstats.sgml b/doc/src/sgml/planstats.sgml
index c7ec749d0a..45b0d2b765 100644
--- a/doc/src/sgml/planstats.sgml
+++ b/doc/src/sgml/planstats.sgml
@@ -729,6 +729,8 @@ EXPLAIN (ANALYZE, TIMING OFF) SELECT * FROM t WHERE a <= 49 AND b > 49;
accurately, the function that the operator is based on). If not, then the
selectivity estimator will behave as if no statistics are available, and
the planner will proceed with default or fall-back assumptions.
+ Use <xref linkend="app-psql"/>'s <command>\dAo+</command> command to list
+ operator families and tell which of their operators are marked as leak-proof.
</para>
<para>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 07419a3b92..4d55472929 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1363,7 +1363,8 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
is specified, only members of operator families whose names match that
pattern are listed.
If <literal>+</literal> is appended to the command name, each operator
- is listed with its sort operator family (if it is an ordering operator).
+ is listed with its sort operator family (if it is an ordering operator),
+ and whether it is leak-proof.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
index 7a928bd7b9..3f0c26dad3 100644
--- a/doc/src/sgml/rules.sgml
+++ b/doc/src/sgml/rules.sgml
@@ -2167,6 +2167,16 @@ CREATE VIEW phone_number WITH (security_barrier) AS
view's row filters.
</para>
+<para>
+ For example, an index scan cannot be selected for queries with
+ <literal>security_barrier</literal> views or row-level security policies if an
+ operator used in the <literal>WHERE</literal> clause is associated with the
+ operator family of the index, but its underlying function is not marked
+ <literal>LEAKPROOF</literal>. Use <xref linkend="app-psql"/>'s
+ <command>\dAo+</command> command to list operator families and tell which of
+ their operators are marked as leak-proof.
+</para>
+
<para>
It is important to understand that even a view created with the
<literal>security_barrier</literal> option is intended to be secure only
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 7c9a1f234c..d1b2910073 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6882,7 +6882,7 @@ listOpFamilyOperators(const char *access_method_pattern,
printQueryOpt myopt = pset.popt;
bool have_where = false;
- static const bool translate_columns[] = {false, false, false, false, false, false};
+ static const bool translate_columns[] = {false, false, false, false, false, false, false};
initPQExpBuffer(&buf);
@@ -6910,8 +6910,15 @@ listOpFamilyOperators(const char *access_method_pattern,
if (verbose)
appendPQExpBuffer(&buf,
- ", ofs.opfname AS \"%s\"\n",
- gettext_noop("Sort opfamily"));
+ ", ofs.opfname AS \"%s\",\n"
+ " CASE\n"
+ " WHEN p.proleakproof THEN '%s'\n"
+ " ELSE '%s'\n"
+ " END AS \"%s\"\n",
+ gettext_noop("Sort opfamily"),
+ gettext_noop("yes"),
+ gettext_noop("no"),
+ gettext_noop("Leak-proof"));
appendPQExpBufferStr(&buf,
"FROM pg_catalog.pg_amop o\n"
" LEFT JOIN pg_catalog.pg_opfamily of ON of.oid = o.amopfamily\n"
@@ -6919,7 +6926,9 @@ listOpFamilyOperators(const char *access_method_pattern,
" LEFT JOIN pg_catalog.pg_namespace nsf ON of.opfnamespace = nsf.oid\n");
if (verbose)
appendPQExpBufferStr(&buf,
- " LEFT JOIN pg_catalog.pg_opfamily ofs ON ofs.oid = o.amopsortfamily\n");
+ " LEFT JOIN pg_catalog.pg_opfamily ofs ON ofs.oid = o.amopsortfamily\n"
+ " LEFT JOIN pg_catalog.pg_operator op ON op.oid = o.amopopr\n"
+ " LEFT JOIN pg_catalog.pg_proc p ON p.oid = op.oprcode\n");
if (access_method_pattern)
{
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 3bbe4c5f97..7632f252c6 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -5082,31 +5082,36 @@ List of access methods
btree | integer_ops | smallint, integer, bigint
(1 row)
-\dAo+ btree float_ops
- List of operators of operator families
- AM | Operator family | Operator | Strategy | Purpose | Sort opfamily
--------+-----------------+---------------------------------------+----------+---------+---------------
- btree | float_ops | <(double precision,double precision) | 1 | search |
- btree | float_ops | <=(double precision,double precision) | 2 | search |
- btree | float_ops | =(double precision,double precision) | 3 | search |
- btree | float_ops | >=(double precision,double precision) | 4 | search |
- btree | float_ops | >(double precision,double precision) | 5 | search |
- btree | float_ops | <(real,real) | 1 | search |
- btree | float_ops | <=(real,real) | 2 | search |
- btree | float_ops | =(real,real) | 3 | search |
- btree | float_ops | >=(real,real) | 4 | search |
- btree | float_ops | >(real,real) | 5 | search |
- btree | float_ops | <(double precision,real) | 1 | search |
- btree | float_ops | <=(double precision,real) | 2 | search |
- btree | float_ops | =(double precision,real) | 3 | search |
- btree | float_ops | >=(double precision,real) | 4 | search |
- btree | float_ops | >(double precision,real) | 5 | search |
- btree | float_ops | <(real,double precision) | 1 | search |
- btree | float_ops | <=(real,double precision) | 2 | search |
- btree | float_ops | =(real,double precision) | 3 | search |
- btree | float_ops | >=(real,double precision) | 4 | search |
- btree | float_ops | >(real,double precision) | 5 | search |
-(20 rows)
+\dAo+ btree array_ops|float_ops
+ List of operators of operator families
+ AM | Operator family | Operator | Strategy | Purpose | Sort opfamily | Leak-proof
+-------+-----------------+---------------------------------------+----------+---------+---------------+------------
+ btree | array_ops | <(anyarray,anyarray) | 1 | search | | no
+ btree | array_ops | <=(anyarray,anyarray) | 2 | search | | no
+ btree | array_ops | =(anyarray,anyarray) | 3 | search | | no
+ btree | array_ops | >=(anyarray,anyarray) | 4 | search | | no
+ btree | array_ops | >(anyarray,anyarray) | 5 | search | | no
+ btree | float_ops | <(double precision,double precision) | 1 | search | | yes
+ btree | float_ops | <=(double precision,double precision) | 2 | search | | yes
+ btree | float_ops | =(double precision,double precision) | 3 | search | | yes
+ btree | float_ops | >=(double precision,double precision) | 4 | search | | yes
+ btree | float_ops | >(double precision,double precision) | 5 | search | | yes
+ btree | float_ops | <(real,real) | 1 | search | | yes
+ btree | float_ops | <=(real,real) | 2 | search | | yes
+ btree | float_ops | =(real,real) | 3 | search | | yes
+ btree | float_ops | >=(real,real) | 4 | search | | yes
+ btree | float_ops | >(real,real) | 5 | search | | yes
+ btree | float_ops | <(double precision,real) | 1 | search | | yes
+ btree | float_ops | <=(double precision,real) | 2 | search | | yes
+ btree | float_ops | =(double precision,real) | 3 | search | | yes
+ btree | float_ops | >=(double precision,real) | 4 | search | | yes
+ btree | float_ops | >(double precision,real) | 5 | search | | yes
+ btree | float_ops | <(real,double precision) | 1 | search | | yes
+ btree | float_ops | <=(real,double precision) | 2 | search | | yes
+ btree | float_ops | =(real,double precision) | 3 | search | | yes
+ btree | float_ops | >=(real,double precision) | 4 | search | | yes
+ btree | float_ops | >(real,double precision) | 5 | search | | yes
+(25 rows)
\dAo * pg_catalog.jsonb_path_ops
List of operators of operator families
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 3b3c6f6e29..d59eae1814 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1266,7 +1266,7 @@ drop role regress_partitioning_role;
\dAc brin pg*.oid*
\dAf spgist
\dAf btree int4
-\dAo+ btree float_ops
+\dAo+ btree array_ops|float_ops
\dAo * pg_catalog.jsonb_path_ops
\dAp+ btree float_ops
\dAp * pg_catalog.uuid_ops
--
2.34.1
