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

Commit7d7b59a

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 parentebcf34d commit7d7b59a

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"
@@ -39,7 +41,6 @@
3941
#include"utils/rel.h"
4042
#include"utils/snapmgr.h"
4143
#include"utils/syscache.h"
42-
#include"utils/typcache.h"
4344

4445

4546
typedefstruct
@@ -61,14 +62,11 @@ static void transientrel_shutdown(DestReceiver *self);
6162
staticvoidtransientrel_destroy(DestReceiver*self);
6263
staticvoidrefresh_matview_datafill(DestReceiver*dest,Query*query,
6364
constchar*queryString);
64-
6565
staticchar*make_temptable_name_n(char*tempname,intn);
66-
staticvoidmv_GenerateOper(StringInfobuf,Oidopoid);
67-
6866
staticvoidrefresh_by_match_merge(OidmatviewOid,OidtempOid,Oidrelowner,
6967
intsave_sec_context);
7068
staticvoidrefresh_by_heap_swap(OidmatviewOid,OidOIDNewHeap,charrelpersistence);
71-
69+
staticboolis_usable_unique_index(RelationindexRel);
7270
staticvoidOpenMatViewIncrementalMaintenance(void);
7371
staticvoidCloseMatViewIncrementalMaintenance(void);
7472

@@ -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);
@@ -536,25 +523,6 @@ make_temptable_name_n(char *tempname, int n)
536523
returnnamebuf.data;
537524
}
538525

539-
staticvoid
540-
mv_GenerateOper(StringInfobuf,Oidopoid)
541-
{
542-
HeapTupleopertup;
543-
Form_pg_operatoroperform;
544-
545-
opertup=SearchSysCache1(OPEROID,ObjectIdGetDatum(opoid));
546-
if (!HeapTupleIsValid(opertup))
547-
elog(ERROR,"cache lookup failed for operator %u",opoid);
548-
operform= (Form_pg_operator)GETSTRUCT(opertup);
549-
Assert(operform->oprkind=='b');
550-
551-
appendStringInfo(buf,"OPERATOR(%s.%s)",
552-
quote_identifier(get_namespace_name(operform->oprnamespace)),
553-
NameStr(operform->oprname));
554-
555-
ReleaseSysCache(opertup);
556-
}
557-
558526
/*
559527
* refresh_by_match_merge
560528
*
@@ -602,7 +570,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
602570
List*indexoidlist;
603571
ListCell*indexoidscan;
604572
int16relnatts;
605-
bool*usedForQual;
573+
Oid*opUsedForQual;
606574

607575
initStringInfo(&querybuf);
608576
matviewRel=heap_open(matviewOid,NoLock);
@@ -614,7 +582,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
614582
diffname=make_temptable_name_n(tempname,2);
615583

616584
relnatts=matviewRel->rd_rel->relnatts;
617-
usedForQual= (bool*)palloc0(sizeof(bool)*relnatts);
618585

619586
/* Open SPI context. */
620587
if (SPI_connect()!=SPI_OK_CONNECT)
@@ -678,58 +645,98 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
678645
* include all rows.
679646
*/
680647
tupdesc=matviewRel->rd_att;
648+
opUsedForQual= (Oid*)palloc0(sizeof(Oid)*relnatts);
681649
foundUniqueIndex= false;
650+
682651
indexoidlist=RelationGetIndexList(matviewRel);
683652

684653
foreach(indexoidscan,indexoidlist)
685654
{
686655
Oidindexoid=lfirst_oid(indexoidscan);
687656
RelationindexRel;
688-
Form_pg_indexindexStruct;
689657

690658
indexRel=index_open(indexoid,RowExclusiveLock);
691-
indexStruct=indexRel->rd_index;
692-
693-
/*
694-
* We're only interested if it is unique, valid, contains no
695-
* expressions, and is not partial.
696-
*/
697-
if (indexStruct->indisunique&&
698-
IndexIsValid(indexStruct)&&
699-
RelationGetIndexExpressions(indexRel)==NIL&&
700-
RelationGetIndexPredicate(indexRel)==NIL)
659+
if (is_usable_unique_index(indexRel))
701660
{
661+
Form_pg_indexindexStruct=indexRel->rd_index;
702662
intnumatts=indexStruct->indnatts;
663+
oidvector*indclass;
664+
DatumindclassDatum;
665+
boolisnull;
703666
inti;
704667

668+
/* Must get indclass the hard way. */
669+
indclassDatum=SysCacheGetAttr(INDEXRELID,
670+
indexRel->rd_indextuple,
671+
Anum_pg_index_indclass,
672+
&isnull);
673+
Assert(!isnull);
674+
indclass= (oidvector*)DatumGetPointer(indclassDatum);
675+
705676
/* Add quals for all columns from this index. */
706677
for (i=0;i<numatts;i++)
707678
{
708679
intattnum=indexStruct->indkey.values[i];
709-
Oidtype;
680+
Oidopclass=indclass->values[i];
681+
Form_pg_attributeattr=TupleDescAttr(tupdesc,attnum-1);
682+
Oidattrtype=attr->atttypid;
683+
HeapTuplecla_ht;
684+
Form_pg_opclasscla_tup;
685+
Oidopfamily;
686+
Oidopcintype;
710687
Oidop;
711-
constchar*colname;
688+
constchar*leftop;
689+
constchar*rightop;
712690

713691
/*
714-
*Only includethecolumn once regardless of how many times
715-
*it shows up in how many indexes.
692+
*Identifytheequality operator associated with this index
693+
*column. First we need to look up the column's opclass.
716694
*/
717-
if (usedForQual[attnum-1])
695+
cla_ht=SearchSysCache1(CLAOID,ObjectIdGetDatum(opclass));
696+
if (!HeapTupleIsValid(cla_ht))
697+
elog(ERROR,"cache lookup failed for opclass %u",opclass);
698+
cla_tup= (Form_pg_opclass)GETSTRUCT(cla_ht);
699+
Assert(cla_tup->opcmethod==BTREE_AM_OID);
700+
opfamily=cla_tup->opcfamily;
701+
opcintype=cla_tup->opcintype;
702+
ReleaseSysCache(cla_ht);
703+
704+
op=get_opfamily_member(opfamily,opcintype,opcintype,
705+
BTEqualStrategyNumber);
706+
if (!OidIsValid(op))
707+
elog(ERROR,"missing operator %d(%u,%u) in opfamily %u",
708+
BTEqualStrategyNumber,opcintype,opcintype,opfamily);
709+
710+
/*
711+
* If we find the same column with the same equality semantics
712+
* in more than one index, we only need to emit the equality
713+
* clause once.
714+
*
715+
* Since we only remember the last equality operator, this
716+
* code could be fooled into emitting duplicate clauses given
717+
* multiple indexes with several different opclasses ... but
718+
* that's so unlikely it doesn't seem worth spending extra
719+
* code to avoid.
720+
*/
721+
if (opUsedForQual[attnum-1]==op)
718722
continue;
719-
usedForQual[attnum-1]=true;
723+
opUsedForQual[attnum-1]=op;
720724

721725
/*
722726
* Actually add the qual, ANDed with any others.
723727
*/
724728
if (foundUniqueIndex)
725729
appendStringInfoString(&querybuf," AND ");
726730

727-
colname=quote_identifier(NameStr((tupdesc->attrs[attnum-1])->attname));
728-
appendStringInfo(&querybuf,"newdata.%s ",colname);
729-
type=attnumTypeId(matviewRel,attnum);
730-
op=lookup_type_cache(type,TYPECACHE_EQ_OPR)->eq_opr;
731-
mv_GenerateOper(&querybuf,op);
732-
appendStringInfo(&querybuf," mv.%s",colname);
731+
leftop=quote_qualified_identifier("newdata",
732+
NameStr(attr->attname));
733+
rightop=quote_qualified_identifier("mv",
734+
NameStr(attr->attname));
735+
736+
generate_operator_clause(&querybuf,
737+
leftop,attrtype,
738+
op,
739+
rightop,attrtype);
733740

734741
foundUniqueIndex= true;
735742
}
@@ -742,11 +749,11 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
742749
list_free(indexoidlist);
743750

744751
/*
745-
* There must be at least one unique index on the matview.
752+
* There must be at least oneusableunique index on the matview.
746753
*
747754
* ExecRefreshMatView() checks that after taking the exclusive lock on the
748755
* matview. So at least one unique index is guaranteed to exist here
749-
* because the lock is still being held.
756+
* because the lock is still being held; so an Assert seems sufficient.
750757
*/
751758
Assert(foundUniqueIndex);
752759

@@ -823,6 +830,51 @@ refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
823830
RecentXmin,ReadNextMultiXactId(),relpersistence);
824831
}
825832

833+
/*
834+
* Check whether specified index is usable for match merge.
835+
*/
836+
staticbool
837+
is_usable_unique_index(RelationindexRel)
838+
{
839+
Form_pg_indexindexStruct=indexRel->rd_index;
840+
841+
/*
842+
* Must be unique, valid, immediate, non-partial, and be defined over
843+
* plain user columns (not expressions). We also require it to be a
844+
* btree. Even if we had any other unique index kinds, we'd not know how
845+
* to identify the corresponding equality operator, nor could we be sure
846+
* that the planner could implement the required FULL JOIN with non-btree
847+
* operators.
848+
*/
849+
if (indexStruct->indisunique&&
850+
indexStruct->indimmediate&&
851+
indexRel->rd_rel->relam==BTREE_AM_OID&&
852+
IndexIsValid(indexStruct)&&
853+
RelationGetIndexPredicate(indexRel)==NIL&&
854+
indexStruct->indnatts>0)
855+
{
856+
/*
857+
* The point of groveling through the index columns individually is to
858+
* reject both index expressions and system columns. Currently,
859+
* matviews couldn't have OID columns so there's no way to create an
860+
* index on a system column; but maybe someday that wouldn't be true,
861+
* so let's be safe.
862+
*/
863+
intnumatts=indexStruct->indnatts;
864+
inti;
865+
866+
for (i=0;i<numatts;i++)
867+
{
868+
intattnum=indexStruct->indkey.values[i];
869+
870+
if (attnum <=0)
871+
return false;
872+
}
873+
return true;
874+
}
875+
return false;
876+
}
877+
826878

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

26342579
/*

0 commit comments

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