On Wed, Mar 18, 2020 at 12:06 PM Amit Langote <[email protected]> wrote: > Hi Peter, > > On Mon, Mar 16, 2020 at 9:49 PM Peter Eisentraut > <[email protected]> wrote: > > > > I was trying to extract some preparatory work from the remaining patches > > and came up with the attached. This is part of your patch 0003, but > > also relevant for part 0004. The problem was that COPY (SELECT *) is > > not sufficient when the table has generated columns, so we need to build > > the column list explicitly. > > > > Thoughts? > > Thank you for that. > > + if (isnull || !remote_is_publishable) > + ereport(ERROR, > + (errmsg("table \"%s.%s\" on the publisher is not publishable", > + nspname, relname))); > > Maybe add a one-line comment above this to say it's an "not supposed > to happen" error or am I missing something? Wouldn't elog() suffice > for this? > > Other than that, looks good.
Wait, the following Assert in copy_table() should now be gone:
Assert(relmapentry->localrel->rd_rel->relkind == RELKIND_RELATION);
because just below it:
/* Start copy on the publisher. */
initStringInfo(&cmd);
- appendStringInfo(&cmd, "COPY %s TO STDOUT",
- quote_qualified_identifier(lrel.nspname, lrel.relname));
+ if (lrel.relkind == RELKIND_RELATION)
+ appendStringInfo(&cmd, "COPY %s TO STDOUT",
+ quote_qualified_identifier(lrel.nspname,
lrel.relname));
+ else
+ {
+ /*
+ * For non-tables, we need to do COPY (SELECT ...), but we can't just
+ * do SELECT * because we need to not copy generated columns.
+ */
By the way, I have rebased the patches, although maybe you've got your
own copies; attached.
--
Thank you,
Amit
v12-0003-Add-subscription-support-to-replicate-into-parti.patch
Description: Binary data
v12-0002-Some-refactoring-of-logical-worker.c.patch
Description: Binary data
v12-0004-Publish-partitioned-table-inserts-as-its-own.patch
Description: Binary data
