Re: BUG #15114: logical decoding Segmentation fault

Hi Michael,

On 29/10/2018 02:54, Michael Paquier wrote:
> On Sun, Oct 28, 2018 at 11:59:49PM +0100, Petr Jelinek wrote:
>> I wrote my own patch to solve this which is quite similar to Alvaro's
>> except that it does not do the manual work to build indexes list as we
>> already have RelationGetReplicaIndex which does all the necessary work
>> and also uses the same rd_idattr for cache that
>> RelationGetIndexAttrBitmap does, it's better for performance and looks
>> safe to me.
>>
>> I attached both patch for PG11+ (ie master) and the backport patch for
>> PG10, they only differ in the line identified above as problematic.
>
> Hmm. I am studying this patch, and this visibly cannot be easily
> reproduced? I would have imagined that something like this would be
> enough, with a predicate index on the publisher side which is in charge
> of loading the :
> -- connect to publisher
> \c postgres postgres /tmp 5432
> create table aa (a int not null, b int);
> create function copy_int(a int) returns int as $$ select $1 $$
> language sql immutable;
> create index aai on aa (b) where copy_int(b) > 0;
> create unique index aai2 on aa (a);
> alter table aa replica identity using index aai2;
> create publication big_tables for table aa;
> insert into aa values (generate_series(1,100), 1);
>
> -- connect to subscriber
> \c postgres postgres /tmp 5433
> create table aa (a int not null, b int);
> create function copy_int(a int) returns int as $$ select $1 $$
> language sql immutable;
> create index aai on aa (b) where copy_int(b) > 0;
> create unique index aai2 on aa (a);
> alter table aa replica identity using index aai2;
> create subscription sub_name CONNECTION 'host=/tmp dbname=postgres
> user=postgres' PUBLICATION big_tables;
>
> Is there something I am missing?

It needs to be function that requires snapshot to be built, which yours
does not. This all happens because we try to inline the SQL functions
into expression so all the expression machinery for running the function
gets involved and if the function needs snapshot the snapshot will be
created (which is where the crash comes from).

The reason why it breaks for OP is that his IMMUTABLE function is not
actually IMMUTABLE, that's also why we don't see it reported much.

Given this I am not sure if we want to add test for this as it's
something one should not do because it has unpredictable side effects,
but if you think it will be useful anyway I can whip something out.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services