On 2019/03/14 5:18, Tom Lane wrote:
> Robert Haas <robertmh...@gmail.com> writes:
>> On Wed, Mar 13, 2019 at 3:14 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> Meanwhile, who's going to take point on cleaning up rd_partcheck?
>>> I don't really understand this code well enough to know whether that
>>> can share one of the existing partitioning-related sub-contexts.
>
>> To your question, I think it probably can't share a context. Briefly,
>> rd_partkey can't change ever, except that when a partitioned relation
>> is in the process of being created it is briefly NULL; once it obtains
>> a value, that value cannot be changed. If you want to range-partition
>> a list-partitioned table or something like that, you have to drop the
>> table and create a new one. I think that's a perfectly acceptable
>> permanent limitation and I have no urge whatever to change it.
>> rd_partdesc changes when you attach or detach a child partition.
>> rd_partcheck is the reverse: it changes when you attach or detach this
>> partition to or from a parent.
>
> Got it. Yeah, it seems like the clearest and least bug-prone solution
> is for those to be in three separate sub-contexts. The only reason
> I was trying to avoid that was the angle of how to back-patch into 11.
> However, we can just add the additional context pointer field at the
> end of the Relation struct in v11, and that should be good enough to
> avoid ABI problems.
Agree that rd_partcheck needs its own context as you have complained in
the past [1].
I think we'll need to back-patch this fix to PG 10 as well. I've attached
patches for all 3 branches.
Thanks,
Amit
[1] https://www.postgresql.org/message-id/22236.1523374067%40sss.pgh.pa.us
diff --git a/src/backend/utils/cache/partcache.c
b/src/backend/utils/cache/partcache.c
index 2b55f25e75..5f0b4dbc0c 100644
--- a/src/backend/utils/cache/partcache.c
+++ b/src/backend/utils/cache/partcache.c
@@ -373,7 +373,12 @@ generate_partition_qual(Relation rel)
elog(ERROR, "unexpected whole-row reference found in partition
key");
/* Save a copy in the relcache */
- oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+ rel->rd_partcheckcxt = AllocSetContextCreate(CacheMemoryContext,
+
"partition constraint",
+
ALLOCSET_DEFAULT_SIZES);
+ MemoryContextCopyAndSetIdentifier(rel->rd_partcheckcxt,
+
RelationGetRelationName(rel));
+ oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt);
rel->rd_partcheck = copyObject(result);
MemoryContextSwitchTo(oldcxt);
diff --git a/src/backend/utils/cache/relcache.c
b/src/backend/utils/cache/relcache.c
index eba77491fd..383570340a 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1203,6 +1203,12 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
/* It's fully valid */
relation->rd_isvalid = true;
+ /*
+ * Initialized by generate_partition_qual() if it's ever called in this
+ * session.
+ */
+ relation->rd_partcheckcxt = NULL;
+
return relation;
}
@@ -2313,8 +2319,8 @@ RelationDestroyRelation(Relation relation, bool
remember_tupdesc)
MemoryContextDelete(relation->rd_partkeycxt);
if (relation->rd_pdcxt)
MemoryContextDelete(relation->rd_pdcxt);
- if (relation->rd_partcheck)
- pfree(relation->rd_partcheck);
+ if (relation->rd_partcheckcxt)
+ MemoryContextDelete(relation->rd_partcheckcxt);
if (relation->rd_fdwroutine)
pfree(relation->rd_fdwroutine);
pfree(relation);
@@ -5544,6 +5550,7 @@ load_relcache_init_file(bool shared)
rel->rd_partkey = NULL;
rel->rd_pdcxt = NULL;
rel->rd_partdesc = NULL;
+ rel->rd_partcheckcxt = NULL;
rel->rd_partcheck = NIL;
rel->rd_indexprs = NIL;
rel->rd_indpred = NIL;
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 54028515a7..c2eae7ef62 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -198,6 +198,9 @@ typedef struct RelationData
/* use "struct" here to avoid needing to include pgstat.h: */
struct PgStat_TableStatus *pgstat_info; /* statistics collection area */
+
+ /* Memory context to hold the content of rd_partcheck. */
+ MemoryContext rd_partcheckcxt;
} RelationData;
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 8219d05d83..a13e1185b0 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1883,7 +1883,12 @@ generate_partition_qual(Relation rel)
elog(ERROR, "unexpected whole-row reference found in partition
key");
/* Save a copy in the relcache */
- oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+ rel->rd_partcheckcxt = AllocSetContextCreate(CacheMemoryContext,
+
"partition constraint",
+
ALLOCSET_DEFAULT_MINSIZE,
+
ALLOCSET_DEFAULT_INITSIZE,
+
ALLOCSET_DEFAULT_MAXSIZE);
+ oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt);
rel->rd_partcheck = copyObject(result);
MemoryContextSwitchTo(oldcxt);
diff --git a/src/backend/utils/cache/relcache.c
b/src/backend/utils/cache/relcache.c
index e2be8ea28c..f44050b89b 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1365,6 +1365,12 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
/* It's fully valid */
relation->rd_isvalid = true;
+ /*
+ * Initialized by generate_partition_qual() if it's ever called in this
+ * session.
+ */
+ relation->rd_partcheckcxt = NULL;
+
return relation;
}
@@ -2403,8 +2409,8 @@ RelationDestroyRelation(Relation relation, bool
remember_tupdesc)
MemoryContextDelete(relation->rd_partkeycxt);
if (relation->rd_pdcxt)
MemoryContextDelete(relation->rd_pdcxt);
- if (relation->rd_partcheck)
- pfree(relation->rd_partcheck);
+ if (relation->rd_partcheckcxt)
+ MemoryContextDelete(relation->rd_partcheckcxt);
if (relation->rd_fdwroutine)
pfree(relation->rd_fdwroutine);
pfree(relation);
@@ -5674,6 +5680,7 @@ load_relcache_init_file(bool shared)
rel->rd_partkey = NULL;
rel->rd_pdcxt = NULL;
rel->rd_partdesc = NULL;
+ rel->rd_partcheckcxt = NULL;
rel->rd_partcheck = NIL;
rel->rd_indexprs = NIL;
rel->rd_indpred = NIL;
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 4bc61e5380..eea0403ed7 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -216,6 +216,9 @@ typedef struct RelationData
/* use "struct" here to avoid needing to include pgstat.h: */
struct PgStat_TableStatus *pgstat_info; /* statistics collection area */
+
+ /* Memory context to hold the content of rd_partcheck. */
+ MemoryContext rd_partcheckcxt;
} RelationData;
diff --git a/src/backend/utils/cache/partcache.c
b/src/backend/utils/cache/partcache.c
index 5757301d05..a39cec602f 100644
--- a/src/backend/utils/cache/partcache.c
+++ b/src/backend/utils/cache/partcache.c
@@ -908,7 +908,12 @@ generate_partition_qual(Relation rel)
elog(ERROR, "unexpected whole-row reference found in partition
key");
/* Save a copy in the relcache */
- oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+ rel->rd_partcheckcxt = AllocSetContextCreate(CacheMemoryContext,
+
"partition constraint",
+
ALLOCSET_DEFAULT_SIZES);
+ MemoryContextCopyAndSetIdentifier(rel->rd_partcheckcxt,
+
RelationGetRelationName(rel));
+ oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt);
rel->rd_partcheck = copyObject(result);
MemoryContextSwitchTo(oldcxt);
diff --git a/src/backend/utils/cache/relcache.c
b/src/backend/utils/cache/relcache.c
index 743d5ea61a..c1e8a89f25 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1242,6 +1242,12 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
/* It's fully valid */
relation->rd_isvalid = true;
+ /*
+ * Initialized by generate_partition_qual() if it's ever called in this
+ * session.
+ */
+ relation->rd_partcheckcxt = NULL;
+
return relation;
}
@@ -2285,8 +2291,8 @@ RelationDestroyRelation(Relation relation, bool
remember_tupdesc)
MemoryContextDelete(relation->rd_partkeycxt);
if (relation->rd_pdcxt)
MemoryContextDelete(relation->rd_pdcxt);
- if (relation->rd_partcheck)
- pfree(relation->rd_partcheck);
+ if (relation->rd_partcheckcxt)
+ MemoryContextDelete(relation->rd_partcheckcxt);
if (relation->rd_fdwroutine)
pfree(relation->rd_fdwroutine);
pfree(relation);
@@ -5628,6 +5634,7 @@ load_relcache_init_file(bool shared)
rel->rd_partkey = NULL;
rel->rd_pdcxt = NULL;
rel->rd_partdesc = NULL;
+ rel->rd_partcheckcxt = NULL;
rel->rd_partcheck = NIL;
rel->rd_indexprs = NIL;
rel->rd_indpred = NIL;
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 84469f5715..632c671234 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -188,6 +188,9 @@ typedef struct RelationData
/* use "struct" here to avoid needing to include pgstat.h: */
struct PgStat_TableStatus *pgstat_info; /* statistics collection area */
+
+ /* Memory context to hold the content of rd_partcheck. */
+ MemoryContext rd_partcheckcxt;
} RelationData;
