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

Commit6497a18

Browse files
committed
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY.
refresh_by_match_merge() has some issues in the way it builds a SQLquery to construct the "diff" table:1. It doesn't require the selected unique index(es) to be indimmediate.2. It doesn't pay attention to the particular equality semantics enforcedby a given index, but just assumes that they must be those of the columndatatype's default btree opclass.3. It doesn't check that the indexes are btrees.4. It's insufficiently careful to ensure that the parser will pick theintended operator when parsing the query. (This would have been asecurity bug beforeCVE-2018-1058.)5. It's not careful about indexes on system columns.The way tofix#4 is to make use of the existing code in ri_triggers.cfor generating an arbitrary binary operator clause. I chose to movethat to ruleutils.c, since that seems a more reasonable place to beexporting such functionality from than ri_triggers.c.While#1,#3, and#5 are just latent given existing feature restrictions,and#2 doesn't arise in the core system for lack of alternate opclasseswith different equality behaviors,#4 seems like an issue worthback-patching. That's the bulk of the change anyway, so just back-patchthe whole thing to 9.4 where this code was introduced.Discussion:https://postgr.es/m/13836.1521413227@sss.pgh.pa.us
1 parent9ad21a6 commit6497a18

File tree

4 files changed

+207
-127
lines changed

4 files changed

+207
-127
lines changed

‎src/backend/commands/matview.c

Lines changed: 116 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
#include"catalog/catalog.h"
2222
#include"catalog/indexing.h"
2323
#include"catalog/namespace.h"
24+
#include"catalog/pg_am.h"
25+
#include"catalog/pg_opclass.h"
2426
#include"catalog/pg_operator.h"
2527
#include"commands/cluster.h"
2628
#include"commands/matview.h"
@@ -40,7 +42,6 @@
4042
#include"utils/rel.h"
4143
#include"utils/snapmgr.h"
4244
#include"utils/syscache.h"
43-
#include"utils/typcache.h"
4445

4546

4647
typedefstruct
@@ -62,14 +63,11 @@ static void transientrel_shutdown(DestReceiver *self);
6263
staticvoidtransientrel_destroy(DestReceiver*self);
6364
staticuint64refresh_matview_datafill(DestReceiver*dest,Query*query,
6465
constchar*queryString);
65-
6666
staticchar*make_temptable_name_n(char*tempname,intn);
67-
staticvoidmv_GenerateOper(StringInfobuf,Oidopoid);
68-
6967
staticvoidrefresh_by_match_merge(OidmatviewOid,OidtempOid,Oidrelowner,
7068
intsave_sec_context);
7169
staticvoidrefresh_by_heap_swap(OidmatviewOid,OidOIDNewHeap,charrelpersistence);
72-
70+
staticboolis_usable_unique_index(RelationindexRel);
7371
staticvoidOpenMatViewIncrementalMaintenance(void);
7472
staticvoidCloseMatViewIncrementalMaintenance(void);
7573

@@ -230,23 +228,12 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
230228
{
231229
Oidindexoid=lfirst_oid(indexoidscan);
232230
RelationindexRel;
233-
Form_pg_indexindexStruct;
234231

235232
indexRel=index_open(indexoid,AccessShareLock);
236-
indexStruct=indexRel->rd_index;
237-
238-
if (indexStruct->indisunique&&
239-
IndexIsValid(indexStruct)&&
240-
RelationGetIndexExpressions(indexRel)==NIL&&
241-
RelationGetIndexPredicate(indexRel)==NIL&&
242-
indexStruct->indnatts>0)
243-
{
244-
hasUniqueIndex= true;
245-
index_close(indexRel,AccessShareLock);
246-
break;
247-
}
248-
233+
hasUniqueIndex=is_usable_unique_index(indexRel);
249234
index_close(indexRel,AccessShareLock);
235+
if (hasUniqueIndex)
236+
break;
250237
}
251238

252239
list_free(indexoidlist);
@@ -557,25 +544,6 @@ make_temptable_name_n(char *tempname, int n)
557544
returnnamebuf.data;
558545
}
559546

560-
staticvoid
561-
mv_GenerateOper(StringInfobuf,Oidopoid)
562-
{
563-
HeapTupleopertup;
564-
Form_pg_operatoroperform;
565-
566-
opertup=SearchSysCache1(OPEROID,ObjectIdGetDatum(opoid));
567-
if (!HeapTupleIsValid(opertup))
568-
elog(ERROR,"cache lookup failed for operator %u",opoid);
569-
operform= (Form_pg_operator)GETSTRUCT(opertup);
570-
Assert(operform->oprkind=='b');
571-
572-
appendStringInfo(buf,"OPERATOR(%s.%s)",
573-
quote_identifier(get_namespace_name(operform->oprnamespace)),
574-
NameStr(operform->oprname));
575-
576-
ReleaseSysCache(opertup);
577-
}
578-
579547
/*
580548
* refresh_by_match_merge
581549
*
@@ -623,7 +591,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
623591
List*indexoidlist;
624592
ListCell*indexoidscan;
625593
int16relnatts;
626-
bool*usedForQual;
594+
Oid*opUsedForQual;
627595

628596
initStringInfo(&querybuf);
629597
matviewRel=heap_open(matviewOid,NoLock);
@@ -635,7 +603,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
635603
diffname=make_temptable_name_n(tempname,2);
636604

637605
relnatts=matviewRel->rd_rel->relnatts;
638-
usedForQual= (bool*)palloc0(sizeof(bool)*relnatts);
639606

640607
/* Open SPI context. */
641608
if (SPI_connect()!=SPI_OK_CONNECT)
@@ -699,59 +666,98 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
699666
* include all rows.
700667
*/
701668
tupdesc=matviewRel->rd_att;
669+
opUsedForQual= (Oid*)palloc0(sizeof(Oid)*relnatts);
702670
foundUniqueIndex= false;
671+
703672
indexoidlist=RelationGetIndexList(matviewRel);
704673

705674
foreach(indexoidscan,indexoidlist)
706675
{
707676
Oidindexoid=lfirst_oid(indexoidscan);
708677
RelationindexRel;
709-
Form_pg_indexindexStruct;
710678

711679
indexRel=index_open(indexoid,RowExclusiveLock);
712-
indexStruct=indexRel->rd_index;
713-
714-
/*
715-
* We're only interested if it is unique, valid, contains no
716-
* expressions, and is not partial.
717-
*/
718-
if (indexStruct->indisunique&&
719-
IndexIsValid(indexStruct)&&
720-
RelationGetIndexExpressions(indexRel)==NIL&&
721-
RelationGetIndexPredicate(indexRel)==NIL)
680+
if (is_usable_unique_index(indexRel))
722681
{
682+
Form_pg_indexindexStruct=indexRel->rd_index;
723683
intnumatts=indexStruct->indnatts;
684+
oidvector*indclass;
685+
DatumindclassDatum;
686+
boolisnull;
724687
inti;
725688

689+
/* Must get indclass the hard way. */
690+
indclassDatum=SysCacheGetAttr(INDEXRELID,
691+
indexRel->rd_indextuple,
692+
Anum_pg_index_indclass,
693+
&isnull);
694+
Assert(!isnull);
695+
indclass= (oidvector*)DatumGetPointer(indclassDatum);
696+
726697
/* Add quals for all columns from this index. */
727698
for (i=0;i<numatts;i++)
728699
{
729700
intattnum=indexStruct->indkey.values[i];
701+
Oidopclass=indclass->values[i];
730702
Form_pg_attributeattr=TupleDescAttr(tupdesc,attnum-1);
731-
Oidtype;
703+
Oidattrtype=attr->atttypid;
704+
HeapTuplecla_ht;
705+
Form_pg_opclasscla_tup;
706+
Oidopfamily;
707+
Oidopcintype;
732708
Oidop;
733-
constchar*colname;
709+
constchar*leftop;
710+
constchar*rightop;
734711

735712
/*
736-
*Only includethecolumn once regardless of how many times
737-
*it shows up in how many indexes.
713+
*Identifytheequality operator associated with this index
714+
*column. First we need to look up the column's opclass.
738715
*/
739-
if (usedForQual[attnum-1])
716+
cla_ht=SearchSysCache1(CLAOID,ObjectIdGetDatum(opclass));
717+
if (!HeapTupleIsValid(cla_ht))
718+
elog(ERROR,"cache lookup failed for opclass %u",opclass);
719+
cla_tup= (Form_pg_opclass)GETSTRUCT(cla_ht);
720+
Assert(cla_tup->opcmethod==BTREE_AM_OID);
721+
opfamily=cla_tup->opcfamily;
722+
opcintype=cla_tup->opcintype;
723+
ReleaseSysCache(cla_ht);
724+
725+
op=get_opfamily_member(opfamily,opcintype,opcintype,
726+
BTEqualStrategyNumber);
727+
if (!OidIsValid(op))
728+
elog(ERROR,"missing operator %d(%u,%u) in opfamily %u",
729+
BTEqualStrategyNumber,opcintype,opcintype,opfamily);
730+
731+
/*
732+
* If we find the same column with the same equality semantics
733+
* in more than one index, we only need to emit the equality
734+
* clause once.
735+
*
736+
* Since we only remember the last equality operator, this
737+
* code could be fooled into emitting duplicate clauses given
738+
* multiple indexes with several different opclasses ... but
739+
* that's so unlikely it doesn't seem worth spending extra
740+
* code to avoid.
741+
*/
742+
if (opUsedForQual[attnum-1]==op)
740743
continue;
741-
usedForQual[attnum-1]=true;
744+
opUsedForQual[attnum-1]=op;
742745

743746
/*
744747
* Actually add the qual, ANDed with any others.
745748
*/
746749
if (foundUniqueIndex)
747750
appendStringInfoString(&querybuf," AND ");
748751

749-
colname=quote_identifier(NameStr(attr->attname));
750-
appendStringInfo(&querybuf,"newdata.%s ",colname);
751-
type=attnumTypeId(matviewRel,attnum);
752-
op=lookup_type_cache(type,TYPECACHE_EQ_OPR)->eq_opr;
753-
mv_GenerateOper(&querybuf,op);
754-
appendStringInfo(&querybuf," mv.%s",colname);
752+
leftop=quote_qualified_identifier("newdata",
753+
NameStr(attr->attname));
754+
rightop=quote_qualified_identifier("mv",
755+
NameStr(attr->attname));
756+
757+
generate_operator_clause(&querybuf,
758+
leftop,attrtype,
759+
op,
760+
rightop,attrtype);
755761

756762
foundUniqueIndex= true;
757763
}
@@ -764,11 +770,11 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
764770
list_free(indexoidlist);
765771

766772
/*
767-
* There must be at least one unique index on the matview.
773+
* There must be at least oneusableunique index on the matview.
768774
*
769775
* ExecRefreshMatView() checks that after taking the exclusive lock on the
770776
* matview. So at least one unique index is guaranteed to exist here
771-
* because the lock is still being held.
777+
* because the lock is still being held; so an Assert seems sufficient.
772778
*/
773779
Assert(foundUniqueIndex);
774780

@@ -845,6 +851,51 @@ refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
845851
RecentXmin,ReadNextMultiXactId(),relpersistence);
846852
}
847853

854+
/*
855+
* Check whether specified index is usable for match merge.
856+
*/
857+
staticbool
858+
is_usable_unique_index(RelationindexRel)
859+
{
860+
Form_pg_indexindexStruct=indexRel->rd_index;
861+
862+
/*
863+
* Must be unique, valid, immediate, non-partial, and be defined over
864+
* plain user columns (not expressions). We also require it to be a
865+
* btree. Even if we had any other unique index kinds, we'd not know how
866+
* to identify the corresponding equality operator, nor could we be sure
867+
* that the planner could implement the required FULL JOIN with non-btree
868+
* operators.
869+
*/
870+
if (indexStruct->indisunique&&
871+
indexStruct->indimmediate&&
872+
indexRel->rd_rel->relam==BTREE_AM_OID&&
873+
IndexIsValid(indexStruct)&&
874+
RelationGetIndexPredicate(indexRel)==NIL&&
875+
indexStruct->indnatts>0)
876+
{
877+
/*
878+
* The point of groveling through the index columns individually is to
879+
* reject both index expressions and system columns. Currently,
880+
* matviews couldn't have OID columns so there's no way to create an
881+
* index on a system column; but maybe someday that wouldn't be true,
882+
* so let's be safe.
883+
*/
884+
intnumatts=indexStruct->indnatts;
885+
inti;
886+
887+
for (i=0;i<numatts;i++)
888+
{
889+
intattnum=indexStruct->indkey.values[i];
890+
891+
if (attnum <=0)
892+
return false;
893+
}
894+
return true;
895+
}
896+
return false;
897+
}
898+
848899

849900
/*
850901
* This should be used to test whether the backend is in a context where it is

‎src/backend/utils/adt/ri_triggers.c

Lines changed: 7 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,6 @@ static void ri_GenerateQual(StringInfo buf,
203203
constchar*leftop,Oidleftoptype,
204204
Oidopoid,
205205
constchar*rightop,Oidrightoptype);
206-
staticvoidri_add_cast_to(StringInfobuf,Oidtypid);
207206
staticvoidri_GenerateQualCollation(StringInfobuf,Oidcollation);
208207
staticintri_NullCheck(HeapTupletup,
209208
constRI_ConstraintInfo*riinfo,boolrel_is_pk);
@@ -2134,13 +2133,10 @@ quoteRelationName(char *buffer, Relation rel)
21342133
/*
21352134
* ri_GenerateQual --- generate a WHERE clause equating two variables
21362135
*
2137-
* The idea is to append " sep leftop op rightop" to buf. The complexity
2138-
* comes from needing to be sure that the parser will select the desired
2139-
* operator. We always name the operator using OPERATOR(schema.op) syntax
2140-
* (readability isn't a big priority here), so as to avoid search-path
2141-
* uncertainties. We have to emit casts too, if either input isn't already
2142-
* the input type of the operator; else we are at the mercy of the parser's
2143-
* heuristics for ambiguous-operator resolution.
2136+
* This basically appends " sep leftop op rightop" to buf, adding casts
2137+
* and schema qualification as needed to ensure that the parser will select
2138+
* the operator we specify. leftop and rightop should be parenthesized
2139+
* if they aren't variables or parameters.
21442140
*/
21452141
staticvoid
21462142
ri_GenerateQual(StringInfobuf,
@@ -2149,60 +2145,9 @@ ri_GenerateQual(StringInfo buf,
21492145
Oidopoid,
21502146
constchar*rightop,Oidrightoptype)
21512147
{
2152-
HeapTupleopertup;
2153-
Form_pg_operatoroperform;
2154-
char*oprname;
2155-
char*nspname;
2156-
2157-
opertup=SearchSysCache1(OPEROID,ObjectIdGetDatum(opoid));
2158-
if (!HeapTupleIsValid(opertup))
2159-
elog(ERROR,"cache lookup failed for operator %u",opoid);
2160-
operform= (Form_pg_operator)GETSTRUCT(opertup);
2161-
Assert(operform->oprkind=='b');
2162-
oprname=NameStr(operform->oprname);
2163-
2164-
nspname=get_namespace_name(operform->oprnamespace);
2165-
2166-
appendStringInfo(buf," %s %s",sep,leftop);
2167-
if (leftoptype!=operform->oprleft)
2168-
ri_add_cast_to(buf,operform->oprleft);
2169-
appendStringInfo(buf," OPERATOR(%s.",quote_identifier(nspname));
2170-
appendStringInfoString(buf,oprname);
2171-
appendStringInfo(buf,") %s",rightop);
2172-
if (rightoptype!=operform->oprright)
2173-
ri_add_cast_to(buf,operform->oprright);
2174-
2175-
ReleaseSysCache(opertup);
2176-
}
2177-
2178-
/*
2179-
* Add a cast specification to buf. We spell out the type name the hard way,
2180-
* intentionally not using format_type_be(). This is to avoid corner cases
2181-
* for CHARACTER, BIT, and perhaps other types, where specifying the type
2182-
* using SQL-standard syntax results in undesirable data truncation. By
2183-
* doing it this way we can be certain that the cast will have default (-1)
2184-
* target typmod.
2185-
*/
2186-
staticvoid
2187-
ri_add_cast_to(StringInfobuf,Oidtypid)
2188-
{
2189-
HeapTupletypetup;
2190-
Form_pg_typetypform;
2191-
char*typname;
2192-
char*nspname;
2193-
2194-
typetup=SearchSysCache1(TYPEOID,ObjectIdGetDatum(typid));
2195-
if (!HeapTupleIsValid(typetup))
2196-
elog(ERROR,"cache lookup failed for type %u",typid);
2197-
typform= (Form_pg_type)GETSTRUCT(typetup);
2198-
2199-
typname=NameStr(typform->typname);
2200-
nspname=get_namespace_name(typform->typnamespace);
2201-
2202-
appendStringInfo(buf,"::%s.%s",
2203-
quote_identifier(nspname),quote_identifier(typname));
2204-
2205-
ReleaseSysCache(typetup);
2148+
appendStringInfo(buf," %s ",sep);
2149+
generate_operator_clause(buf,leftop,leftoptype,opoid,
2150+
rightop,rightoptype);
22062151
}
22072152

22082153
/*

0 commit comments

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