MIME-Version: 1.0
References: 
 <CANxoLDejn2JFXF7kVMtcznW6XyessrJ7WToSaQau2GxBdAGSUg@mail.gmail.com>
 <983C7B03-D9C3-4F15-8913-7616EC5E6414@gmail.com>
 <CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com>
 <20260623.162104.775158751399277188.horikyota.ntt@gmail.com>
In-Reply-To: <20260623.162104.775158751399277188.horikyota.ntt@gmail.com>
From: Akshay Joshi <akshay.joshi@enterprisedb.com>
Date: Tue, 23 Jun 2026 14:52:37 +0530
Message-ID: 
 <CANxoLDdq06oXRYuzYfn2gkn3yfoxp=nA14xu5p+qSEOzpHXEMQ@mail.gmail.com>
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE
 statements
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: li.evan.chao@gmail.com, zsolt.parragi@percona.com,
	pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="0000000000006c006f0654e84d48"
Archived-At: 
 <https://www.postgresql.org/message-id/CANxoLDdq06oXRYuzYfn2gkn3yfoxp%3DnA14xu5p%2BqSEOzpHXEMQ%40mail.gmail.com>
Precedence: bulk

--0000000000006c006f0654e84d48
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jun 23, 2026 at 12:51=E2=80=AFPM Kyotaro Horiguchi <horikyota.ntt@g=
mail.com>
wrote:

> At Mon, 22 Jun 2026 18:10:53 +0530, Akshay Joshi <
> akshay.joshi@enterprisedb.com> wrote in
> > The v9 patch is ready for review.
>
> I have not looked closely at the DDL generation logic itself, but I
> have a few comments on how pg_get_table_ddl handles its options.
>
> Since pg_get_table_ddl_internal() appears to copy these values into
> TableDdlContext almost immediately, I wonder whether TableDdlContext
> could be initialized by the caller instead.
>
> Using positional boolean arguments is probably fine when there are
> only a handful of options, but with around fifteen of them the current
> approach seems somewhat error-prone.
>
> It might also be clearer to initialize the default values first, and
> then override only the fields corresponding to explicitly specified
> options, rather than folding the default handling and option lookup
> into the same expression.
>

I assume that changing the implementation model, as I mentioned in my other
email, will solve this problem as well. We can drop the entire includes_*
family and only_foreign_keys, and replace them with two mutually exclusive
variadic keys:
include =3D> 'kind1,kind2,...' =E2=80=94 emit only these kinds
exclude =3D> 'kind1,kind2,...' =E2=80=94 emit everything except these kinds
>
>
> Regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

--0000000000006c006f0654e84d48
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Tue, Jun 23,=
 2026 at 12:51=E2=80=AFPM Kyotaro Horiguchi &lt;<a href=3D"mailto:horikyota=
.ntt@gmail.com">horikyota.ntt@gmail.com</a>&gt; wrote:<br></div><blockquote=
 class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px so=
lid rgb(204,204,204);padding-left:1ex">At Mon, 22 Jun 2026 18:10:53 +0530, =
Akshay Joshi &lt;<a href=3D"mailto:akshay.joshi@enterprisedb.com" target=3D=
"_blank">akshay.joshi@enterprisedb.com</a>&gt; wrote in <br>
&gt; The v9 patch is ready for review.<br>
<br>
I have not looked closely at the DDL generation logic itself, but I<br>
have a few comments on how pg_get_table_ddl handles its options.<br>
<br>
Since pg_get_table_ddl_internal() appears to copy these values into<br>
TableDdlContext almost immediately, I wonder whether TableDdlContext<br>
could be initialized by the caller instead.<br>
<br>
Using positional boolean arguments is probably fine when there are<br>
only a handful of options, but with around fifteen of them the current<br>
approach seems somewhat error-prone.<br>
<br>
It might also be clearer to initialize the default values first, and<br>
then override only the fields corresponding to explicitly specified<br>
options, rather than folding the default handling and option lookup<br>
into the same expression.<br></blockquote><div><br></div><div><span style=
=3D"background-color:transparent">I assume that changing the implementation=
 model, as I mentioned in my other email, will solve this problem as well. =
We can drop the entire includes_* family and only_foreign_keys, and replace=
 them with two mutually exclusive variadic keys:</span></div>include =3D&gt=
; &#39;kind1,kind2,...&#39; =E2=80=94 emit only these kinds<br>exclude =3D&=
gt; &#39;kind1,kind2,...&#39; =E2=80=94 emit everything except these kinds<=
blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l=
eft:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Regards,<br>
<br>
-- <br>
Kyotaro Horiguchi<br>
NTT Open Source Software Center<br>
</blockquote></div></div>

--0000000000006c006f0654e84d48--