public inbox for [email protected]  

help / color / mirror / Atom feed

From: Zsolt Parragi <[email protected]>
To: [email protected]
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Date: Thu, 11 Jun 2026 14:00:59 -0700
Message-ID: <CAN4CZFP8JTUGvYJAk1QoFB7xmqQTmc72_FOe38qsE9S=CcYD4g@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDco7z7Fj7_pj_3YQLvjq8ZH2Ai4c3GcOamed01fZhJ77Q@mail.gmail.com>
References: <CANxoLDfjQnhM=E6JSyYo9s9OdjqoN8s_3wE5yL=kaDu_X8j-dA@mail.gmail.com>
	<CANxoLDcxmXNHNyPt5v+LaBJVJmyt=j95D7TDE9SepoM7y15t-w@mail.gmail.com>
	<CAN4CZFNqf41TCg1ENccq6DAYQpArCbnKUSidHnRKaVMWGj5pBA@mail.gmail.com>
	<CANxoLDfL1AxL=k9SsRNReKQ-sJUSy6RvCqeog5HWckFxff=0Pg@mail.gmail.com>
	<CAN4CZFP5-fAGwxMCyKGePgp3N8NCpoFXkruO=5xjTC=gPwqYxw@mail.gmail.com>
	<CANxoLDc6UFMsyzbXKF8SpMoXdoKE=-yRPqtvyBMKKJ2zOUKX4A@mail.gmail.com>
	<CAN4CZFMhnZyQsdLC5iZNego77JHJyP6J_4CpWu=99vKHCd-vzg@mail.gmail.com>
	<CANxoLDco7z7Fj7_pj_3YQLvjq8ZH2Ai4c3GcOamed01fZhJ77Q@mail.gmail.com>

The new version seem to work correctly to me, I didn't find any
further issues. Now the main blockers seem to be the remaining TODOs
related to includes_triggers/includes_policies.

I only have some minor comments about code structuring:

+ * get_inheritance_parents
+ *		Return a List of parent OIDs for relid, ordered by inhseqno.
+ *
+ * find_inheritance_children() walks the opposite direction (parent->children),

Shouldn't this follow the same naming and parameter pattern and live
at the same place in pg_inherits?

+static char *
+lookup_qualified_relname(Oid relid)
+...
+static char *
+lookup_relname_for_emit(Oid relid, bool schema_qualified, Oid base_namespace)

Is lookup_qualified_relname needed? It is only called within
lookup_relname_for_emit, and it results in a double syscache lookup,
which could be avoided if these were a single function.

+		/* COMPRESSION clause, only if explicitly set on the column. */
+		if (CompressionMethodIsValid(att->attcompression))
+		{
+			const char *cm = NULL;
+
+			switch (att->attcompression)
+			{
+				case TOAST_PGLZ_COMPRESSION:
+					cm = "pglz";
+					break;
+				case TOAST_LZ4_COMPRESSION:
+					cm = "lz4";
+					break;
+			}
+			if (cm)
+				appendStringInfo(buf, " COMPRESSION %s", cm);
+		}

Isn't this basically GetCompressionMethodName(att->attcompression)?

+		/* STORAGE clause, only if it differs from the type's default. */
+		if (att->attstorage != get_typstorage(att->atttypid))
+		{
+			const char *storage = NULL;
+
+			switch (att->attstorage)
+			{
+				case TYPSTORAGE_PLAIN:
+...

And this seems like storage_name(att->attstorage) from tablecmds.c,
the only issue is that that's currently static

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]
  Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
  In-Reply-To: <CAN4CZFP8JTUGvYJAk1QoFB7xmqQTmc72_FOe38qsE9S=CcYD4g@mail.gmail.com>

* 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