From: Japin Li <japinli@hotmail.com>
To: Akshay Joshi <akshay.joshi@enterprisedb.com>
Cc: Ilmar Y <tanswis42@gmail.com>,  pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE
 POLICY statement
In-Reply-To: 
 <CANxoLDe7xiCWY-UEmzoK_uQdKh3PPNcGXJ3qdzFy3c0o_F+P_Q@mail.gmail.com>
	(Akshay Joshi's message of "Fri, 29 May 2026 12:20:54 +0530")
References: 
 <CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail.gmail.com>
	<CANxoLDffrZGRTGpW_sPQ-hPEYs0hgjaFgJQh3PJFpPu5Zsbgvg@mail.gmail.com>
	<177997571870.313758.10720313850275742354.pgcf@coridan.postgresql.org>
	<CANxoLDe7xiCWY-UEmzoK_uQdKh3PPNcGXJ3qdzFy3c0o_F+P_Q@mail.gmail.com>
User-Agent: mu4e 1.14.1; emacs 30.2
Date: Fri, 29 May 2026 15:38:32 +0800
Message-ID: 
 <SY7PR01MB1092118DD149062E230159D89B6162@SY7PR01MB10921.ausprd01.prod.outlook.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: SY7PR01MB10921:EE_|SY4PR01MB6297:EE_
X-MS-Office365-Filtering-Correlation-Id: ee301430-b3cd-4dc0-da2a-08debd554b53
X-Microsoft-Antispam: 
	BCL:0;ARA:14566002|22091999003|24121999003|5072599009|23021999003|15080799012|6090799003|24021099003|11091999009|19110799012|8060799015|12121999013|440099028|3412199025|26121999007|52005399003|40105399003;
X-Microsoft-Antispam-Message-Info: 
	=?utf-8?B?ZDdFeTNKR1JwSks2TmQvV01iaGR0K3lhZjFDRDdzeDBGdUgzcnFKSGVkZFd6?=
 =?utf-8?B?KzhFZFhvY1NsUGx1WW8rK1NSYXJKaExiSnlxdVdteDhyaGh0c0V0aU1CVFp1?=
 =?utf-8?B?b3BCcklzUUV3WHVnQjFZNHc2aGJjYVZKVGVCSVlkT3UreGdERy9zYno2L1dF?=
 =?utf-8?B?cGkzUU1qbW1yTXMzVCtJdjhyeUpESjRESWV6U3VaOHFISktkKzRqUGtFKzlH?=
 =?utf-8?B?REU4RjJHQnVjZnVBbWE1ZlZSK2g5bjJ3V0xENE5wZWJETnpmSDZxNXB1L1dt?=
 =?utf-8?B?ZnhUQVJwcUpDVmJ4b1pwMFBwNDZMZ3Qzc1hoYVVPQ1AzVktDclJsS2V4Y2R5?=
 =?utf-8?B?OGRid003MURRRUNVWU56c3M0eHFUYU9ZOXdsaEdoWW1MRUxYbmZ2Qi95bWRH?=
 =?utf-8?B?UWZEcWVWTHExWE5wRzlvSXA4dXN6cXRMTEdIZldxbERkWmtEWUNnQWIzbEh3?=
 =?utf-8?B?djBFZEVMd3Q5UVNNVWVPMTcwUU45TFdpWkF0KzZ6ZDExK1hvdm41NVBkeHQz?=
 =?utf-8?B?YmMxdStvd1hCOXpmelQ0bmZLb2RCS0hNamxqcEIzTDhFcHM5SzQyUXF3QXlU?=
 =?utf-8?B?d3NaQ2YyT0JRRzhCSnpmcDZ0NzQyWWpnN08rZ09Ta1JjUTBlcHEwbnNxWnZT?=
 =?utf-8?B?SHNOa04yaGlJb1pVNjA1WlpiNGtENTBoTmlYU242OGtSM0t1ODQxczdlS2ta?=
 =?utf-8?B?SVk1K0pUUkhWejEzbmpiaGN5bnI5cnFGYmFaQm1JaGRNbVVpN0ZnODV5ajlM?=
 =?utf-8?B?QkwwVVROZnQxMUJ2dUlOM1JjSitJVlN1bmMwYmtaNUdHQjZGNnZhRmgxb1gy?=
 =?utf-8?B?UzgrMHNianllUElmZTQ0eVhpcHFVWUFuZjdnbjdmR0NxTk96bTUxSjVUSzh6?=
 =?utf-8?B?ek9JMy83SG5Feis3VGZ3M1lWU3FRNUJ6aFVrSFdtZHNoMjJGcE5WTGVEMmJw?=
 =?utf-8?B?cmlSQWNaZHlyS3Q3VVdKczhBZ1dKNkd1WDRudDk2L1JZdWZ2Rys4M2ZkdS8x?=
 =?utf-8?B?eEc0V2JhODk4M1JJdGhUeWF3QkNaVWoxOUgxZ1d6cWZzWVFzenlOOXZBc09p?=
 =?utf-8?B?TWk2UUZTSWxpTlVQd1IvUTNJNnkyQ1R2VHptWjRxM0UyY25mSmwzYmJBd2hR?=
 =?utf-8?B?NlZtT2xUWkdXRklwMDlRZXQzeWhySkJwOVFLNUR6ZERiVVlPRXVCTjhNVU9X?=
 =?utf-8?B?alNHKzF4OGVZRHFVMVRQUUZNRUhrMTNaMWduUTdWNnFSMFNndW5GeGFVcktY?=
 =?utf-8?B?L3pHc1U3c0FDMk5obFQyUmFZNFJ0RUtZc1Q4cllMbHQyQlZHNjkyUm5xRGZS?=
 =?utf-8?B?VmtsaWJMNzI0K0dYNmRPNFUxQitSWkdCSXVMK1pmZTlReUpSZDFZMk5RSDN2?=
 =?utf-8?B?MU1EaE9UR3V4WHkraThVR3hwVGFlN1NOWUI3dXFYYjJYUStwOGNNRWIra3ZE?=
 =?utf-8?B?c0FNYklNRk1MWk8yRHVYanFUUjhYQm1CdUhQWkVsTVJtVDVIVml5QzZhSDVs?=
 =?utf-8?Q?zTuQnSN4XS/YXTbN0DttnMRITuY?=
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?U3NhdURjVnozVDk5WUQ3Y2RRSCtuNXE1MkIzQzVOR3N4WThhb0JrTTZzTzAv?=
 =?utf-8?B?OE5BTzNUK3VaSzc1T2ZYc2laSWJNdHFnOHgwU0pDWFZQb1dWWTBHa2dkS2dZ?=
 =?utf-8?B?YWh6ekFCTzF6TkJlSEliVmVmZWpyVnlxTTRlNmdhWjQ4NVRvNStmcnN2QmdF?=
 =?utf-8?B?OGlsYnRyZ3RrdlFBNE5yK0l1dm5nOFFrNmR6S081R3p1REE5WTcvWXozSE9w?=
 =?utf-8?B?TFppQklDRFR3SXNmZFZIcDlkQnpDMGpHS2ZkOTVoZ0RMUWFTaFQ2YnNDUzJ2?=
 =?utf-8?B?RkJLMDJsRml2c2JGbTVFa2h0N3B5L1hQZFl0Y0FtWSt1TlA3NFhmblIxNTY2?=
 =?utf-8?B?V1RQM0xmYzFKV3lleUY3WWIwODg5a0VleWRCZ0NBem9LUDN2bmtocFZKL291?=
 =?utf-8?B?dUhzYmVUYmRveUVZdHlnZVdvd0JDN0dKYnhWekRDS2wydmUxc2tZSzcyRW4z?=
 =?utf-8?B?Z3VZbDVTc29MYkt6dmZLY0FwSVBxQnUxUHBIazVPY0JjbXpjejJocy9sOEtD?=
 =?utf-8?B?S1hLS3NzTldwdDB0VW1RY1EzeDFhTVM5c3RHVEtvUlM4NnZ1cW8yZFZkdnhu?=
 =?utf-8?B?T3J4dTMxMVJoRlIrUUVteVExQTZIWUFGd2ZQQzVTWHIreWg5dXFqMVZaaW9a?=
 =?utf-8?B?OUYxb1cvN21wVVZUaXE5S2syR3V6VUxCMUZ5dFppalZiRktiUWF3NXIvTWZL?=
 =?utf-8?B?S2VlVHlVTVRLZDh0SkEzODBES0REbUgwRHMxcVJOWk4xNHZhTit3RmE2eWdu?=
 =?utf-8?B?VU10SXhUcFc1RTBUVmdwN1ZIa05OMWtMWXplbk5QSmpjSXZnWEsrRHlFcml6?=
 =?utf-8?B?U3VidG1SUWZZdWp6enltOEZUc2NFSzdCNG1PVXFqeXQvWEpsS2NWRXBpcW5C?=
 =?utf-8?B?eXBMcmQ5RHV4MXd1eDBxalZZS0VnZDVmRTJGU1ozZDRWVHhQSDVEVDAySWlW?=
 =?utf-8?B?Zy9GV20xeU9sVFN0bnkwQjdIekdKZGpiYnpYMmIrMitnN21OS3hvK0hIRVhO?=
 =?utf-8?B?OW5Jb3BSOFhsS08zT25ub0hKa1BOSll2Yk9lSzgwSmFVYXhYK3M2NjY2R3hZ?=
 =?utf-8?B?dWVXNEY3SkJDVDAveFVwc2xoeW9vVjViZEVDTGkyV0hCNjVueHAzOUZDeHpQ?=
 =?utf-8?B?a3pLVUZnN1ErY1ZVK0g5cjQ0aDZFYjRJNUZ4aGtDTERQSzc1MGF0d0c3cXEv?=
 =?utf-8?B?c0d5THBEcUxtbCtmNXZXdk1QMWVjUU8rb3Z2QmxGalFtdVZCeHk2R3phelhW?=
 =?utf-8?B?K0ZxY3Azc2pLZDliOWQvWlp1MzBTMTlDMUx3WFY4NUdNOGR0TnYzQzI3TUIr?=
 =?utf-8?B?TzIyRkgzSFNqVWE5WXlPVkdWN2htRVhFdDI3QUlnV3FoaHBPTStSQ0hPK200?=
 =?utf-8?B?M0I5Nm53c0wvMThYVjczQVJGT1JkbG5CYmI4WEd2N2RFaTdPNGwzUzNLU1Ez?=
 =?utf-8?B?dGRPQVRLa1NYTWRJRnl4WXo1OGt4NC8zMEdzdXlMMVp4MFRaSHJJNVJuaWVt?=
 =?utf-8?B?cHBnUWVHSjZrenlrQ2RzRTFrVHRlTERLUkFRNTBUNlFFaEo1WmdGRmxUYzg0?=
 =?utf-8?B?UW1UcjhhU0J1TXF2VXNEWEovQUhJRGVtNWdIS2YzZjNGUXRzS3JadE83aHNQ?=
 =?utf-8?B?UVNGWHlCNEdZbHRxYkdSOTQrck9HNllZQytnTUhIN1c5S3JCYXZnWGR6eDlG?=
 =?utf-8?B?bFZvT2k1MVZDeFdiVFhRQjQxbStkeWpldHV2Q0RhZVZFekJITWRSY2RuMUZJ?=
 =?utf-8?B?ZGVSa3VtNm0vM1M5bVRLWGh6M09nRU1DMkRLVWdnYWlLNHI2YUpYYnVzYmY0?=
 =?utf-8?B?Rmg0ZndaZWxyWTJuS1RZbE1rRWxOa0JJNmFoOHRkTkxIQ0dsVXRXcGluRHYv?=
 =?utf-8?Q?5YxD/ufdNNVG7?=
X-OriginatorOrg: sct-15-20-9412-4-msonline-outlook-feddd.templateTenant
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 ee301430-b3cd-4dc0-da2a-08debd554b53
X-MS-Exchange-CrossTenant-AuthSource: SY7PR01MB10921.ausprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 May 2026 07:38:37.0783
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 
	00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SY4PR01MB6297
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: 
 <https://www.postgresql.org/message-id/SY7PR01MB1092118DD149062E230159D89B6162%40SY7PR01MB10921.ausprd01.prod.outlook.com>
Precedence: bulk

On Fri, 29 May 2026 at 12:20, Akshay Joshi <akshay.joshi@enterprisedb.com> =
wrote:
> Thanks for the reviews.=20
>
> My original patch (v9) was actually correct. After considering Japin's re=
view comment, I initially thought the extra
> parentheses weren't necessary, but they are indeed required for handling =
boolean values properly in non-pretty mode too,
> so I kept them in USING (%s) / WITH CHECK (%s) for both modes.
>

My bad!  I had not considered this situation.

> `pg_get_expr()` only adds outer parentheses for composite expressions (vi=
a the deparsers for `OpExpr`, `BoolExpr`, etc.).
> For atomic top-level nodes like `Const`, `Var`, `current_user`, `NULL`, e=
tc.=20
> For example:
>
>     CREATE POLICY p ON t USING (true);
>     SELECT pg_get_policy_ddl('t', 'p');  -- previously: ... USING true;  =
(syntax error)
>
> This is exactly why `pg_dump` always wraps the expression unconditionally=
; see `src/bin/pg_dump/pg_dump.c`:4473-4477:
>
>     if (polinfo->polqual !=3D NULL)
>         appendPQExpBuffer(query, " USING (%s)", polinfo->polqual);
>     if (polinfo->polwithcheck !=3D NULL)
>         appendPQExpBuffer(query, " WITH CHECK (%s)", polinfo->polwithchec=
k);
>
> I've also added a round-trip regression test with `USING (true)` / `WITH =
CHECK (false)` that captures the generated DDL,
> drops the policies, re-executes the DDL, and verifies the policies are re=
created.=20
>
> v11 Patch attached for review.
>
> On Thu, May 28, 2026 at 7:12=E2=80=AFPM Ilmar Y <tanswis42@gmail.com> wro=
te:
>
>  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 statemen=
t
>  can be executed again.
>
>  The patch applies cleanly on current master at
>  8a86aa313a714adc56c74e4b08793e4e6102b5ca.
>
>  git diff --check reports no issues.
>
>  I built with:
>
>  ./configure --prefix=3D"$PWD/pg-install" --without-readline --without-zl=
ib --without-icu
>  make -s -j8
>  make -s install
>
>  make -C src/test/regress check TESTS=3Drowsecurity
>
>  ended up running the full parallel_schedule in this makefile; all 245 te=
sts
>  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 boolea=
n
>  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 d=
dl;
>
>  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 m=
ode
>  too, or the tests should include a round-trip execution check for genera=
ted
>  DDL with simple boolean expressions.
>
>  I used two small SQL reproducers for the manual checks; the complete rep=
ro is
>  included above.
>
>  I have not reviewed the broader pg_get_*_ddl API design or every possibl=
e
>  policy expression form.
>
>  Regards,
>  Ilmar Yunusov
>
>  The new status of this patch is: Waiting on Author

--=20
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.