Re: psql: Add leakproof field to \dAo+ meta-command results - Mailing list pgsql-hackers

On 2024-07-30 08:30 +0200, Yugo NAGATA wrote:
> On Tue, 30 Jul 2024 01:36:55 +0200
> Erik Wienhold <ewie@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.

LGTM, thanks.

> > 
> > 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 <nagata@sraoss.co.jp>

-- 
Erik