Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit aaf10f3

Browse filesBrowse files
tglsfdctglsfdc
committed
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
1 parent 4ec90f5 commit aaf10f3
Copy full SHA for aaf10f3

File tree

Expand file treeCollapse file tree

5 files changed

+75
-19
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+75
-19
lines changed

‎src/backend/utils/cache/lsyscache.c

Copy file name to clipboardExpand all lines: src/backend/utils/cache/lsyscache.c
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1846,6 +1846,30 @@ get_rel_relkind(Oid relid)
18461846
return '\0';
18471847
}
18481848

1849+
/*
1850+
* get_rel_relispartition
1851+
*
1852+
* Returns the relispartition flag associated with a given relation.
1853+
*/
1854+
bool
1855+
get_rel_relispartition(Oid relid)
1856+
{
1857+
HeapTuple tp;
1858+
1859+
tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
1860+
if (HeapTupleIsValid(tp))
1861+
{
1862+
Form_pg_class reltup = (Form_pg_class) GETSTRUCT(tp);
1863+
bool result;
1864+
1865+
result = reltup->relispartition;
1866+
ReleaseSysCache(tp);
1867+
return result;
1868+
}
1869+
else
1870+
return false;
1871+
}
1872+
18491873
/*
18501874
* get_rel_tablespace
18511875
*

‎src/backend/utils/cache/partcache.c

Copy file name to clipboardExpand all lines: src/backend/utils/cache/partcache.c
+29-19Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -797,31 +797,38 @@ RelationGetPartitionQual(Relation rel)
797797
* get_partition_qual_relid
798798
*
799799
* Returns an expression tree describing the passed-in relation's partition
800-
* constraint. If there is no partition constraint returns NULL; this can
801-
* happen if the default partition is the only partition.
800+
* constraint.
801+
*
802+
* If the relation is not found, or is not a partition, or there is no
803+
* partition constraint, return NULL. We must guard against the first two
804+
* cases because this supports a SQL function that could be passed any OID.
805+
* The last case can happen even if relispartition is true, when a default
806+
* partition is the only partition.
802807
*/
803808
Expr *
804809
get_partition_qual_relid(Oid relid)
805810
{
806-
Relation rel = heap_open(relid, AccessShareLock);
807811
Expr *result = NULL;
808-
List *and_args;
809812

810-
/* Do the work only if this relation is a partition. */
811-
if (rel->rd_rel->relispartition)
813+
/* Do the work only if this relation exists and is a partition. */
814+
if (get_rel_relispartition(relid))
812815
{
816+
Relation rel = relation_open(relid, AccessShareLock);
817+
List *and_args;
818+
813819
and_args = generate_partition_qual(rel);
814820

821+
/* Convert implicit-AND list format to boolean expression */
815822
if (and_args == NIL)
816823
result = NULL;
817824
else if (list_length(and_args) > 1)
818825
result = makeBoolExpr(AND_EXPR, and_args, -1);
819826
else
820827
result = linitial(and_args);
821-
}
822828

823-
/* Keep the lock. */
824-
heap_close(rel, NoLock);
829+
/* Keep the lock, to allow safe deparsing against the rel by caller. */
830+
relation_close(rel, NoLock);
831+
}
825832

826833
return result;
827834
}
@@ -845,7 +852,6 @@ generate_partition_qual(Relation rel)
845852
MemoryContext oldcxt;
846853
Datum boundDatum;
847854
bool isnull;
848-
PartitionBoundSpec *bound;
849855
List *my_qual = NIL,
850856
*result = NIL;
851857
Relation parent;
@@ -859,8 +865,8 @@ generate_partition_qual(Relation rel)
859865
return copyObject(rel->rd_partcheck);
860866

861867
/* Grab at least an AccessShareLock on the parent table */
862-
parent = heap_open(get_partition_parent(RelationGetRelid(rel)),
863-
AccessShareLock);
868+
parent = relation_open(get_partition_parent(RelationGetRelid(rel)),
869+
AccessShareLock);
864870

865871
/* Get pg_class.relpartbound */
866872
tuple = SearchSysCache1(RELOID, RelationGetRelid(rel));
@@ -871,13 +877,17 @@ generate_partition_qual(Relation rel)
871877
boundDatum = SysCacheGetAttr(RELOID, tuple,
872878
Anum_pg_class_relpartbound,
873879
&isnull);
874-
if (isnull)
875-
elog(ERROR, "null relpartbound for relation %u", RelationGetRelid(rel));
876-
bound = castNode(PartitionBoundSpec,
877-
stringToNode(TextDatumGetCString(boundDatum)));
878-
ReleaseSysCache(tuple);
880+
if (!isnull)
881+
{
882+
PartitionBoundSpec *bound;
883+
884+
bound = castNode(PartitionBoundSpec,
885+
stringToNode(TextDatumGetCString(boundDatum)));
879886

880-
my_qual = get_qual_from_partbound(rel, parent, bound);
887+
my_qual = get_qual_from_partbound(rel, parent, bound);
888+
}
889+
890+
ReleaseSysCache(tuple);
881891

882892
/* Add the parent's quals to the list (if any) */
883893
if (parent->rd_rel->relispartition)
@@ -903,7 +913,7 @@ generate_partition_qual(Relation rel)
903913
MemoryContextSwitchTo(oldcxt);
904914

905915
/* Keep the parent locked until commit */
906-
heap_close(parent, NoLock);
916+
relation_close(parent, NoLock);
907917

908918
return result;
909919
}

‎src/include/utils/lsyscache.h

Copy file name to clipboardExpand all lines: src/include/utils/lsyscache.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ extern char *get_rel_name(Oid relid);
128128
extern Oid get_rel_namespace(Oid relid);
129129
extern Oid get_rel_type_id(Oid relid);
130130
extern char get_rel_relkind(Oid relid);
131+
extern bool get_rel_relispartition(Oid relid);
131132
extern Oid get_rel_tablespace(Oid relid);
132133
extern char get_rel_persistence(Oid relid);
133134
extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);

‎src/test/regress/expected/indexing.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/indexing.out
+19Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,25 @@ Indexes:
7575
"idxpart1_a_idx" btree (a)
7676
"idxpart1_b_c_idx" btree (b, c)
7777

78+
\d+ idxpart1_a_idx
79+
Index "public.idxpart1_a_idx"
80+
Column | Type | Key? | Definition | Storage | Stats target
81+
--------+---------+------+------------+---------+--------------
82+
a | integer | yes | a | plain |
83+
Partition of: idxparti
84+
No partition constraint
85+
btree, for table "public.idxpart1"
86+
87+
\d+ idxpart1_b_c_idx
88+
Index "public.idxpart1_b_c_idx"
89+
Column | Type | Key? | Definition | Storage | Stats target
90+
--------+---------+------+------------+----------+--------------
91+
b | integer | yes | b | plain |
92+
c | text | yes | c | extended |
93+
Partition of: idxparti2
94+
No partition constraint
95+
btree, for table "public.idxpart1"
96+
7897
drop table idxpart;
7998
-- If a partition already has an index, don't create a duplicative one
8099
create table idxpart (a int, b int) partition by range (a, b);

‎src/test/regress/sql/indexing.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/indexing.sql
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ create table idxpart1 (like idxpart);
4444
\d idxpart1
4545
alter table idxpart attach partition idxpart1 for values from (0) to (10);
4646
\d idxpart1
47+
\d+ idxpart1_a_idx
48+
\d+ idxpart1_b_c_idx
4749
drop table idxpart;
4850

4951
-- If a partition already has an index, don't create a duplicative one

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.