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

Commitaa1cacd

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 parentc553e4a commitaa1cacd

File tree

4 files changed

+203
-111
lines changed

4 files changed

+203
-111
lines changed

‎src/backend/commands/matview.c

Lines changed: 112 additions & 49 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

@@ -490,25 +488,6 @@ make_temptable_name_n(char *tempname, int n)
490488
returnnamebuf.data;
491489
}
492490

493-
staticvoid
494-
mv_GenerateOper(StringInfobuf,Oidopoid)
495-
{
496-
HeapTupleopertup;
497-
Form_pg_operatoroperform;
498-
499-
opertup=SearchSysCache1(OPEROID,ObjectIdGetDatum(opoid));
500-
if (!HeapTupleIsValid(opertup))
501-
elog(ERROR,"cache lookup failed for operator %u",opoid);
502-
operform= (Form_pg_operator)GETSTRUCT(opertup);
503-
Assert(operform->oprkind=='b');
504-
505-
appendStringInfo(buf,"OPERATOR(%s.%s)",
506-
quote_identifier(get_namespace_name(operform->oprnamespace)),
507-
NameStr(operform->oprname));
508-
509-
ReleaseSysCache(opertup);
510-
}
511-
512491
/*
513492
* refresh_by_match_merge
514493
*
@@ -556,7 +535,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
556535
List*indexoidlist;
557536
ListCell*indexoidscan;
558537
int16relnatts;
559-
bool*usedForQual;
538+
Oid*opUsedForQual;
560539

561540
initStringInfo(&querybuf);
562541
matviewRel=heap_open(matviewOid,NoLock);
@@ -568,7 +547,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
568547
diffname=make_temptable_name_n(tempname,2);
569548

570549
relnatts=matviewRel->rd_rel->relnatts;
571-
usedForQual= (bool*)palloc0(sizeof(bool)*relnatts);
572550

573551
/* Open SPI context. */
574552
if (SPI_connect()!=SPI_OK_CONNECT)
@@ -632,58 +610,98 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
632610
* include all rows.
633611
*/
634612
tupdesc=matviewRel->rd_att;
613+
opUsedForQual= (Oid*)palloc0(sizeof(Oid)*relnatts);
635614
foundUniqueIndex= false;
615+
636616
indexoidlist=RelationGetIndexList(matviewRel);
637617

638618
foreach(indexoidscan,indexoidlist)
639619
{
640620
Oidindexoid=lfirst_oid(indexoidscan);
641621
RelationindexRel;
642-
Form_pg_indexindexStruct;
643622

644623
indexRel=index_open(indexoid,RowExclusiveLock);
645-
indexStruct=indexRel->rd_index;
646-
647-
/*
648-
* We're only interested if it is unique, valid, contains no
649-
* expressions, and is not partial.
650-
*/
651-
if (indexStruct->indisunique&&
652-
IndexIsValid(indexStruct)&&
653-
RelationGetIndexExpressions(indexRel)==NIL&&
654-
RelationGetIndexPredicate(indexRel)==NIL)
624+
if (is_usable_unique_index(indexRel))
655625
{
626+
Form_pg_indexindexStruct=indexRel->rd_index;
656627
intnumatts=indexStruct->indnatts;
628+
oidvector*indclass;
629+
DatumindclassDatum;
630+
boolisnull;
657631
inti;
658632

633+
/* Must get indclass the hard way. */
634+
indclassDatum=SysCacheGetAttr(INDEXRELID,
635+
indexRel->rd_indextuple,
636+
Anum_pg_index_indclass,
637+
&isnull);
638+
Assert(!isnull);
639+
indclass= (oidvector*)DatumGetPointer(indclassDatum);
640+
659641
/* Add quals for all columns from this index. */
660642
for (i=0;i<numatts;i++)
661643
{
662644
intattnum=indexStruct->indkey.values[i];
663-
Oidtype;
645+
Oidopclass=indclass->values[i];
646+
Form_pg_attributeattr=TupleDescAttr(tupdesc,attnum-1);
647+
Oidattrtype=attr->atttypid;
648+
HeapTuplecla_ht;
649+
Form_pg_opclasscla_tup;
650+
Oidopfamily;
651+
Oidopcintype;
664652
Oidop;
665-
constchar*colname;
653+
constchar*leftop;
654+
constchar*rightop;
655+
656+
/*
657+
* Identify the equality operator associated with this index
658+
* column. First we need to look up the column's opclass.
659+
*/
660+
cla_ht=SearchSysCache1(CLAOID,ObjectIdGetDatum(opclass));
661+
if (!HeapTupleIsValid(cla_ht))
662+
elog(ERROR,"cache lookup failed for opclass %u",opclass);
663+
cla_tup= (Form_pg_opclass)GETSTRUCT(cla_ht);
664+
Assert(cla_tup->opcmethod==BTREE_AM_OID);
665+
opfamily=cla_tup->opcfamily;
666+
opcintype=cla_tup->opcintype;
667+
ReleaseSysCache(cla_ht);
668+
669+
op=get_opfamily_member(opfamily,opcintype,opcintype,
670+
BTEqualStrategyNumber);
671+
if (!OidIsValid(op))
672+
elog(ERROR,"missing operator %d(%u,%u) in opfamily %u",
673+
BTEqualStrategyNumber,opcintype,opcintype,opfamily);
666674

667675
/*
668-
* Only include the column once regardless of how many times
669-
* it shows up in how many indexes.
676+
* If we find the same column with the same equality semantics
677+
* in more than one index, we only need to emit the equality
678+
* clause once.
679+
*
680+
* Since we only remember the last equality operator, this
681+
* code could be fooled into emitting duplicate clauses given
682+
* multiple indexes with several different opclasses ... but
683+
* that's so unlikely it doesn't seem worth spending extra
684+
* code to avoid.
670685
*/
671-
if (usedForQual[attnum-1])
686+
if (opUsedForQual[attnum-1]==op)
672687
continue;
673-
usedForQual[attnum-1]=true;
688+
opUsedForQual[attnum-1]=op;
674689

675690
/*
676691
* Actually add the qual, ANDed with any others.
677692
*/
678693
if (foundUniqueIndex)
679694
appendStringInfoString(&querybuf," AND ");
680695

681-
colname=quote_identifier(NameStr((tupdesc->attrs[attnum-1])->attname));
682-
appendStringInfo(&querybuf,"newdata.%s ",colname);
683-
type=attnumTypeId(matviewRel,attnum);
684-
op=lookup_type_cache(type,TYPECACHE_EQ_OPR)->eq_opr;
685-
mv_GenerateOper(&querybuf,op);
686-
appendStringInfo(&querybuf," mv.%s",colname);
696+
leftop=quote_qualified_identifier("newdata",
697+
NameStr(attr->attname));
698+
rightop=quote_qualified_identifier("mv",
699+
NameStr(attr->attname));
700+
701+
generate_operator_clause(&querybuf,
702+
leftop,attrtype,
703+
op,
704+
rightop,attrtype);
687705

688706
foundUniqueIndex= true;
689707
}
@@ -775,6 +793,51 @@ refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
775793
RecentXmin,ReadNextMultiXactId(),relpersistence);
776794
}
777795

796+
/*
797+
* Check whether specified index is usable for match merge.
798+
*/
799+
staticbool
800+
is_usable_unique_index(RelationindexRel)
801+
{
802+
Form_pg_indexindexStruct=indexRel->rd_index;
803+
804+
/*
805+
* Must be unique, valid, immediate, non-partial, and be defined over
806+
* plain user columns (not expressions). We also require it to be a
807+
* btree. Even if we had any other unique index kinds, we'd not know how
808+
* to identify the corresponding equality operator, nor could we be sure
809+
* that the planner could implement the required FULL JOIN with non-btree
810+
* operators.
811+
*/
812+
if (indexStruct->indisunique&&
813+
indexStruct->indimmediate&&
814+
indexRel->rd_rel->relam==BTREE_AM_OID&&
815+
IndexIsValid(indexStruct)&&
816+
RelationGetIndexPredicate(indexRel)==NIL&&
817+
indexStruct->indnatts>0)
818+
{
819+
/*
820+
* The point of groveling through the index columns individually is to
821+
* reject both index expressions and system columns. Currently,
822+
* matviews couldn't have OID columns so there's no way to create an
823+
* index on a system column; but maybe someday that wouldn't be true,
824+
* so let's be safe.
825+
*/
826+
intnumatts=indexStruct->indnatts;
827+
inti;
828+
829+
for (i=0;i<numatts;i++)
830+
{
831+
intattnum=indexStruct->indkey.values[i];
832+
833+
if (attnum <=0)
834+
return false;
835+
}
836+
return true;
837+
}
838+
return false;
839+
}
840+
778841

779842
/*
780843
* 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