public inbox for [email protected]  
help / color / mirror / Atom feed
From: Ilmar Y <[email protected]>
To: [email protected]
Cc: Akshay Joshi <[email protected]>
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
Date: Thu, 28 May 2026 13:41:58 +0000
Message-ID: <177997571870.313758.10720313850275742354.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <CANxoLDffrZGRTGpW_sPQ-hPEYs0hgjaFgJQh3PJFpPu5Zsbgvg@mail.gmail.com>
References: <CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com>
	<CANxoLDffrZGRTGpW_sPQ-hPEYs0hgjaFgJQh3PJFpPu5Zsbgvg@mail.gmail.com>

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:       tested, failed
Spec compliant:           not tested
Documentation:            not tested

Hi,

I looked at v10, focused on whether the generated CREATE POLICY statement
can be executed again.

The patch applies cleanly on current master at
8a86aa313a714adc56c74e4b08793e4e6102b5ca.

git diff --check reports no issues.

I built with:

./configure --prefix="$PWD/pg-install" --without-readline --without-zlib --without-icu
make -s -j8
make -s install

make -C src/test/regress check TESTS=rowsecurity

ended up running the full parallel_schedule in this makefile; all 245 tests
passed, including rowsecurity.

I found one correctness issue in the generated non-pretty DDL.  The code
assumes that pg_get_expr_ext(..., false) already returns the parentheses
required by CREATE POLICY syntax, but that is not true for simple boolean
constants.

For example:

CREATE TABLE t(a int);
CREATE POLICY p_true ON t USING (true);
SELECT ddl FROM pg_get_policy_ddl('t', 'p_true', 'pretty', 'false') AS ddl;

returns:

CREATE POLICY p_true ON public.t USING true;

If I drop the policy and execute that generated statement, it fails:

ERROR:  syntax error at or near "true"
LINE 1: CREATE POLICY p_true ON public.t USING true;
                                               ^

The same issue reproduces for WITH CHECK:

CREATE POLICY p_check ON t FOR INSERT WITH CHECK (false);

is reconstructed as:

CREATE POLICY p_check ON public.t FOR INSERT WITH CHECK false;

and executing it fails at "false".

So I think USING and WITH CHECK need to be parenthesized in non-pretty mode
too, or the tests should include a round-trip execution check for generated
DDL with simple boolean expressions.

I used two small SQL reproducers for the manual checks; the complete repro is
included above.

I have not reviewed the broader pg_get_*_ddl API design or every possible
policy expression form.

Regards,
Ilmar Yunusov

The new status of this patch is: Waiting on Author


reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement
  In-Reply-To: <177997571870.313758.10720313850275742354.pgcf@coridan.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox