On Fri, Aug 22, 2014 at 5:23 PM, Alvaro Herrera <alvhe...@2ndquadrant.com>
wrote:
>
> Fabrízio de Royes Mello wrote:
> > On Fri, Aug 22, 2014 at 4:45 PM, Alvaro Herrera <
alvhe...@2ndquadrant.com>
> > wrote:
>
> > > I pointed out, in the email just before pushing the patch, that
perhaps
> > > we should pass down the new relpersistence flag into finish_heap_swap,
> > > and from there we could pass it down to reindex_index() which is where
> > > it would be needed. I'm not sure it's worth the trouble, but I think
we
> > > can still ask Fabrizio to rework that part.
>
> > I can rework this part if it's a real concern.
>
> I guess we can make a better assessment by seeing what it would take.
> I'm afraid it will turn out to be really ugly.
>
> Now, there's some desire to have unlogged indexes on logged tables; I
> guess if we have that, then eventually there will also be a desire to
> swap individual indexes from logged to unlogged. Perhaps whatever fix
> we come up with here would pave the road for that future feature.
>
Álvaro,
I did a refactoring to pass down the relpersistence to "finish_heap_swap"
and then change the catalog inside the "reindex_index" instead of in the
rewrite table phase.
That way we can get rid the function ATChangeIndexesPersistence in the
src/backend/commands/tablecmds.c.
Thoughts?
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index ee10594..173f412 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1973,7 +1973,7 @@ index_build(Relation heapRelation,
* created it, or truncated twice in a subsequent transaction, the
* relfilenode won't change, and nothing needs to be done here.
*/
- if (heapRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
+ if (indexRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
!smgrexists(indexRelation->rd_smgr, INIT_FORKNUM))
{
RegProcedure ambuildempty = indexRelation->rd_am->ambuildempty;
@@ -3099,7 +3099,7 @@ IndexGetRelation(Oid indexId, bool missing_ok)
* reindex_index - This routine is used to recreate a single index
*/
void
-reindex_index(Oid indexId, bool skip_constraint_checks)
+reindex_index(Oid indexId, bool skip_constraint_checks, char relpersistence)
{
Relation iRel,
heapRelation;
@@ -3160,6 +3160,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks)
indexInfo->ii_ExclusionStrats = NULL;
}
+ /*
+ * Check if need to set the new relpersistence
+ */
+ if (iRel->rd_rel->relpersistence != relpersistence)
+ iRel->rd_rel->relpersistence = relpersistence;
+
/* We'll build a new physical relation for the index */
RelationSetNewRelfilenode(iRel, InvalidTransactionId,
InvalidMultiXactId);
@@ -3358,11 +3364,19 @@ reindex_relation(Oid relid, int flags)
foreach(indexId, indexIds)
{
Oid indexOid = lfirst_oid(indexId);
+ char relpersistence = rel->rd_rel->relpersistence;
if (is_pg_class)
RelationSetIndexList(rel, doneIndexes, InvalidOid);
- reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS));
+ /* Check for flags to force UNLOGGED or PERMANENT persistence */
+ if (flags & REINDEX_REL_FORCE_UNLOGGED)
+ relpersistence = RELPERSISTENCE_UNLOGGED;
+ else if (flags & REINDEX_REL_FORCE_PERMANENT)
+ relpersistence = RELPERSISTENCE_PERMANENT;
+
+ reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
+ relpersistence);
CommandCounterIncrement();
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index ff80b09..f285026 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -588,7 +588,8 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
*/
finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
swap_toast_by_content, false, true,
- frozenXid, cutoffMulti);
+ frozenXid, cutoffMulti,
+ OldHeap->rd_rel->relpersistence);
}
@@ -1474,7 +1475,8 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
bool check_constraints,
bool is_internal,
TransactionId frozenXid,
- MultiXactId cutoffMulti)
+ MultiXactId cutoffMulti,
+ char newrelpersistence)
{
ObjectAddress object;
Oid mapped_tables[4];
@@ -1518,6 +1520,12 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
reindex_flags = REINDEX_REL_SUPPRESS_INDEX_USE;
if (check_constraints)
reindex_flags |= REINDEX_REL_CHECK_CONSTRAINTS;
+
+ if (newrelpersistence == RELPERSISTENCE_UNLOGGED)
+ reindex_flags |= REINDEX_REL_FORCE_UNLOGGED;
+ else if (newrelpersistence == RELPERSISTENCE_PERMANENT)
+ reindex_flags |= REINDEX_REL_FORCE_PERMANENT;
+
reindex_relation(OIDOldHeap, reindex_flags);
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index fdfa6ca..6156fcc 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1678,7 +1678,7 @@ ReindexIndex(RangeVar *indexRelation)
RangeVarCallbackForReindexIndex,
(void *) &heapOid);
- reindex_index(indOid, false);
+ reindex_index(indOid, false, indexRelation->relpersistence);
return indOid;
}
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index d8d3c08..b2c0fc9 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -65,7 +65,7 @@ static char *make_temptable_name_n(char *tempname, int n);
static void mv_GenerateOper(StringInfo buf, Oid opoid);
static void refresh_by_match_merge(Oid matviewOid, Oid tempOid);
-static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap);
+static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence);
static void OpenMatViewIncrementalMaintenance(void);
static void CloseMatViewIncrementalMaintenance(void);
@@ -280,7 +280,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
Assert(matview_maintenance_depth == old_depth);
}
else
- refresh_by_heap_swap(matviewOid, OIDNewHeap);
+ refresh_by_heap_swap(matviewOid, OIDNewHeap, relpersistence);
return matviewOid;
}
@@ -761,10 +761,10 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid)
* swapping is handled by the called function, so it is not needed here.
*/
static void
-refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap)
+refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
{
finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true,
- RecentXmin, ReadNextMultiXactId());
+ RecentXmin, ReadNextMultiXactId(), relpersistence);
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3720a0f..0802d1e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -389,7 +389,6 @@ static void ATExecClusterOn(Relation rel, const char *indexName,
LOCKMODE lockmode);
static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
static bool ATPrepChangePersistence(Relation rel, bool toLogged);
-static void ATChangeIndexesPersistence(Oid relid, char relpersistence);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
char *tablespacename, LOCKMODE lockmode);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
@@ -3710,16 +3709,6 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
ATRewriteTable(tab, OIDNewHeap, lockmode);
/*
- * Change the persistence marking of indexes, if necessary. This
- * is so that the new copies are built with the right persistence
- * in the reindex step below. Note we cannot do this earlier,
- * because the rewrite step might read the indexes, and that would
- * cause buffers for them to have the wrong setting.
- */
- if (tab->chgPersistence)
- ATChangeIndexesPersistence(tab->relid, tab->newrelpersistence);
-
- /*
* Swap the physical files of the old and new heaps, then rebuild
* indexes and discard the old heap. We can use RecentXmin for
* the table's new relfrozenxid because we rewrote all the tuples
@@ -3731,7 +3720,8 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
false, false, true,
!OidIsValid(tab->newTableSpace),
RecentXmin,
- ReadNextMultiXactId());
+ ReadNextMultiXactId(),
+ persistence);
}
else
{
@@ -10799,48 +10789,6 @@ ATPrepChangePersistence(Relation rel, bool toLogged)
}
/*
- * Update the pg_class entry of each index for the given relation to the
- * given persistence.
- */
-static void
-ATChangeIndexesPersistence(Oid relid, char relpersistence)
-{
- Relation rel;
- Relation pg_class;
- List *indexes;
- ListCell *cell;
-
- pg_class = heap_open(RelationRelationId, RowExclusiveLock);
-
- /* We already have a lock on the table */
- rel = relation_open(relid, NoLock);
- indexes = RelationGetIndexList(rel);
- foreach(cell, indexes)
- {
- Oid indexid = lfirst_oid(cell);
- HeapTuple tuple;
- Form_pg_class pg_class_form;
-
- tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for relation %u",
- indexid);
-
- pg_class_form = (Form_pg_class) GETSTRUCT(tuple);
- pg_class_form->relpersistence = relpersistence;
- simple_heap_update(pg_class, &tuple->t_self, tuple);
-
- /* keep catalog indexes current */
- CatalogUpdateIndexes(pg_class, tuple);
-
- heap_freetuple(tuple);
- }
-
- heap_close(pg_class, RowExclusiveLock);
- heap_close(rel, NoLock);
-}
-
-/*
* Execute ALTER TABLE SET SCHEMA
*/
Oid
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index b6f03df..8b1d30a 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2983,6 +2983,7 @@ RelationSetNewRelfilenode(Relation relation, TransactionId freezeXid,
}
classform->relfrozenxid = freezeXid;
classform->relminmxid = minmulti;
+ classform->relpersistence = relation->rd_rel->relpersistence;
simple_heap_update(pg_class, &tuple->t_self, tuple);
CatalogUpdateIndexes(pg_class, tuple);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 006b180..74111b6 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -102,12 +102,15 @@ extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
-extern void reindex_index(Oid indexId, bool skip_constraint_checks);
+extern void reindex_index(Oid indexId, bool skip_constraint_checks,
+ char relpersistence);
/* Flag bits for reindex_relation(): */
#define REINDEX_REL_PROCESS_TOAST 0x01
#define REINDEX_REL_SUPPRESS_INDEX_USE 0x02
#define REINDEX_REL_CHECK_CONSTRAINTS 0x04
+#define REINDEX_REL_FORCE_UNLOGGED 0x08
+#define REINDEX_REL_FORCE_PERMANENT 0x10
extern bool reindex_relation(Oid relid, int flags);
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index f7730a9..0b7e877 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -33,6 +33,7 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
bool check_constraints,
bool is_internal,
TransactionId frozenXid,
- MultiXactId minMulti);
+ MultiXactId minMulti,
+ char newrelpersistence);
#endif /* CLUSTER_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
