From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: pointless check in RelationBuildPartitionDesc
Date: Tue, 4 Sep 2018 13:50:25 -0300
Message-ID: <20180904165025.zhby7ahrbkp7jf5l@alvherre.pgsql> (raw)
In-Reply-To: <20180904125128.ntliwrygsylaar4b@alvherre.pgsql>
Proposed patch. Checking isnull in a elog(ERROR) is important, because
the column is not marked NOT NULL. This is not true for other columns
where we simply do Assert(!isnull).
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:ispartition.patch (text/plain, 3.7K) download
commit dc5f9933595c95471898e506e004a1799ea4eae5[m
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
AuthorDate: Tue Sep 4 13:24:25 2018 -0300
CommitDate: Tue Sep 4 13:27:30 2018 -0300
fix relpartbound accesses
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex f46af41b56..ffdd14b819 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -773,9 +773,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
InvalidOid,
typaddress);
- /* Store inheritance information for new rel. */- StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);-
/*
* We must bump the command counter to make the newly-created relation
* tuple visible for opening.
@@ -869,6 +866,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
heap_close(parent, NoLock);
}
+ /* Store inheritance information for new rel. */+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);+
/*
* Process the partitioning specification (if any) and store the partition
* key information into the catalog.
@@ -14705,7 +14705,9 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
(void) SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relpartbound,
&isnull);
- Assert(!isnull);+ if (isnull)+ elog(ERROR, "null relpartbound for relation %u",+ RelationGetRelid(partRel));
/* Clear relpartbound and reset relispartition */
memset(new_val, 0, sizeof(new_val));
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.cindex 9015a05d32..4ed9920618 100644--- a/src/backend/partitioning/partbounds.c+++ b/src/backend/partitioning/partbounds.c@@ -1641,8 +1641,9 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec,
datum = SysCacheGetAttr(RELOID, tuple,
Anum_pg_class_relpartbound,
&isnull);
+ if (isnull)+ elog(ERROR, "null relpartbound for relation %u", inhrelid);- Assert(!isnull);
bspec = (PartitionBoundSpec *)
stringToNode(TextDatumGetCString(datum));
if (!IsA(bspec, PartitionBoundSpec))
diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.cindex 115a9fe78f..2fc876b7d8 100644--- a/src/backend/utils/cache/partcache.c+++ b/src/backend/utils/cache/partcache.c@@ -302,23 +302,11 @@ RelationBuildPartitionDesc(Relation rel)
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for relation %u", inhrelid);
- /*- * It is possible that the pg_class tuple of a partition has not been- * updated yet to set its relpartbound field. The only case where- * this happens is when we open the parent relation to check using its- * partition descriptor that a new partition's bound does not overlap- * some existing partition.- */- if (!((Form_pg_class) GETSTRUCT(tuple))->relispartition)- {- ReleaseSysCache(tuple);- continue;- }-
datum = SysCacheGetAttr(RELOID, tuple,
Anum_pg_class_relpartbound,
&isnull);
- Assert(!isnull);+ if (isnull)+ elog(ERROR, "null relpartbound for partition %u" inhrelid);
boundspec = (Node *) stringToNode(TextDatumGetCString(datum));
/*
@@ -883,9 +871,8 @@ generate_partition_qual(Relation rel)
boundDatum = SysCacheGetAttr(RELOID, tuple,
Anum_pg_class_relpartbound,
&isnull);
- if (isnull) /* should not happen */- elog(ERROR, "relation \"%s\" has relpartbound = null",- RelationGetRelationName(rel));+ if (isnull)+ elog(ERROR, "null relpartbound for relation %u", RelationGetRelid(rel));
bound = castNode(PartitionBoundSpec,
stringToNode(TextDatumGetCString(boundDatum)));
ReleaseSysCache(tuple);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, Langote_Amit_f8@lab.ntt.co.jp, michael@paquier.xyz
Subject: Re: pointless check in RelationBuildPartitionDesc
In-Reply-To: <20180904165025.zhby7ahrbkp7jf5l@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox