Hello.

At Wed, 1 Aug 2018 09:25:18 -0700, Andres Freund <and...@anarazel.de> wrote in 
<20180801162518.jnb2ql5dfmgwp...@alap3.anarazel.de>
> Hi,
> 
> The issue at [1] is caused by missing invalidations, and [2] seems like
> a likely candidate too. I wonder if it'd be good to have a relcache test
> mode akin to CLOBBER_CACHE_ALWAYS and RELCACHE_FORCE_RELEASE, that tries
> to ensure that we've done sufficiently to ensure the right invalidations
> are sent.
> 
> I think what we'd kind of want is to ensure that relcache entries are
> rebuilt at the earliest possible time, but *not* later. That'd mean
> they're out of date if there's missing invalidations. Unfortunately I'm
> not clear on how that'd be achievable?  Ideas?
> 
> The best I can come up with is to code some additional dependencies into
> CacheInvalidateHeapTuple(), and add tracking ensuring we've sent the
> right messages. But that seems somewhat painful and filled with holes.
> 
> [1] 
> http://archives.postgresql.org/message-id/CAKoxK%2B5fVodiCtMsXKV_1YAKXbzwSfp7DgDqUmcUAzeAhf%3DHEQ%40mail.gmail.com
> [2] https://www.postgresql.org/message-id/12259.1532117...@sss.pgh.pa.us

As for [1], it is not a issue on invalidation. It happens also if
the relation has any index and even drop is not needed. The
following steps are sufficient.

create table t( pk serial, t text );
insert into t( t ) values( 'hello' ), ('world');
create index idx_fake0 on t (pk);
create index idx_fake on t ( f_fake( pk ) ); -- ERROR

index_create() creates a new pg_index entry with indislive = true
before building it. So the planner (RelationGetIndexList) sees
the not-yet-populated index while planning the query in f_fake().

The attached patch fixes the issue, but I'm not sure this story
is applicable to [2].

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From eaa5de68ff27bd43a643089f8c5963e5cc3d20cc Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp>
Date: Thu, 2 Aug 2018 18:52:24 +0900
Subject: [PATCH] Don't use currently-building index to populate it

index_create() creates a pg_index entry with indislive = true before
populating it. If it is a function index where the function runs a
query on the parent relation, planner sees the just created entry and
tries to access the heap page that is not created yet.  This patch let
index_create() not to set indislive = true after population.
---
 src/backend/catalog/index.c | 55 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 8b276bc430..b561c8696d 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -124,6 +124,7 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
 					bool immediate,
 					bool isvalid,
 					bool isready);
+static void ActivateIndexRelation(Oid indexoid);
 static void index_update_stats(Relation rel,
 				   bool hasindex,
 				   double reltuples);
@@ -666,7 +667,7 @@ UpdateIndexRelation(Oid indexoid,
 	values[Anum_pg_index_indisvalid - 1] = BoolGetDatum(isvalid);
 	values[Anum_pg_index_indcheckxmin - 1] = BoolGetDatum(false);
 	values[Anum_pg_index_indisready - 1] = BoolGetDatum(isready);
-	values[Anum_pg_index_indislive - 1] = BoolGetDatum(true);
+	values[Anum_pg_index_indislive - 1] = BoolGetDatum(false);
 	values[Anum_pg_index_indisreplident - 1] = BoolGetDatum(false);
 	values[Anum_pg_index_indkey - 1] = PointerGetDatum(indkey);
 	values[Anum_pg_index_indcollation - 1] = PointerGetDatum(indcollation);
@@ -693,6 +694,55 @@ UpdateIndexRelation(Oid indexoid,
 	heap_freetuple(tuple);
 }
 
+/* ----------------------------------------------------------------
+ *		ActivateIndexRelation
+ *
+ * Publish index by marking it "relislive"
+
+ *  UpdateIndexRelation builds an index relation with relislive = false so as
+ *  not to be used by the quieries used to build the index. This function
+ *  marks the index as "live" so that it can be used hereafter.
+ *  ----------------------------------------------------------------
+ */
+static void
+ActivateIndexRelation(Oid indexoid)
+{
+	Relation	indrel;
+	SysScanDesc	sdesc;
+	ScanKeyData	skey;
+	HeapTuple	tup;
+
+	ScanKeyInit(&skey,
+				Anum_pg_index_indexrelid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(indexoid));
+	indrel = heap_open(IndexRelationId, RowExclusiveLock);
+	sdesc = systable_beginscan(indrel, InvalidOid, true, NULL, 1, &skey);
+
+	/*
+	 * We must see one and only one entry for the key. But don't bother
+	 * checking that.
+	 */
+	while (HeapTupleIsValid(tup = systable_getnext(sdesc)))
+	{
+		Datum values[Natts_pg_index];
+		bool nulls[Natts_pg_index];
+		bool replaces[Natts_pg_index];
+
+		MemSet(values, 0, sizeof(values));
+		MemSet(nulls, 0, sizeof(nulls));
+		MemSet(replaces, 0, sizeof(replaces));
+		values[Anum_pg_index_indislive] = BoolGetDatum(true);
+		replaces[Anum_pg_index_indislive] = true;
+		tup = heap_modify_tuple(tup, RelationGetDescr(indrel),
+								values, nulls, replaces);
+		CatalogTupleUpdate(indrel, &tup->t_self, tup);
+		heap_freetuple(tup);
+	}
+	systable_endscan(sdesc);
+	heap_close(indrel, RowExclusiveLock);
+}
+
 
 /*
  * index_create
@@ -1188,6 +1238,9 @@ index_create(Relation heapRelation,
 					true);
 	}
 
+	/* let queries use this index */
+	ActivateIndexRelation(indexRelationId);
+
 	/*
 	 * Close the index; but we keep the lock that we acquired above until end
 	 * of transaction.  Closing the heap is caller's responsibility.
-- 
2.16.3

Reply via email to