Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commitaaf10f3

Browse files
committed
Fix assorted bugs in pg_get_partition_constraintdef().
It failed if passed a nonexistent relation OID, or one that was a non-heaprelation, because of blindly applying heap_open to a user-supplied OID.This is not OK behavior for a SQL-exposed function; we have a projectpolicy that we should return NULL in such cases. Moreover, sincepg_get_partition_constraintdef ought now to work on indexes, restrictingit to heaps is flat wrong anyway.The underlying function generate_partition_qual() wasn't on board withindexes having partition quals either, nor for that matter with relshaving relispartition set but yet null relpartbound. (One wonderswhether the person who wrote the function comment blocks claiming thatthese functions allow a missing relpartbound had ever tested it.)Fix by testing relispartition before opening the rel, and by usingrelation_open not heap_open. (If any other relkinds ever grow theability to have relispartition set, the code will work with themautomatically.) Also, don't reject null relpartbound ingenerate_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 allin v10, but I thought s/heap_open/relation_open/ would be a goodidea 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 parent4ec90f5 commitaaf10f3

File tree

5 files changed

+75
-19
lines changed

5 files changed

+75
-19
lines changed

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

Lines 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(Oidrelid)
1856+
{
1857+
HeapTupletp;
1858+
1859+
tp=SearchSysCache1(RELOID,ObjectIdGetDatum(relid));
1860+
if (HeapTupleIsValid(tp))
1861+
{
1862+
Form_pg_classreltup= (Form_pg_class)GETSTRUCT(tp);
1863+
boolresult;
1864+
1865+
result=reltup->relispartition;
1866+
ReleaseSysCache(tp);
1867+
returnresult;
1868+
}
1869+
else
1870+
return false;
1871+
}
1872+
18491873
/*
18501874
* get_rel_tablespace
18511875
*

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

Lines 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(Oidrelid)
805810
{
806-
Relationrel=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 relationexists andis a partition. */
814+
if (get_rel_relispartition(relid))
812815
{
816+
Relationrel=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
elseif (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
returnresult;
827834
}
@@ -845,7 +852,6 @@ generate_partition_qual(Relation rel)
845852
MemoryContextoldcxt;
846853
DatumboundDatum;
847854
boolisnull;
848-
PartitionBoundSpec*bound;
849855
List*my_qual=NIL,
850856
*result=NIL;
851857
Relationparent;
@@ -859,8 +865,8 @@ generate_partition_qual(Relation rel)
859865
returncopyObject(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
returnresult;
909919
}

‎src/include/utils/lsyscache.h

Lines 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
externOidget_rel_namespace(Oidrelid);
129129
externOidget_rel_type_id(Oidrelid);
130130
externcharget_rel_relkind(Oidrelid);
131+
externboolget_rel_relispartition(Oidrelid);
131132
externOidget_rel_tablespace(Oidrelid);
132133
externcharget_rel_persistence(Oidrelid);
133134
externOidget_transform_fromsql(Oidtypid,Oidlangid,List*trftypes);

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

Lines 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

Lines 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
altertable idxpart attach partition idxpart1 forvaluesfrom (0) to (10);
4646
\d idxpart1
47+
\d+ idxpart1_a_idx
48+
\d+ idxpart1_b_c_idx
4749
droptable idxpart;
4850

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

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp