public inbox for [email protected]  

help / color / mirror / Atom feed

From: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Date: Tue, 23 Jun 2026 16:21:04 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com>
References: <CANxoLDejn2JFXF7kVMtcznW6XyessrJ7WToSaQau2GxBdAGSUg@mail.gmail.com>
	<[email protected]>
	<CANxoLDe_GOXyyZs7GN3VUJoT+o1DwqZGhid-TWWvS0D8d5ggYw@mail.gmail.com>

At Mon, 22 Jun 2026 18:10:53 +0530, Akshay Joshi <[email protected]> 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

view thread (54+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox