Date: Tue, 23 Jun 2026 16:21:04 +0900 (JST)
Message-Id: <20260623.162104.775158751399277188.horikyota.ntt@gmail.com>
To: akshay.joshi@enterprisedb.com
Cc: li.evan.chao@gmail.com, zsolt.parragi@percona.com,
 pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE
 statements
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: 
 <CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com>
References: 
 <CANxoLDejn2JFXF7kVMtcznW6XyessrJ7WToSaQau2GxBdAGSUg@mail.gmail.com>
	<983C7B03-D9C3-4F15-8913-7616EC5E6414@gmail.com>
	<CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com>
User-Agent: Mew version 6.8 on Emacs 29.4
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20260623.162104.775158751399277188.horikyota.ntt%40gmail.com>
Precedence: bulk

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.

Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center