Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vBYkT-000XpA-Pv for pgsql-hackers@arkaria.postgresql.org; Wed, 22 Oct 2025 13:19:29 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1vBYkS-00GUKJ-IJ for pgsql-hackers@arkaria.postgresql.org; Wed, 22 Oct 2025 13:19:27 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vBYkS-00GUK3-63 for pgsql-hackers@lists.postgresql.org; Wed, 22 Oct 2025 13:19:27 +0000 Received: from mail-lf1-x135.google.com ([2a00:1450:4864:20::135]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vBYkN-003cTR-2H for pgsql-hackers@postgresql.org; Wed, 22 Oct 2025 13:19:25 +0000 Received: by mail-lf1-x135.google.com with SMTP id 2adb3069b0e04-591ec7af7a1so2427496e87.3 for ; Wed, 22 Oct 2025 06:19:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb.com; s=google; t=1761139162; x=1761743962; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=1NXBkMJjM/A4dRRT9ZfsUdq7DTtPup6hVDVTTb0j0HA=; b=EZ9pIttnf372lNR56GnxieywX1JWiCkKi7Wg1smCBLBN7rFqBvqM37djM3FQm6gZtl 3fo+rqR4eyxmNLnBp6tpn9NATw4qW24w2EQaHFZF2f3ZjZM4AtnC9CuKGMu1yTIyg33g uTSe2SJNu5W5qAP0b1MBWkXRoK/iJjIUzLLnyQpH9WJp/Aj1Rf7wHo/UJtfzw600DA+w qAlTj9AA8hhwwZfolxSNSigolMXbe7V1fIuD8B+PNJnjon6g+gGDpJyT9GZKOuLn0b2O QlhGqBEJk74D6DJcQ+so29hkrEjyZC3Ck8+uhwynUBXGPbKM7u4Z1+2d/QjpxaZdek1T muFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761139162; x=1761743962; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=1NXBkMJjM/A4dRRT9ZfsUdq7DTtPup6hVDVTTb0j0HA=; b=icmeF8JiWegkbCLjtr38SXRk3kH9pVC1vYIo6Z8pYSb3ph0mOdCvl0CMfVIFk64VoN Nn4F0kZnJYek3ymkTIORpWVk1L+X+ANUc2tmwS0nYUKIIqMW/10OGlnCfEh50cpSh6rZ enMsroPcmouk5PQ9umj2hUI5EhZoPv+lcPBGBzy85YgQWAF/Pg0Xw7yxHwKtzBFALMV8 XwpO35ZRjvyaQBMi5r+f2YOu73IinA/0Kkh7ZCHJaBcTs+n1Agxz14OXu1CJpYQFc0NV 3G/AAu34xFCJocTDf2eB8Y0GhH5m4JS0xYJjp6f9h0UvUCzzougEVqRL2pnAvcUz+vZt OFeA== X-Forwarded-Encrypted: i=1; AJvYcCVWJQy8Yfk3N6g9i/XhcFriLMuoDSNnFFFks54/W3jMAkshFJHYxMfdykgZDC2Y1twej0ufaTuo0lXE48GQ@postgresql.org X-Gm-Message-State: AOJu0YxdMjUDyCWoBCal4q7RYiyPAD6cG2LGS/41WKtg7Q97u395MmSO kdbEn+M5+c74yOxqtN9wuyS+XXP9BNIVPVtdDocFt2iVvXWZXK3mkUfvCgi4iGnSDhg9aLlb/x/ pmGIqWSBwEtlSFXlqIe7SWQssO5PwBmCsnQPYXC/Q X-Gm-Gg: ASbGncu9nCnyPt7dUwfPqxjKYOoXjudsRJuGuo8QZHq2SjHoItrka/6F62JHDI6Edos Mt9yvJRYH+aynZTZXmKr+Jg1Lg0vQmpgssIsnZVPcH4o0GS/V7p8lR1oY5o7eIgT//79ISZJQxn w22fCaIY7YbLqHlxuiUJVhovRtxEoLU/nRZwebHuc0XnugakIff7isDlY3JyOaEcbLWj+mjRbyN rGqFTxtUPe1k186ixkWJTOZWlsEdaXg4sF6YZ5f7oWqCOiEKKke44KV/kAWzkGHfSSqosXD6haH Bkfu1I4= X-Google-Smtp-Source: AGHT+IFw1W5L8+LbS+I08uPfyssLbh7cJV18jGJ5LWpYrU7pcjNAPsUcGGrz8buhCCMRFnuS+g70itv/l1Kh8XnDcSs= X-Received: by 2002:a05:6512:15a2:b0:591:c2b9:5be1 with SMTP id 2adb3069b0e04-591d8557d6cmr6120293e87.33.1761139161796; Wed, 22 Oct 2025 06:19:21 -0700 (PDT) MIME-Version: 1.0 References: <15389F54-0234-412A-ADEE-78B810A35E2B@gmail.com> In-Reply-To: <15389F54-0234-412A-ADEE-78B810A35E2B@gmail.com> From: Akshay Joshi Date: Wed, 22 Oct 2025 18:49:10 +0530 X-Gm-Features: AS18NWCi3gT3mGy4hEufFTwxyKlkzHW47X_NxEj31JCrefpVdtBHPanhy6J_VQA Message-ID: Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement To: Chao Li Cc: Philip Alger , pgsql-hackers Content-Type: multipart/alternative; boundary="00000000000023aa1c0641bf2a19" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000023aa1c0641bf2a19 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Oct 21, 2025 at 2:39=E2=80=AFPM Chao Li wr= ote: > > > > On Oct 16, 2025, at 20:50, Akshay Joshi > wrote: > > > > Please find attached the v3 patch, which resolves all compilation error= s > and warnings. > > > > On Thu, Oct 16, 2025 at 6:06=E2=80=AFPM Philip Alger wrote: > > Hi Akshay, > > > > > > As for the statement terminator, it=E2=80=99s useful to include it, whi= le > running multiple queries together could result in a syntax error. In my > opinion, there=E2=80=99s no harm in providing the statement terminator. > > However, I=E2=80=99ve modified the logic to add the statement terminato= r at the > end instead of appending to a new line. > > > > > > Regarding putting the terminator at the end, I think my original commen= t > got cut off by my poor editing. Yes, that's what I was referring to; no > need to append it to a new line. > > > > -- > > Best, Phil Alger > > > > 1 - ruleutils.c > ``` > + if (pretty) > + { > + /* Indent with tabs */ > + for (int i =3D 0; i < noOfTabChars; i++) > + { > + appendStringInfoString(buf, "\t"); > + } > ``` > > As you are adding a single char of =E2=80=98\t=E2=80=99, better to use > appendStringInfoChar() that is cheaper. > > 2 - ruleutils.c > ``` > + initStringInfo(&buf); > + > + targetTable =3D relation_open(tableID, AccessShareLock); > ``` > > Looks like only usage of opening the table is to get the table name. In > that case, why don=E2=80=99t just call get_rel_name(tableID) and store th= e table > name in a local variable? > Fixed the above review comments in the v4 patch. I have used generate_qualified_relation_name(tableID) instead of get_rel_name(tableID). > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > > --00000000000023aa1c0641bf2a19 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

On Tue, Oct 21, 202= 5 at 2:39=E2=80=AFPM Chao Li <= li.evan.chao@gmail.com> wrote:


> On Oct 16, 2025, at 20:50, Akshay Joshi <akshay.joshi@enterprisedb.com&= gt; wrote:
>
> Please find attached the v3 patch, which resolves all compilation erro= rs and warnings.
>
> On Thu, Oct 16, 2025 at 6:06=E2=80=AFPM Philip Alger <paalger0@gmail.com> wrote= :
> Hi Akshay,
>
>
> As for the statement terminator, it=E2=80=99s useful to include it, wh= ile running multiple queries together could result in a syntax error. In my= opinion, there=E2=80=99s no harm in providing the statement terminator. > However, I=E2=80=99ve modified the logic to add the statement terminat= or at the end instead of appending to a new line.
>
>
> Regarding putting the terminator at the end, I think my original comme= nt got cut off by my poor editing. Yes, that's what I was referring to;= no need to append it to a new line.
>
> --
> Best, Phil Alger
> <v3-0001-Add-pg_get_policy_ddl-function-to-reconstruct-CREATE.patch= >

1 - ruleutils.c
```
+=C2=A0 =C2=A0 =C2=A0 =C2=A0if (pretty)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0{
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Indent with tabs= */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for (int i =3D 0; i= < noOfTabChars; i++)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0appendStringInfoString(buf, "\t");
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
```

As you are adding a single char of =E2=80=98\t=E2=80=99, better to use appe= ndStringInfoChar() that is cheaper.

2 - ruleutils.c
```
+=C2=A0 =C2=A0 =C2=A0 =C2=A0initStringInfo(&buf);
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0targetTable =3D relation_open(tableID, AccessSh= areLock);
```

Looks like only usage of opening the table is to get the table name. In tha= t case, why don=E2=80=99t just call get_rel_name(tableID) and store the tab= le name in a local variable?

=C2=A0 =C2=A0 Fixed t= he above review comments in the v4 patch. I have used=C2=A0generate_qualified_relation_name(tableID)=C2=A0instead of get_rel_name(tableID).

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
ht= tps://www.highgo.com/




--00000000000023aa1c0641bf2a19--