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


Reply via email to