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

Commite17e905

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 parent1568156 commite17e905

File tree

4 files changed

+208
-127
lines changed

4 files changed

+208
-127
lines changed

‎src/backend/commands/matview.c

Lines changed: 117 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,58 +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];
730-
Oidtype;
701+
Oidopclass=indclass->values[i];
702+
Form_pg_attributeattr=TupleDescAttr(tupdesc,attnum-1);
703+
Oidattrtype=attr->atttypid;
704+
HeapTuplecla_ht;
705+
Form_pg_opclasscla_tup;
706+
Oidopfamily;
707+
Oidopcintype;
731708
Oidop;
732-
constchar*colname;
709+
constchar*leftop;
710+
constchar*rightop;
733711

734712
/*
735-
*Only includethecolumn once regardless of how many times
736-
*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.
737715
*/
738-
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)
739743
continue;
740-
usedForQual[attnum-1]=true;
744+
opUsedForQual[attnum-1]=op;
741745

742746
/*
743747
* Actually add the qual, ANDed with any others.
744748
*/
745749
if (foundUniqueIndex)
746750
appendStringInfoString(&querybuf," AND ");
747751

748-
colname=quote_identifier(NameStr((tupdesc->attrs[attnum-1])->attname));
749-
appendStringInfo(&querybuf,"newdata.%s ",colname);
750-
type=attnumTypeId(matviewRel,attnum);
751-
op=lookup_type_cache(type,TYPECACHE_EQ_OPR)->eq_opr;
752-
mv_GenerateOper(&querybuf,op);
753-
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);
754761

755762
foundUniqueIndex= true;
756763
}
@@ -763,11 +770,11 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
763770
list_free(indexoidlist);
764771

765772
/*
766-
* There must be at least one unique index on the matview.
773+
* There must be at least oneusableunique index on the matview.
767774
*
768775
* ExecRefreshMatView() checks that after taking the exclusive lock on the
769776
* matview. So at least one unique index is guaranteed to exist here
770-
* because the lock is still being held.
777+
* because the lock is still being held; so an Assert seems sufficient.
771778
*/
772779
Assert(foundUniqueIndex);
773780

@@ -844,6 +851,51 @@ refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
844851
RecentXmin,ReadNextMultiXactId(),relpersistence);
845852
}
846853

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+
847899

848900
/*
849901
* 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
@@ -205,7 +205,6 @@ static void ri_GenerateQual(StringInfo buf,
205205
constchar*leftop,Oidleftoptype,
206206
Oidopoid,
207207
constchar*rightop,Oidrightoptype);
208-
staticvoidri_add_cast_to(StringInfobuf,Oidtypid);
209208
staticvoidri_GenerateQualCollation(StringInfobuf,Oidcollation);
210209
staticintri_NullCheck(HeapTupletup,
211210
constRI_ConstraintInfo*riinfo,boolrel_is_pk);
@@ -2557,13 +2556,10 @@ quoteRelationName(char *buffer, Relation rel)
25572556
/*
25582557
* ri_GenerateQual --- generate a WHERE clause equating two variables
25592558
*
2560-
* The idea is to append " sep leftop op rightop" to buf. The complexity
2561-
* comes from needing to be sure that the parser will select the desired
2562-
* operator. We always name the operator using OPERATOR(schema.op) syntax
2563-
* (readability isn't a big priority here), so as to avoid search-path
2564-
* uncertainties. We have to emit casts too, if either input isn't already
2565-
* the input type of the operator; else we are at the mercy of the parser's
2566-
* heuristics for ambiguous-operator resolution.
2559+
* This basically appends " sep leftop op rightop" to buf, adding casts
2560+
* and schema qualification as needed to ensure that the parser will select
2561+
* the operator we specify. leftop and rightop should be parenthesized
2562+
* if they aren't variables or parameters.
25672563
*/
25682564
staticvoid
25692565
ri_GenerateQual(StringInfobuf,
@@ -2572,60 +2568,9 @@ ri_GenerateQual(StringInfo buf,
25722568
Oidopoid,
25732569
constchar*rightop,Oidrightoptype)
25742570
{
2575-
HeapTupleopertup;
2576-
Form_pg_operatoroperform;
2577-
char*oprname;
2578-
char*nspname;
2579-
2580-
opertup=SearchSysCache1(OPEROID,ObjectIdGetDatum(opoid));
2581-
if (!HeapTupleIsValid(opertup))
2582-
elog(ERROR,"cache lookup failed for operator %u",opoid);
2583-
operform= (Form_pg_operator)GETSTRUCT(opertup);
2584-
Assert(operform->oprkind=='b');
2585-
oprname=NameStr(operform->oprname);
2586-
2587-
nspname=get_namespace_name(operform->oprnamespace);
2588-
2589-
appendStringInfo(buf," %s %s",sep,leftop);
2590-
if (leftoptype!=operform->oprleft)
2591-
ri_add_cast_to(buf,operform->oprleft);
2592-
appendStringInfo(buf," OPERATOR(%s.",quote_identifier(nspname));
2593-
appendStringInfoString(buf,oprname);
2594-
appendStringInfo(buf,") %s",rightop);
2595-
if (rightoptype!=operform->oprright)
2596-
ri_add_cast_to(buf,operform->oprright);
2597-
2598-
ReleaseSysCache(opertup);
2599-
}
2600-
2601-
/*
2602-
* Add a cast specification to buf. We spell out the type name the hard way,
2603-
* intentionally not using format_type_be(). This is to avoid corner cases
2604-
* for CHARACTER, BIT, and perhaps other types, where specifying the type
2605-
* using SQL-standard syntax results in undesirable data truncation. By
2606-
* doing it this way we can be certain that the cast will have default (-1)
2607-
* target typmod.
2608-
*/
2609-
staticvoid
2610-
ri_add_cast_to(StringInfobuf,Oidtypid)
2611-
{
2612-
HeapTupletypetup;
2613-
Form_pg_typetypform;
2614-
char*typname;
2615-
char*nspname;
2616-
2617-
typetup=SearchSysCache1(TYPEOID,ObjectIdGetDatum(typid));
2618-
if (!HeapTupleIsValid(typetup))
2619-
elog(ERROR,"cache lookup failed for type %u",typid);
2620-
typform= (Form_pg_type)GETSTRUCT(typetup);
2621-
2622-
typname=NameStr(typform->typname);
2623-
nspname=get_namespace_name(typform->typnamespace);
2624-
2625-
appendStringInfo(buf,"::%s.%s",
2626-
quote_identifier(nspname),quote_identifier(typname));
2627-
2628-
ReleaseSysCache(typetup);
2571+
appendStringInfo(buf," %s ",sep);
2572+
generate_operator_clause(buf,leftop,leftoptype,opoid,
2573+
rightop,rightoptype);
26292574
}
26302575

26312576
/*

0 commit comments

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