Date: Sun, 15 Sep 2019 22:02:51 +0200
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Arseny Sher <a.sher@postgrespro.ru>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: (Re)building index using itself or another index of the same
 table
Message-ID: <20190915200251.tusdmm6hrus5ngj2@development>
References: <87d0g5d0tm.fsf@ars-thinkpad>
 <21174.1568300908@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <21174.1568300908@sss.pgh.pa.us>
User-Agent: NeoMutt/20180716
Precedence: bulk

On Thu, Sep 12, 2019 at 11:08:28AM -0400, Tom Lane wrote:
>Arseny Sher <a.sher@postgrespro.ru> writes:
>> A problem of similar nature can be reproduced with the following
>> stripped-down scenario:
>
>> CREATE TABLE pears(f1 int primary key, f2 int);
>> INSERT INTO pears SELECT i, i+1 FROM generate_series(1, 100) i;
>> CREATE OR REPLACE FUNCTION pears_f(i int) RETURNS int LANGUAGE SQL IMMUTABLE AS $$
>>   SELECT f1 FROM pears WHERE pears.f2 = 42
>> $$;
>> CREATE index ON pears ((pears_f(f1)));
>
>We've seen complaints about this sort of thing before, and rejected
>them because, as you say, that function is NOT immutable.  When you
>lie to the system like that, you should not be surprised if things
>break.
>
>> There is already a mechanism which prevents usage of indexes during
>> reindex -- ReindexIsProcessingIndex et al. However, to the contrary of
>> what index.c:3664 comment say, these protect only indexes on system
>> catalogs, not user tables: the only real caller is genam.c.
>> Attached patch extends it: the same check is added to
>> get_relation_info. Also SetReindexProcessing is cocked in index_create
>> to defend from index self usage during creation as in stripped example
>> above. There are some other still unprotected callers of index_build;
>> concurrent index creation doesn't need it because index is
>> 'not indisvalid' during the build, and in RelationTruncateIndexes
>> table is empty, so it looks like it can be omitted.
>
>I have exactly no faith that this fixes things enough to make such
>cases supportable.  And I have no interest in opening that can of
>worms anyway.  I'd rather put in some code to reject database
>accesses in immutable functions.
>

Same here. My hunch is a non-trivaial fraction of applications using
this "trick" is silently broken in various subtle ways.

>> One might argue that function selecting from table can hardly be called
>> immutable, and immutability is required for index expressions. However,
>> if user is sure table contents doesn't change, why not?
>
>If the table contents never change, why are you doing VACUUM FULL on it?
>

It's possible the columns referenced by the index expression are not
changing, but some additional columns are updated.

regards

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