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 1vCXI9-00FMec-2D for pgsql-hackers@arkaria.postgresql.org; Sat, 25 Oct 2025 05:58:16 +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 1vCXI5-002O9O-Hk for pgsql-hackers@arkaria.postgresql.org; Sat, 25 Oct 2025 05:58:12 +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 1vCXI5-002O9G-6I for pgsql-hackers@lists.postgresql.org; Sat, 25 Oct 2025 05:58:12 +0000 Received: from mail-lf1-x132.google.com ([2a00:1450:4864:20::132]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vCXI0-0046sM-2u for pgsql-hackers@postgresql.org; Sat, 25 Oct 2025 05:58:10 +0000 Received: by mail-lf1-x132.google.com with SMTP id 2adb3069b0e04-592ee9a16adso4574066e87.0 for ; Fri, 24 Oct 2025 22:58:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb.com; s=google; t=1761371886; x=1761976686; 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=k3dUB0kZZVHIfBicPfQ0bK1J4Uo2t1/4cH6G+ydKb6Q=; b=WSXRjvbmICcZut1hYWFQcj4ZFrl3U1YltnSacDC+kbA9vW2S/d2uipt721GSqlWP8c ceVuJurGu4hn0zQVjnVpGatub6J0HLMducZi5imcThpN6bbwM9KN/rashM2wbltiyUz5 R3tNY2u9K7hsvW6pa5naJEnGL0L/Dwj+dlZqVvfxA1p9k8GdIEJc64Tj+UDl/WOV/b2I 8zZaYn48oaFWg/hEgraS2nE8Ui89DqA1uiiVlV5/K2VK+ZGWQ0w4g+altg7kZMvIHwZw fYh7DmDCxHC7cuuC5dmRAVgJjDZwlhbmHC+S5pOGiLN1VBSvAZWBWSkynV8jplsWhQY8 rQAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761371886; x=1761976686; 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=k3dUB0kZZVHIfBicPfQ0bK1J4Uo2t1/4cH6G+ydKb6Q=; b=hrPxsvjtrou6FghLmvQviWjrpo5mopk86tdlCftjDfqmpbuOmvbtRD8BTwdxIcNx5T BjFUis0hgilRuhcLr8P/b9kJPrEgRkUcaEba5GpF3BeVLntFp66vJBaIoGTixM+F+ZGq q+o9cAUDxsSuZAtGKiPbi0dr19R/8kA+BAvJ4UYMJlCp+f3h/jqKhAm9ff6n2P2rcUaQ 6sBbPKqe7aMlByeAP6jx1aH8Whj1wh+xzn0zUaO8UdxokEklH9YTqzLUdcW+0CsFIWjL P1S+NcXCPlre6a2O2xe0vxAPMZKCzHZU0yordeDYO/CmPaJHRFJZD8NQDW6ZpjNZDist xyUg== X-Forwarded-Encrypted: i=1; AJvYcCW2wR94DfHOeThv2rZqmLRJebMq+ZTiIuwFuPA7P0loK4opDTyxoMvmayG7MTTpjRgTzyaDMrmv6YBmZbH5@postgresql.org X-Gm-Message-State: AOJu0YxHl6ZFdhyHcvXf71BZIJMesaErCTZrc+5Aa8WcP4yQT5H6Csut OWwOFMlW8oZOXL4TmT0hXPgRVHAdmj04Jpwl9RpHwikfP1718CwcLb0AemVbfOj9P0vjLwhjQPb aLIxSYXgqq6VePvpNUrmliTpBvebq9F18Enijm/zd X-Gm-Gg: ASbGnctWcHnvcb+fwa3VWr3+x3Gd9DR6MqBLop8nW8SREHESwRHWOQv3SJVCvEokfbG BFSnXBUY/+xtul0E2DTGQyagFUCGKrNC2Y6iV4WN6+WXWcyaHBRmGiB/uG+6RMYJTnJSnlCiYq6 NgD8omVGh9uRjpYPvf5Lt2hmtI2Kj7cgvCnLn7zBsTEmZ73W2flEyBCBPzy9rZSPHFJYMxa/s2x S64C/K9UVFXaQJX+KgL5Sp3kcgQG2VPnZOPgXIuiZs0a6O8M33PafzSOY+RRQ== X-Google-Smtp-Source: AGHT+IHsf5GYzep4ysdKOtAvRC8P+TDbrl+HLk4Hul4/ZCeIypHA2foDHFRDwWMlXmtz0NO2yjVsula0PnbwCqqalxI= X-Received: by 2002:a05:6512:a84:b0:577:494e:ca63 with SMTP id 2adb3069b0e04-591d84e38a9mr9369378e87.12.1761371885574; Fri, 24 Oct 2025 22:58:05 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Sat, 25 Oct 2025 11:27:54 +0530 X-Gm-Features: AWmQ_bl0w_xqsUEVRq3QyI6cNNR6oWKx_-RdyIfJgE6vRsT_7B1Uj3RQYm3Zis8 Message-ID: Subject: Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement To: Philip Alger Cc: jian he , pgsql-hackers Content-Type: multipart/alternative; boundary="0000000000008eafd70641f55951" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000008eafd70641f55951 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Oct 23, 2025 at 12:19=E2=80=AFAM Philip Alger = wrote: > > > >>> The get_formatted_string function is needed. Instead of using multiple >> if statements for the pretty flag, it=E2=80=99s better to have a generic >> function. This will be useful if the pretty-format DDL implementation is >> accepted by the community, as it can be reused by other >> pg_get__ddl() DDL functions added in the future. >> >>> >>> in pg_get_triggerdef_worker, I found the below code pattern: >>> /* >>> * In non-pretty mode, always schema-qualify the target table name >>> for >>> * safety. In pretty mode, schema-qualify only if not visible. >>> */ >>> appendStringInfo(&buf, " ON %s ", >>> pretty ? >>> generate_relation_name(trigrec->tgrelid, NIL) : >>> generate_qualified_relation_name(trigrec->tgrelid)= ); >>> >>> maybe we can apply it too while construct query string: >>> "CREATE POLICY %s ON %s", >>> >> >> In my opinion, the table name should always be schema-qualified, which I >> have addressed in the v4 patch. The implementation of the pretty flag >> here differs from pg_get_triggerdef_worker, which is used only for the >> target table name. In my patch, the pretty flag adds \t and \n to each >> different clause (example AS, FOR, USING ...) >> >> > > I think that's where the confusion lies with adding `pretty` to the DDL > functions. The `pretty` flag set to `true` on other functions is used to > "not" show the schema name. But in the case of this function, it is used = to > format the statement. I wonder if the word `format` or `pretty_format` is= a > better name? `pretty` itself in the codebase isn't a very clear name > regardless. > In my opinion, we should first decide whether we want the DDL statement to be in a 'pretty' format or not. Personally, I believe it should be, since parsing a one-line DDL statement can be quite complex and difficult for users of this function. From a user=E2=80=99s perspective, having the entir= e DDL in a single line makes it harder to read and understand. The flag allows users to disable the pretty format by passing false. If the community agrees on this approach, we can then think about choosing a more appropriate word for it. > > -- > Best, > Phil Alger > --0000000000008eafd70641f55951 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

On Thu, Oct 23, 202= 5 at 12:19=E2=80=AFAM Philip Alger <paalger0@gmail.com> wrote:



The get_fo= rmatted_string function is needed. Instead of using multiple i= f statements for the pretty flag, it=E2=80=99s better t= o have a generic function. This will be useful if the pretty-format DDL imp= lementation is accepted by the community, as it can be reused by other pg_g= et_<object>_ddl() DDL functions added in the future.=C2=A0

in pg_get_triggerdef_worker, I found the below code pattern:
=C2=A0 =C2=A0 /*
=C2=A0 =C2=A0 =C2=A0* In non-pretty mode, always schema-qualify the target = table name for
=C2=A0 =C2=A0 =C2=A0* safety.=C2=A0 In pretty mode, schema-qualify only if = not visible.
=C2=A0 =C2=A0 =C2=A0*/
=C2=A0 =C2=A0 appendStringInfo(&buf, " ON %s ",
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0pretty ?
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0generate_relation_name(trigrec->tgrelid, NIL) :
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0generate_qualified_relation_name(trigrec->tgrelid));

maybe we can apply it too while construct query string:
"CREATE POLICY %s ON %s",

In my opinion,= the table name should always be schema-qualified, which=C2=A0I have addres= sed in the v4 patch. The implementation of the pretty flag her= e differs from pg_get_triggerdef_worker, which is used only fo= r the target table name. In my patch, the pretty flag adds \t and \n to each different clause (example AS, FOR,= USING ...)
=C2=A0

I think that's whe= re the confusion=C2=A0lies with=C2=A0adding `pretty` to the DDL functions. = The `pretty` flag set to `true` on other=C2=A0functions is used to "no= t" show the schema name. But in the=C2=A0case of=C2=A0this function,= =C2=A0it is used to format the statement. I wonder if the word `format` or = `pretty_format` is a better name? `pretty` itself in the codebase isn't= a very clear name regardless.

In my opinion, we= should first decide whether we want the DDL statement to be in a 'pret= ty' format or not. Personally, I believe it should be, since parsing a = one-line DDL statement can be quite complex and difficult for users of this= function. From a user=E2=80=99s perspective, having the entire DDL in a si= ngle line makes it harder to read and understand.=C2=A0The flag allows user= s to disable the pretty format by passing false.

If the community agrees on this approach, we can then think about choo= sing a more appropriate word for it.

--
Best,=C2=A0
Phil Alger
--0000000000008eafd70641f55951--