Re: don't mark indexes invalid unnecessarily

On 2018-Dec-05, Amit Langote wrote:

> Hi,
> 
> On 2018/12/04 7:50, Alvaro Herrera wrote:
> > While working on FKs pointing to partitioned tables, I noticed that in
> > PG11 we fail to produce a working dump in the case of a partitioned
> > table that doesn't have partitions.
> > 
> > The attached patch fixes that.
> 
> The fix makes sense.  I see that the fix is similar in spirit to:
> 
> commit 9139aa19423b736470f669e566f8ef6a7f19b801

Ooh, right, it's pretty much the same.  I wasn't aware of this commit,
thanks for pointing it out.

> > I patched it so that it continues
> > to work (and now it tests the correct thing).  To be precise, the test
> > was doing this:
> > 
> >  create table parted_conflict (a int, b text) partition by range (a);
> >  create table parted_conflict_1 partition of parted_conflict for values from (0) to (1000) partition by range (a);
> >  create unique index on only parted_conflict_1 (a);
> >  create unique index on only parted_conflict (a);
> >  alter index parted_conflict_a_idx attach partition parted_conflict_1_a_idx;
> >  create table parted_conflict_1_1 partition of parted_conflict_1 for values from (0) to (500);
> > 
> > with the expectation that the partition would not have a proper index on
> > which to run an INSERT INTO parted_conflict_1 ON CONFLICT (a), expecting
> > that partition parted_conflict_1_1 would not have the index.  But in
> > reality parted_conflict_1_1 does have the index (and it only fails
> > because the index in its parent is marked invalid).  So the patch moves
> > the CREATE TABLE parted_conflict_1_1 to before the indexes creation, so
> > that the partition really does not have the index, and then it gets the
> > expected error.
> 
> ... isn't the test really trying to show that invalid unique index on
> table mentioned in the insert command cannot be used to proceed with the
> on conflict clause, and so isn't broken per se?  But maybe I misunderstood
> you.

You're right, I misinterpreted one bit: I was thinking that when we
create the first partition parted_conflict_1_1, then the index
automatically created in it causes the index in parted_conflict_1 to
become valid.  But that's not so: the index remains invalid.  This seems
a bit broken in itself, but it's not really important because that's
precisely what's getting fixed by my patch, namely that the index
created ONLY is valid if there are no partitions.  So if we want the
index to appear invalid (to continue to reproduce the scenario), we need
to create the partition first.

Thanks for reviewing.

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