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

Commite1f186d

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 parentb6ba94e commite1f186d

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
@@ -20,6 +20,8 @@
2020
#include"catalog/catalog.h"
2121
#include"catalog/indexing.h"
2222
#include"catalog/namespace.h"
23+
#include"catalog/pg_am.h"
24+
#include"catalog/pg_opclass.h"
2325
#include"catalog/pg_operator.h"
2426
#include"commands/cluster.h"
2527
#include"commands/matview.h"
@@ -38,7 +40,6 @@
3840
#include"utils/rel.h"
3941
#include"utils/snapmgr.h"
4042
#include"utils/syscache.h"
41-
#include"utils/typcache.h"
4243

4344

4445
typedefstruct
@@ -60,14 +61,11 @@ static void transientrel_shutdown(DestReceiver *self);
6061
staticvoidtransientrel_destroy(DestReceiver*self);
6162
staticvoidrefresh_matview_datafill(DestReceiver*dest,Query*query,
6263
constchar*queryString);
63-
6464
staticchar*make_temptable_name_n(char*tempname,intn);
65-
staticvoidmv_GenerateOper(StringInfobuf,Oidopoid);
66-
6765
staticvoidrefresh_by_match_merge(OidmatviewOid,OidtempOid,Oidrelowner,
6866
intsave_sec_context);
6967
staticvoidrefresh_by_heap_swap(OidmatviewOid,OidOIDNewHeap);
70-
68+
staticboolis_usable_unique_index(RelationindexRel);
7169
staticvoidOpenMatViewIncrementalMaintenance(void);
7270
staticvoidCloseMatViewIncrementalMaintenance(void);
7371

@@ -477,25 +475,6 @@ make_temptable_name_n(char *tempname, int n)
477475
returnnamebuf.data;
478476
}
479477

480-
staticvoid
481-
mv_GenerateOper(StringInfobuf,Oidopoid)
482-
{
483-
HeapTupleopertup;
484-
Form_pg_operatoroperform;
485-
486-
opertup=SearchSysCache1(OPEROID,ObjectIdGetDatum(opoid));
487-
if (!HeapTupleIsValid(opertup))
488-
elog(ERROR,"cache lookup failed for operator %u",opoid);
489-
operform= (Form_pg_operator)GETSTRUCT(opertup);
490-
Assert(operform->oprkind=='b');
491-
492-
appendStringInfo(buf,"OPERATOR(%s.%s)",
493-
quote_identifier(get_namespace_name(operform->oprnamespace)),
494-
NameStr(operform->oprname));
495-
496-
ReleaseSysCache(opertup);
497-
}
498-
499478
/*
500479
* refresh_by_match_merge
501480
*
@@ -543,7 +522,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
543522
List*indexoidlist;
544523
ListCell*indexoidscan;
545524
int16relnatts;
546-
bool*usedForQual;
525+
Oid*opUsedForQual;
547526

548527
initStringInfo(&querybuf);
549528
matviewRel=heap_open(matviewOid,NoLock);
@@ -555,7 +534,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
555534
diffname=make_temptable_name_n(tempname,2);
556535

557536
relnatts=matviewRel->rd_rel->relnatts;
558-
usedForQual= (bool*)palloc0(sizeof(bool)*relnatts);
559537

560538
/* Open SPI context. */
561539
if (SPI_connect()!=SPI_OK_CONNECT)
@@ -619,58 +597,98 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
619597
* include all rows.
620598
*/
621599
tupdesc=matviewRel->rd_att;
600+
opUsedForQual= (Oid*)palloc0(sizeof(Oid)*relnatts);
622601
foundUniqueIndex= false;
602+
623603
indexoidlist=RelationGetIndexList(matviewRel);
624604

625605
foreach(indexoidscan,indexoidlist)
626606
{
627607
Oidindexoid=lfirst_oid(indexoidscan);
628608
RelationindexRel;
629-
Form_pg_indexindexStruct;
630609

631610
indexRel=index_open(indexoid,RowExclusiveLock);
632-
indexStruct=indexRel->rd_index;
633-
634-
/*
635-
* We're only interested if it is unique, valid, contains no
636-
* expressions, and is not partial.
637-
*/
638-
if (indexStruct->indisunique&&
639-
IndexIsValid(indexStruct)&&
640-
RelationGetIndexExpressions(indexRel)==NIL&&
641-
RelationGetIndexPredicate(indexRel)==NIL)
611+
if (is_usable_unique_index(indexRel))
642612
{
613+
Form_pg_indexindexStruct=indexRel->rd_index;
643614
intnumatts=indexStruct->indnatts;
615+
oidvector*indclass;
616+
DatumindclassDatum;
617+
boolisnull;
644618
inti;
645619

620+
/* Must get indclass the hard way. */
621+
indclassDatum=SysCacheGetAttr(INDEXRELID,
622+
indexRel->rd_indextuple,
623+
Anum_pg_index_indclass,
624+
&isnull);
625+
Assert(!isnull);
626+
indclass= (oidvector*)DatumGetPointer(indclassDatum);
627+
646628
/* Add quals for all columns from this index. */
647629
for (i=0;i<numatts;i++)
648630
{
649631
intattnum=indexStruct->indkey.values[i];
650-
Oidtype;
632+
Oidopclass=indclass->values[i];
633+
Form_pg_attributeattr=TupleDescAttr(tupdesc,attnum-1);
634+
Oidattrtype=attr->atttypid;
635+
HeapTuplecla_ht;
636+
Form_pg_opclasscla_tup;
637+
Oidopfamily;
638+
Oidopcintype;
651639
Oidop;
652-
constchar*colname;
640+
constchar*leftop;
641+
constchar*rightop;
642+
643+
/*
644+
* Identify the equality operator associated with this index
645+
* column. First we need to look up the column's opclass.
646+
*/
647+
cla_ht=SearchSysCache1(CLAOID,ObjectIdGetDatum(opclass));
648+
if (!HeapTupleIsValid(cla_ht))
649+
elog(ERROR,"cache lookup failed for opclass %u",opclass);
650+
cla_tup= (Form_pg_opclass)GETSTRUCT(cla_ht);
651+
Assert(cla_tup->opcmethod==BTREE_AM_OID);
652+
opfamily=cla_tup->opcfamily;
653+
opcintype=cla_tup->opcintype;
654+
ReleaseSysCache(cla_ht);
655+
656+
op=get_opfamily_member(opfamily,opcintype,opcintype,
657+
BTEqualStrategyNumber);
658+
if (!OidIsValid(op))
659+
elog(ERROR,"missing operator %d(%u,%u) in opfamily %u",
660+
BTEqualStrategyNumber,opcintype,opcintype,opfamily);
653661

654662
/*
655-
* Only include the column once regardless of how many times
656-
* it shows up in how many indexes.
663+
* If we find the same column with the same equality semantics
664+
* in more than one index, we only need to emit the equality
665+
* clause once.
666+
*
667+
* Since we only remember the last equality operator, this
668+
* code could be fooled into emitting duplicate clauses given
669+
* multiple indexes with several different opclasses ... but
670+
* that's so unlikely it doesn't seem worth spending extra
671+
* code to avoid.
657672
*/
658-
if (usedForQual[attnum-1])
673+
if (opUsedForQual[attnum-1]==op)
659674
continue;
660-
usedForQual[attnum-1]=true;
675+
opUsedForQual[attnum-1]=op;
661676

662677
/*
663678
* Actually add the qual, ANDed with any others.
664679
*/
665680
if (foundUniqueIndex)
666681
appendStringInfoString(&querybuf," AND ");
667682

668-
colname=quote_identifier(NameStr((tupdesc->attrs[attnum-1])->attname));
669-
appendStringInfo(&querybuf,"newdata.%s ",colname);
670-
type=attnumTypeId(matviewRel,attnum);
671-
op=lookup_type_cache(type,TYPECACHE_EQ_OPR)->eq_opr;
672-
mv_GenerateOper(&querybuf,op);
673-
appendStringInfo(&querybuf," mv.%s",colname);
683+
leftop=quote_qualified_identifier("newdata",
684+
NameStr(attr->attname));
685+
rightop=quote_qualified_identifier("mv",
686+
NameStr(attr->attname));
687+
688+
generate_operator_clause(&querybuf,
689+
leftop,attrtype,
690+
op,
691+
rightop,attrtype);
674692

675693
foundUniqueIndex= true;
676694
}
@@ -762,6 +780,51 @@ refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap)
762780
RecentXmin,ReadNextMultiXactId());
763781
}
764782

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

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

26212566
/*

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp