mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-12-27 08:39:28 +08:00
Fix assorted bugs in pg_get_partition_constraintdef().
It failed if passed a nonexistent relation OID, or one that was a non-heap relation, because of blindly applying heap_open to a user-supplied OID. This is not OK behavior for a SQL-exposed function; we have a project policy that we should return NULL in such cases. Moreover, since pg_get_partition_constraintdef ought now to work on indexes, restricting it to heaps is flat wrong anyway. The underlying function generate_partition_qual() wasn't on board with indexes having partition quals either, nor for that matter with rels having relispartition set but yet null relpartbound. (One wonders whether the person who wrote the function comment blocks claiming that these functions allow a missing relpartbound had ever tested it.) Fix by testing relispartition before opening the rel, and by using relation_open not heap_open. (If any other relkinds ever grow the ability to have relispartition set, the code will work with them automatically.) Also, don't reject null relpartbound in generate_partition_qual. Back-patch to v11, and all but the null-relpartbound change to v10. (It's not really necessary to change generate_partition_qual at all in v10, but I thought s/heap_open/relation_open/ would be a good idea anyway just to keep the code in sync with later branches.) Per report from Justin Pryzby. Discussion: https://postgr.es/m/20180927200020.GJ776@telsasoft.com
This commit is contained in:
parent
4ec90f53f1
commit
aaf10f32a3
24
src/backend/utils/cache/lsyscache.c
vendored
24
src/backend/utils/cache/lsyscache.c
vendored
@ -1846,6 +1846,30 @@ get_rel_relkind(Oid relid)
|
||||
return '\0';
|
||||
}
|
||||
|
||||
/*
|
||||
* get_rel_relispartition
|
||||
*
|
||||
* Returns the relispartition flag associated with a given relation.
|
||||
*/
|
||||
bool
|
||||
get_rel_relispartition(Oid relid)
|
||||
{
|
||||
HeapTuple tp;
|
||||
|
||||
tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
|
||||
if (HeapTupleIsValid(tp))
|
||||
{
|
||||
Form_pg_class reltup = (Form_pg_class) GETSTRUCT(tp);
|
||||
bool result;
|
||||
|
||||
result = reltup->relispartition;
|
||||
ReleaseSysCache(tp);
|
||||
return result;
|
||||
}
|
||||
else
|
||||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* get_rel_tablespace
|
||||
*
|
||||
|
40
src/backend/utils/cache/partcache.c
vendored
40
src/backend/utils/cache/partcache.c
vendored
@ -797,31 +797,38 @@ RelationGetPartitionQual(Relation rel)
|
||||
* get_partition_qual_relid
|
||||
*
|
||||
* Returns an expression tree describing the passed-in relation's partition
|
||||
* constraint. If there is no partition constraint returns NULL; this can
|
||||
* happen if the default partition is the only partition.
|
||||
* constraint.
|
||||
*
|
||||
* If the relation is not found, or is not a partition, or there is no
|
||||
* partition constraint, return NULL. We must guard against the first two
|
||||
* cases because this supports a SQL function that could be passed any OID.
|
||||
* The last case can happen even if relispartition is true, when a default
|
||||
* partition is the only partition.
|
||||
*/
|
||||
Expr *
|
||||
get_partition_qual_relid(Oid relid)
|
||||
{
|
||||
Relation rel = heap_open(relid, AccessShareLock);
|
||||
Expr *result = NULL;
|
||||
|
||||
/* Do the work only if this relation exists and is a partition. */
|
||||
if (get_rel_relispartition(relid))
|
||||
{
|
||||
Relation rel = relation_open(relid, AccessShareLock);
|
||||
List *and_args;
|
||||
|
||||
/* Do the work only if this relation is a partition. */
|
||||
if (rel->rd_rel->relispartition)
|
||||
{
|
||||
and_args = generate_partition_qual(rel);
|
||||
|
||||
/* Convert implicit-AND list format to boolean expression */
|
||||
if (and_args == NIL)
|
||||
result = NULL;
|
||||
else if (list_length(and_args) > 1)
|
||||
result = makeBoolExpr(AND_EXPR, and_args, -1);
|
||||
else
|
||||
result = linitial(and_args);
|
||||
}
|
||||
|
||||
/* Keep the lock. */
|
||||
heap_close(rel, NoLock);
|
||||
/* Keep the lock, to allow safe deparsing against the rel by caller. */
|
||||
relation_close(rel, NoLock);
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
@ -845,7 +852,6 @@ generate_partition_qual(Relation rel)
|
||||
MemoryContext oldcxt;
|
||||
Datum boundDatum;
|
||||
bool isnull;
|
||||
PartitionBoundSpec *bound;
|
||||
List *my_qual = NIL,
|
||||
*result = NIL;
|
||||
Relation parent;
|
||||
@ -859,7 +865,7 @@ generate_partition_qual(Relation rel)
|
||||
return copyObject(rel->rd_partcheck);
|
||||
|
||||
/* Grab at least an AccessShareLock on the parent table */
|
||||
parent = heap_open(get_partition_parent(RelationGetRelid(rel)),
|
||||
parent = relation_open(get_partition_parent(RelationGetRelid(rel)),
|
||||
AccessShareLock);
|
||||
|
||||
/* Get pg_class.relpartbound */
|
||||
@ -871,13 +877,17 @@ generate_partition_qual(Relation rel)
|
||||
boundDatum = SysCacheGetAttr(RELOID, tuple,
|
||||
Anum_pg_class_relpartbound,
|
||||
&isnull);
|
||||
if (isnull)
|
||||
elog(ERROR, "null relpartbound for relation %u", RelationGetRelid(rel));
|
||||
if (!isnull)
|
||||
{
|
||||
PartitionBoundSpec *bound;
|
||||
|
||||
bound = castNode(PartitionBoundSpec,
|
||||
stringToNode(TextDatumGetCString(boundDatum)));
|
||||
ReleaseSysCache(tuple);
|
||||
|
||||
my_qual = get_qual_from_partbound(rel, parent, bound);
|
||||
}
|
||||
|
||||
ReleaseSysCache(tuple);
|
||||
|
||||
/* Add the parent's quals to the list (if any) */
|
||||
if (parent->rd_rel->relispartition)
|
||||
@ -903,7 +913,7 @@ generate_partition_qual(Relation rel)
|
||||
MemoryContextSwitchTo(oldcxt);
|
||||
|
||||
/* Keep the parent locked until commit */
|
||||
heap_close(parent, NoLock);
|
||||
relation_close(parent, NoLock);
|
||||
|
||||
return result;
|
||||
}
|
||||
|
@ -128,6 +128,7 @@ extern char *get_rel_name(Oid relid);
|
||||
extern Oid get_rel_namespace(Oid relid);
|
||||
extern Oid get_rel_type_id(Oid relid);
|
||||
extern char get_rel_relkind(Oid relid);
|
||||
extern bool get_rel_relispartition(Oid relid);
|
||||
extern Oid get_rel_tablespace(Oid relid);
|
||||
extern char get_rel_persistence(Oid relid);
|
||||
extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
|
||||
|
@ -75,6 +75,25 @@ Indexes:
|
||||
"idxpart1_a_idx" btree (a)
|
||||
"idxpart1_b_c_idx" btree (b, c)
|
||||
|
||||
\d+ idxpart1_a_idx
|
||||
Index "public.idxpart1_a_idx"
|
||||
Column | Type | Key? | Definition | Storage | Stats target
|
||||
--------+---------+------+------------+---------+--------------
|
||||
a | integer | yes | a | plain |
|
||||
Partition of: idxparti
|
||||
No partition constraint
|
||||
btree, for table "public.idxpart1"
|
||||
|
||||
\d+ idxpart1_b_c_idx
|
||||
Index "public.idxpart1_b_c_idx"
|
||||
Column | Type | Key? | Definition | Storage | Stats target
|
||||
--------+---------+------+------------+----------+--------------
|
||||
b | integer | yes | b | plain |
|
||||
c | text | yes | c | extended |
|
||||
Partition of: idxparti2
|
||||
No partition constraint
|
||||
btree, for table "public.idxpart1"
|
||||
|
||||
drop table idxpart;
|
||||
-- If a partition already has an index, don't create a duplicative one
|
||||
create table idxpart (a int, b int) partition by range (a, b);
|
||||
|
@ -44,6 +44,8 @@ create table idxpart1 (like idxpart);
|
||||
\d idxpart1
|
||||
alter table idxpart attach partition idxpart1 for values from (0) to (10);
|
||||
\d idxpart1
|
||||
\d+ idxpart1_a_idx
|
||||
\d+ idxpart1_b_c_idx
|
||||
drop table idxpart;
|
||||
|
||||
-- If a partition already has an index, don't create a duplicative one
|
||||
|
Loading…
Reference in New Issue
Block a user