Date: Wed, 3 Jun 2020 22:25:00 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>,
	Michael Paquier <michael@paquier.xyz>,
	Masahiko Sawada <masahiko.sawada@2ndquadrant.com>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	sitnikov.vladimir@gmail.com,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: SIGSEGV from START_REPLICATION 0/XXXXXXX in XLogSendPhysical ()
 at walsender.c:2762
Message-ID: <20200604022500.GA23786@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200604013311.virtnjcrqj3fgs34@alap3.anarazel.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Jun-03, Andres Freund wrote:

> On 2020-06-03 18:27:12 -0400, Alvaro Herrera wrote:
> > On 2020-Jun-03, Andres Freund wrote:
> > > I don't think we should prohibit this. For one, it'd probably break some
> > > clients, without a meaningful need.
> > 
> > There *is* a need, namely to keep complexity down.  This is quite
> > convoluted, it's got a lot of historical baggage because of the way it
> > was implemented, and it's very difficult to understand.  The greatest
> > motive I see is to make this easier to understand, so that it is easier
> > to modify and improve in the future.
> 
> That seems like a possibly convincing argument for not introducing the
> capability, but doesn't seem strong enough to remove it.

This "capability" has never been introduced.  The fact that it's there
is just an accident.  In fact, it's not a capability, since the feature
(physical replication) is invoked differently -- namely, using a
physical replication connection.  JDBC uses a logical replication
connection for it only because they never realized that they were
supposed to do differently, because we failed to throw the correct
error message in the first place.

> > I don't think having a physical replication connection access catalog
> > data directly is a great idea.  We already have gadgets like
> > IDENTIFY_SYSTEM for physical replication that can do that, and if you
> > need particular settings you can use SHOW (commit d1ecd539477).  If
> > there was a strong need for even more than that, we can add something to
> > the grammar.
> 
> Those special case things are a bad idea, and we shouldn't introduce
> more.

What special case things?  The replication connection has never been
supposed to run SQL.  That's why we have SHOW in the replication
grammar.

> It's unrealistic that we can ever make that support everything,
> and since we already have to support the database connected thing, I
> don't see the point.

A logical replication connection is not supposed to be used for physical
replication.  That's just going to make more bugs appear.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services