pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: adding partitioned tables to publications
Date: Mon, 16 Mar 2020 13:49:26 +0100
Message-ID: <d760bb60-563c-dd01-9903-f9337d50e8a6@2ndquadrant.com> (raw)
In-Reply-To: <CA+HiwqFjo=vCXDdkwprf6ipaVeegCpW79n-Ds8BK5cNQsSey2A@mail.gmail.com>
References: <CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com>
	<7a23e60b-ffe8-bce1-5c44-db48d3bc163e@2ndquadrant.com>
	<CA+HiwqGZuouqjvWGv2MZ3EOyVej2_6Z=Yv2GHia4Mfk+YY_fHA@mail.gmail.com>
	<CA+HiwqGUZJYaJSM5cPKKFrFSJbTsDyJ9o4Vd=M2d7i5brKvGbA@mail.gmail.com>
	<b6d8519b-353f-1949-79b5-460a6712e20f@2ndquadrant.com>
	<CA+HiwqHScTtXD97yC_ZRLk7TXf+0HYH+UeDvACf2Q-LBH-2ctw@mail.gmail.com>
	<CA+FpmFfKmsNww_C5hWU-=wg9h3cyV7vZcCUKWrrhtLkteUtY4w@mail.gmail.com>
	<CA+HiwqE2ie6_T01fMzsf7c5=hCGoVdj2yTStCJ78zOmayMVWbw@mail.gmail.com>
	<825b31ea-ad9f-21d1-29b6-2a494513458d@2ndquadrant.com>
	<CA+HiwqEqm3xvnMHjstkrECrpVv9R7YgYYpRHHVrffSvA9Zrf9g@mail.gmail.com>
	<CA+HiwqGiFpkHxx0mc2zpdJLGzfqEaqsdqUZurNto9kbkwzcQ1w@mail.gmail.com>
	<CA+FpmFfve1TtLvj5CwpgEZrjNiFG-EWh4ywpbtrB7-UTkZo2gg@mail.gmail.com>
	<CA+HiwqHfV73E4aJxX6d79xSEZjp+stFKnD0+7OktPw2_tD=pYg@mail.gmail.com>
	<73a1877a-6f1d-9f2e-da51-bc2eb57e07d5@2ndquadrant.com>
	<CA+HiwqFjo=vCXDdkwprf6ipaVeegCpW79n-Ds8BK5cNQsSey2A@mail.gmail.com>

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?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Prepare-to-support-non-tables-in-publications.patch (text/plain, 4.9K) download From c52672ff1de4d7c55c4b50eb7986f45801ea4c60 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter@eisentraut.org> Date: Mon, 16 Mar 2020 13:27:51 +0100 Subject: [PATCH] Prepare to support non-tables in publications This by itself doesn't change any functionality but prepares the way for having relations other than base tables in publications. Make arrangements for COPY handling the initial table sync. For non-tables we have to use COPY (SELECT ...) instead of directly copying from the table, but then we have to take care to omit generated columns from the column list. Also, remove a hardcoded reference to relkind = 'r' and rely on pg_relation_is_publishable(), which matches what the publisher can actually publish and is correct even in cross-version scenarios. Discussion: https://www.postgresql.org/message-id/flat/CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com --- src/backend/replication/logical/tablesync.c | 43 ++++++++++++++++----- src/include/replication/logicalproto.h | 1 + 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 98825f01e9..358b0a3726 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -639,26 +639,27 @@ fetch_remote_table_info(char *nspname, char *relname, WalRcvExecResult *res; StringInfoData cmd; TupleTableSlot *slot; - Oid tableRow[2] = {OIDOID, CHAROID}; - Oid attrRow[4] = {TEXTOID, OIDOID, INT4OID, BOOLOID}; + Oid tableRow[] = {OIDOID, CHAROID, CHAROID, BOOLOID}; + Oid attrRow[] = {TEXTOID, OIDOID, INT4OID, BOOLOID}; bool isnull; int natt; + bool remote_is_publishable; lrel->nspname = nspname; lrel->relname = relname; /* First fetch Oid and replica identity. */ initStringInfo(&cmd); - appendStringInfo(&cmd, "SELECT c.oid, c.relreplident" + appendStringInfo(&cmd, "SELECT c.oid, c.relreplident, c.relkind," + " pg_relation_is_publishable(c.oid)" " FROM pg_catalog.pg_class c" " INNER JOIN pg_catalog.pg_namespace n" " ON (c.relnamespace = n.oid)" " WHERE n.nspname = %s" - " AND c.relname = %s" - " AND c.relkind = 'r'", + " AND c.relname = %s", quote_literal_cstr(nspname), quote_literal_cstr(relname)); - res = walrcv_exec(wrconn, cmd.data, 2, tableRow); + res = walrcv_exec(wrconn, cmd.data, lengthof(tableRow), tableRow); if (res->status != WALRCV_OK_TUPLES) ereport(ERROR, @@ -675,6 +676,13 @@ fetch_remote_table_info(char *nspname, char *relname, Assert(!isnull); lrel->replident = DatumGetChar(slot_getattr(slot, 2, &isnull)); Assert(!isnull); + lrel->relkind = DatumGetChar(slot_getattr(slot, 3, &isnull)); + Assert(!isnull); + remote_is_publishable = DatumGetBool(slot_getattr(slot, 4, &isnull)); + if (isnull || !remote_is_publishable) + ereport(ERROR, + (errmsg("table \"%s.%s\" on the publisher is not publishable", + nspname, relname))); ExecDropSingleTupleTableSlot(slot); walrcv_clear_result(res); @@ -696,7 +704,7 @@ fetch_remote_table_info(char *nspname, char *relname, lrel->remoteid, (walrcv_server_version(wrconn) >= 120000 ? "AND a.attgenerated = ''" : ""), lrel->remoteid); - res = walrcv_exec(wrconn, cmd.data, 4, attrRow); + res = walrcv_exec(wrconn, cmd.data, lengthof(attrRow), attrRow); if (res->status != WALRCV_OK_TUPLES) ereport(ERROR, @@ -765,8 +773,25 @@ copy_table(Relation rel) /* 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. + */ + appendStringInfo(&cmd, "COPY (SELECT "); + for (int i = 0; i < lrel.natts; i++) + { + appendStringInfoString(&cmd, quote_identifier(lrel.attnames[i])); + if (i < lrel.natts - 1) + appendStringInfoString(&cmd, ", "); + } + appendStringInfo(&cmd, " FROM %s) TO STDOUT", + quote_qualified_identifier(lrel.nspname, lrel.relname)); + } res = walrcv_exec(wrconn, cmd.data, 0, NULL); pfree(cmd.data); if (res->status != WALRCV_OK_COPY_OUT) diff --git a/src/include/replication/logicalproto.h b/src/include/replication/logicalproto.h index 2cc2dc4db3..4860561be9 100644 --- a/src/include/replication/logicalproto.h +++ b/src/include/replication/logicalproto.h @@ -49,6 +49,7 @@ typedef struct LogicalRepRelation char **attnames; /* column names */ Oid *atttyps; /* column types */ char replident; /* replica identity */ + char relkind; /* remote relation kind */ Bitmapset *attkeys; /* Bitmap of key columns */ } LogicalRepRelation; -- 2.25.0

view thread (86+ messages)  latest in thread

Message-ID: <d760bb60-563c-dd01-9903-f9337d50e8a6@2ndquadrant.com>
Permalink:  ../d760bb60-563c-dd01-9903-f9337d50e8a6@2ndquadrant.com/
Also on:    postgresql.org/message-id/d760bb60-563c-dd01-9903-f9337d50e8a6@2ndquadrant.com

 ·  · 

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: pgsql-hackers@postgresql.org
  Cc: peter.eisentraut@2ndquadrant.com, amitlangote09@gmail.com, rafia.pghackers@gmail.com
  Subject: Re: adding partitioned tables to publications
  In-Reply-To: <d760bb60-563c-dd01-9903-f9337d50e8a6@2ndquadrant.com>

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

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