fix-corner-case-matview-refresh-issues-1.patch
text/x-diff
Filename: fix-corner-case-matview-refresh-issues-1.patch
Type: text/x-diff
Part: 0
Patch
Format: unified
| File | + | − |
|---|---|---|
| src/backend/commands/matview.c | 0 | 0 |
| src/backend/utils/adt/ri_triggers.c | 0 | 0 |
| src/backend/utils/adt/ruleutils.c | 0 | 0 |
| src/include/utils/builtins.h | 0 | 0 |
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index ab6a889..7159a27 100644
*** a/src/backend/commands/matview.c
--- b/src/backend/commands/matview.c
***************
*** 21,26 ****
--- 21,28 ----
#include "catalog/catalog.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
+ #include "catalog/pg_am.h"
+ #include "catalog/pg_opclass.h"
#include "catalog/pg_operator.h"
#include "commands/cluster.h"
#include "commands/matview.h"
***************
*** 40,46 ****
#include "utils/rel.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
- #include "utils/typcache.h"
typedef struct
--- 42,47 ----
*************** static void transientrel_shutdown(DestRe
*** 62,75 ****
static void transientrel_destroy(DestReceiver *self);
static uint64 refresh_matview_datafill(DestReceiver *dest, Query *query,
const char *queryString);
-
static char *make_temptable_name_n(char *tempname, int n);
- static void mv_GenerateOper(StringInfo buf, Oid opoid);
-
static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
int save_sec_context);
static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence);
!
static void OpenMatViewIncrementalMaintenance(void);
static void CloseMatViewIncrementalMaintenance(void);
--- 63,73 ----
static void transientrel_destroy(DestReceiver *self);
static uint64 refresh_matview_datafill(DestReceiver *dest, Query *query,
const char *queryString);
static char *make_temptable_name_n(char *tempname, int n);
static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
int save_sec_context);
static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence);
! static bool is_usable_unique_index(Relation indexRel);
static void OpenMatViewIncrementalMaintenance(void);
static void CloseMatViewIncrementalMaintenance(void);
*************** ExecRefreshMatView(RefreshMatViewStmt *s
*** 230,252 ****
{
Oid indexoid = lfirst_oid(indexoidscan);
Relation indexRel;
- Form_pg_index indexStruct;
indexRel = index_open(indexoid, AccessShareLock);
! indexStruct = indexRel->rd_index;
!
! if (indexStruct->indisunique &&
! IndexIsValid(indexStruct) &&
! RelationGetIndexExpressions(indexRel) == NIL &&
! RelationGetIndexPredicate(indexRel) == NIL &&
! indexStruct->indnatts > 0)
! {
! hasUniqueIndex = true;
! index_close(indexRel, AccessShareLock);
! break;
! }
!
index_close(indexRel, AccessShareLock);
}
list_free(indexoidlist);
--- 228,239 ----
{
Oid indexoid = lfirst_oid(indexoidscan);
Relation indexRel;
indexRel = index_open(indexoid, AccessShareLock);
! hasUniqueIndex = is_usable_unique_index(indexRel);
index_close(indexRel, AccessShareLock);
+ if (hasUniqueIndex)
+ break;
}
list_free(indexoidlist);
*************** make_temptable_name_n(char *tempname, in
*** 557,581 ****
return namebuf.data;
}
- static void
- mv_GenerateOper(StringInfo buf, Oid opoid)
- {
- HeapTuple opertup;
- Form_pg_operator operform;
-
- opertup = SearchSysCache1(OPEROID, ObjectIdGetDatum(opoid));
- if (!HeapTupleIsValid(opertup))
- elog(ERROR, "cache lookup failed for operator %u", opoid);
- operform = (Form_pg_operator) GETSTRUCT(opertup);
- Assert(operform->oprkind == 'b');
-
- appendStringInfo(buf, "OPERATOR(%s.%s)",
- quote_identifier(get_namespace_name(operform->oprnamespace)),
- NameStr(operform->oprname));
-
- ReleaseSysCache(opertup);
- }
-
/*
* refresh_by_match_merge
*
--- 544,549 ----
*************** refresh_by_match_merge(Oid matviewOid, O
*** 623,629 ****
List *indexoidlist;
ListCell *indexoidscan;
int16 relnatts;
! bool *usedForQual;
initStringInfo(&querybuf);
matviewRel = heap_open(matviewOid, NoLock);
--- 591,597 ----
List *indexoidlist;
ListCell *indexoidscan;
int16 relnatts;
! Oid *opUsedForQual;
initStringInfo(&querybuf);
matviewRel = heap_open(matviewOid, NoLock);
*************** refresh_by_match_merge(Oid matviewOid, O
*** 635,641 ****
diffname = make_temptable_name_n(tempname, 2);
relnatts = matviewRel->rd_rel->relnatts;
- usedForQual = (bool *) palloc0(sizeof(bool) * relnatts);
/* Open SPI context. */
if (SPI_connect() != SPI_OK_CONNECT)
--- 603,608 ----
*************** refresh_by_match_merge(Oid matviewOid, O
*** 699,744 ****
* include all rows.
*/
tupdesc = matviewRel->rd_att;
foundUniqueIndex = false;
indexoidlist = RelationGetIndexList(matviewRel);
foreach(indexoidscan, indexoidlist)
{
Oid indexoid = lfirst_oid(indexoidscan);
Relation indexRel;
- Form_pg_index indexStruct;
indexRel = index_open(indexoid, RowExclusiveLock);
! indexStruct = indexRel->rd_index;
!
! /*
! * We're only interested if it is unique, valid, contains no
! * expressions, and is not partial.
! */
! if (indexStruct->indisunique &&
! IndexIsValid(indexStruct) &&
! RelationGetIndexExpressions(indexRel) == NIL &&
! RelationGetIndexPredicate(indexRel) == NIL)
{
int numatts = indexStruct->indnatts;
int i;
/* Add quals for all columns from this index. */
for (i = 0; i < numatts; i++)
{
int attnum = indexStruct->indkey.values[i];
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
! Oid type;
Oid op;
! const char *colname;
/*
! * Only include the column once regardless of how many times
! * it shows up in how many indexes.
*/
! if (usedForQual[attnum - 1])
continue;
! usedForQual[attnum - 1] = true;
/*
* Actually add the qual, ANDed with any others.
--- 666,747 ----
* include all rows.
*/
tupdesc = matviewRel->rd_att;
+ opUsedForQual = (Oid *) palloc0(sizeof(Oid) * relnatts);
foundUniqueIndex = false;
+
indexoidlist = RelationGetIndexList(matviewRel);
foreach(indexoidscan, indexoidlist)
{
Oid indexoid = lfirst_oid(indexoidscan);
Relation indexRel;
indexRel = index_open(indexoid, RowExclusiveLock);
! if (is_usable_unique_index(indexRel))
{
+ Form_pg_index indexStruct = indexRel->rd_index;
int numatts = indexStruct->indnatts;
+ oidvector *indclass;
+ Datum indclassDatum;
+ bool isnull;
int i;
+ /* Must get indclass the hard way. */
+ indclassDatum = SysCacheGetAttr(INDEXRELID,
+ indexRel->rd_indextuple,
+ Anum_pg_index_indclass,
+ &isnull);
+ Assert(!isnull);
+ indclass = (oidvector *) DatumGetPointer(indclassDatum);
+
/* Add quals for all columns from this index. */
for (i = 0; i < numatts; i++)
{
int attnum = indexStruct->indkey.values[i];
+ Oid opclass = indclass->values[i];
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
! Oid attrtype = attr->atttypid;
! HeapTuple cla_ht;
! Form_pg_opclass cla_tup;
! Oid opfamily;
! Oid opcintype;
Oid op;
! const char *leftop;
! const char *rightop;
/*
! * Identify the equality operator associated with this index
! * column. First we need to look up the column's opclass.
*/
! cla_ht = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclass));
! if (!HeapTupleIsValid(cla_ht))
! elog(ERROR, "cache lookup failed for opclass %u", opclass);
! cla_tup = (Form_pg_opclass) GETSTRUCT(cla_ht);
! Assert(cla_tup->opcmethod == BTREE_AM_OID);
! opfamily = cla_tup->opcfamily;
! opcintype = cla_tup->opcintype;
! ReleaseSysCache(cla_ht);
!
! op = get_opfamily_member(opfamily, opcintype, opcintype,
! BTEqualStrategyNumber);
! if (!OidIsValid(op))
! elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
! BTEqualStrategyNumber, opcintype, opcintype, opfamily);
!
! /*
! * If we find the same column with the same equality semantics
! * in more than one index, we only need to emit the equality
! * clause once.
! *
! * Since we only remember the last equality operator, this
! * code could be fooled into emitting duplicate clauses given
! * multiple indexes with several different opclasses ... but
! * that's so unlikely it doesn't seem worth spending extra
! * code to avoid.
! */
! if (opUsedForQual[attnum - 1] == op)
continue;
! opUsedForQual[attnum - 1] = op;
/*
* Actually add the qual, ANDed with any others.
*************** refresh_by_match_merge(Oid matviewOid, O
*** 746,757 ****
if (foundUniqueIndex)
appendStringInfoString(&querybuf, " AND ");
! colname = quote_identifier(NameStr(attr->attname));
! appendStringInfo(&querybuf, "newdata.%s ", colname);
! type = attnumTypeId(matviewRel, attnum);
! op = lookup_type_cache(type, TYPECACHE_EQ_OPR)->eq_opr;
! mv_GenerateOper(&querybuf, op);
! appendStringInfo(&querybuf, " mv.%s", colname);
foundUniqueIndex = true;
}
--- 749,763 ----
if (foundUniqueIndex)
appendStringInfoString(&querybuf, " AND ");
! leftop = quote_qualified_identifier("newdata",
! NameStr(attr->attname));
! rightop = quote_qualified_identifier("mv",
! NameStr(attr->attname));
!
! generate_operator_clause(&querybuf,
! leftop, attrtype,
! op,
! rightop, attrtype);
foundUniqueIndex = true;
}
*************** refresh_by_match_merge(Oid matviewOid, O
*** 764,774 ****
list_free(indexoidlist);
/*
! * There must be at least one unique index on the matview.
*
* ExecRefreshMatView() checks that after taking the exclusive lock on the
* matview. So at least one unique index is guaranteed to exist here
! * because the lock is still being held.
*/
Assert(foundUniqueIndex);
--- 770,780 ----
list_free(indexoidlist);
/*
! * There must be at least one usable unique index on the matview.
*
* ExecRefreshMatView() checks that after taking the exclusive lock on the
* matview. So at least one unique index is guaranteed to exist here
! * because the lock is still being held; so an Assert seems sufficient.
*/
Assert(foundUniqueIndex);
*************** refresh_by_heap_swap(Oid matviewOid, Oid
*** 845,850 ****
--- 851,901 ----
RecentXmin, ReadNextMultiXactId(), relpersistence);
}
+ /*
+ * Check whether specified index is usable for match merge.
+ */
+ static bool
+ is_usable_unique_index(Relation indexRel)
+ {
+ Form_pg_index indexStruct = indexRel->rd_index;
+
+ /*
+ * Must be unique, valid, immediate, non-partial, and be defined over
+ * plain user columns (not expressions). We also require it to be a
+ * btree. Even if we had any other unique index kinds, we'd not know how
+ * to identify the corresponding equality operator, nor could we be sure
+ * that the planner could implement the required FULL JOIN with non-btree
+ * operators.
+ */
+ if (indexStruct->indisunique &&
+ indexStruct->indimmediate &&
+ indexRel->rd_rel->relam == BTREE_AM_OID &&
+ IndexIsValid(indexStruct) &&
+ RelationGetIndexPredicate(indexRel) == NIL &&
+ indexStruct->indnatts > 0)
+ {
+ /*
+ * The point of groveling through the index columns individually is to
+ * reject both index expressions and system columns. Currently,
+ * matviews couldn't have OID columns so there's no way to create an
+ * index on a system column; but maybe someday that wouldn't be true,
+ * so let's be safe.
+ */
+ int numatts = indexStruct->indnatts;
+ int i;
+
+ for (i = 0; i < numatts; i++)
+ {
+ int attnum = indexStruct->indkey.values[i];
+
+ if (attnum <= 0)
+ return false;
+ }
+ return true;
+ }
+ return false;
+ }
+
/*
* This should be used to test whether the backend is in a context where it is
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 8faae1d..4d7fee0 100644
*** a/src/backend/utils/adt/ri_triggers.c
--- b/src/backend/utils/adt/ri_triggers.c
*************** static void ri_GenerateQual(StringInfo b
*** 203,209 ****
const char *leftop, Oid leftoptype,
Oid opoid,
const char *rightop, Oid rightoptype);
- static void ri_add_cast_to(StringInfo buf, Oid typid);
static void ri_GenerateQualCollation(StringInfo buf, Oid collation);
static int ri_NullCheck(HeapTuple tup,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
--- 203,208 ----
*************** quoteRelationName(char *buffer, Relation
*** 2134,2146 ****
/*
* ri_GenerateQual --- generate a WHERE clause equating two variables
*
! * The idea is to append " sep leftop op rightop" to buf. The complexity
! * comes from needing to be sure that the parser will select the desired
! * operator. We always name the operator using OPERATOR(schema.op) syntax
! * (readability isn't a big priority here), so as to avoid search-path
! * uncertainties. We have to emit casts too, if either input isn't already
! * the input type of the operator; else we are at the mercy of the parser's
! * heuristics for ambiguous-operator resolution.
*/
static void
ri_GenerateQual(StringInfo buf,
--- 2133,2142 ----
/*
* ri_GenerateQual --- generate a WHERE clause equating two variables
*
! * This basically appends " sep leftop op rightop" to buf, adding casts
! * and schema qualification as needed to ensure that the parser will select
! * the operator we specify. leftop and rightop should be parenthesized
! * if they aren't variables or parameters.
*/
static void
ri_GenerateQual(StringInfo buf,
*************** ri_GenerateQual(StringInfo buf,
*** 2149,2208 ****
Oid opoid,
const char *rightop, Oid rightoptype)
{
! HeapTuple opertup;
! Form_pg_operator operform;
! char *oprname;
! char *nspname;
!
! opertup = SearchSysCache1(OPEROID, ObjectIdGetDatum(opoid));
! if (!HeapTupleIsValid(opertup))
! elog(ERROR, "cache lookup failed for operator %u", opoid);
! operform = (Form_pg_operator) GETSTRUCT(opertup);
! Assert(operform->oprkind == 'b');
! oprname = NameStr(operform->oprname);
!
! nspname = get_namespace_name(operform->oprnamespace);
!
! appendStringInfo(buf, " %s %s", sep, leftop);
! if (leftoptype != operform->oprleft)
! ri_add_cast_to(buf, operform->oprleft);
! appendStringInfo(buf, " OPERATOR(%s.", quote_identifier(nspname));
! appendStringInfoString(buf, oprname);
! appendStringInfo(buf, ") %s", rightop);
! if (rightoptype != operform->oprright)
! ri_add_cast_to(buf, operform->oprright);
!
! ReleaseSysCache(opertup);
! }
!
! /*
! * Add a cast specification to buf. We spell out the type name the hard way,
! * intentionally not using format_type_be(). This is to avoid corner cases
! * for CHARACTER, BIT, and perhaps other types, where specifying the type
! * using SQL-standard syntax results in undesirable data truncation. By
! * doing it this way we can be certain that the cast will have default (-1)
! * target typmod.
! */
! static void
! ri_add_cast_to(StringInfo buf, Oid typid)
! {
! HeapTuple typetup;
! Form_pg_type typform;
! char *typname;
! char *nspname;
!
! typetup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typid));
! if (!HeapTupleIsValid(typetup))
! elog(ERROR, "cache lookup failed for type %u", typid);
! typform = (Form_pg_type) GETSTRUCT(typetup);
!
! typname = NameStr(typform->typname);
! nspname = get_namespace_name(typform->typnamespace);
!
! appendStringInfo(buf, "::%s.%s",
! quote_identifier(nspname), quote_identifier(typname));
!
! ReleaseSysCache(typetup);
}
/*
--- 2145,2153 ----
Oid opoid,
const char *rightop, Oid rightoptype)
{
! appendStringInfo(buf, " %s ", sep);
! generate_operator_clause(buf, leftop, leftoptype, opoid,
! rightop, rightoptype);
}
/*
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b58ee3c..2cd54ec 100644
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
*************** static char *generate_function_name(Oid
*** 462,467 ****
--- 462,468 ----
bool has_variadic, bool *use_variadic_p,
ParseExprKind special_exprkind);
static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2);
+ static void add_cast_to(StringInfo buf, Oid typid);
static char *generate_qualified_type_name(Oid typid);
static text *string_to_text(char *str);
static char *flatten_reloptions(Oid relid);
*************** generate_operator_name(Oid operid, Oid a
*** 10850,10855 ****
--- 10851,10935 ----
}
/*
+ * generate_operator_clause --- generate a binary-operator WHERE clause
+ *
+ * This is used for internally-generated-and-executed SQL queries, where
+ * precision is essential and readability is secondary. The basic
+ * requirement is to append "leftop op rightop" to buf, where leftop and
+ * rightop are given as strings and are assumed to yield types leftoptype
+ * and rightoptype; the operator is identified by OID. The complexity
+ * comes from needing to be sure that the parser will select the desired
+ * operator when the query is parsed. We always name the operator using
+ * OPERATOR(schema.op) syntax, so as to avoid search-path uncertainties.
+ * We have to emit casts too, if either input isn't already the input type
+ * of the operator; else we are at the mercy of the parser's heuristics for
+ * ambiguous-operator resolution. The caller must ensure that leftop and
+ * rightop are suitable arguments for a cast operation; it's best to insert
+ * parentheses if they aren't just variables or parameters.
+ */
+ void
+ generate_operator_clause(StringInfo buf,
+ const char *leftop, Oid leftoptype,
+ Oid opoid,
+ const char *rightop, Oid rightoptype)
+ {
+ HeapTuple opertup;
+ Form_pg_operator operform;
+ char *oprname;
+ char *nspname;
+
+ opertup = SearchSysCache1(OPEROID, ObjectIdGetDatum(opoid));
+ if (!HeapTupleIsValid(opertup))
+ elog(ERROR, "cache lookup failed for operator %u", opoid);
+ operform = (Form_pg_operator) GETSTRUCT(opertup);
+ Assert(operform->oprkind == 'b');
+ oprname = NameStr(operform->oprname);
+
+ nspname = get_namespace_name(operform->oprnamespace);
+
+ appendStringInfoString(buf, leftop);
+ if (leftoptype != operform->oprleft)
+ add_cast_to(buf, operform->oprleft);
+ appendStringInfo(buf, " OPERATOR(%s.", quote_identifier(nspname));
+ appendStringInfoString(buf, oprname);
+ appendStringInfo(buf, ") %s", rightop);
+ if (rightoptype != operform->oprright)
+ add_cast_to(buf, operform->oprright);
+
+ ReleaseSysCache(opertup);
+ }
+
+ /*
+ * Add a cast specification to buf. We spell out the type name the hard way,
+ * intentionally not using format_type_be(). This is to avoid corner cases
+ * for CHARACTER, BIT, and perhaps other types, where specifying the type
+ * using SQL-standard syntax results in undesirable data truncation. By
+ * doing it this way we can be certain that the cast will have default (-1)
+ * target typmod.
+ */
+ static void
+ add_cast_to(StringInfo buf, Oid typid)
+ {
+ HeapTuple typetup;
+ Form_pg_type typform;
+ char *typname;
+ char *nspname;
+
+ typetup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typid));
+ if (!HeapTupleIsValid(typetup))
+ elog(ERROR, "cache lookup failed for type %u", typid);
+ typform = (Form_pg_type) GETSTRUCT(typetup);
+
+ typname = NameStr(typform->typname);
+ nspname = get_namespace_name(typform->typnamespace);
+
+ appendStringInfo(buf, "::%s.%s",
+ quote_identifier(nspname), quote_identifier(typname));
+
+ ReleaseSysCache(typetup);
+ }
+
+ /*
* generate_qualified_type_name
* Compute the name to display for a type specified by OID
*
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 01e76ae..d0416e9 100644
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
*************** extern bool quote_all_identifiers;
*** 78,83 ****
--- 78,87 ----
extern const char *quote_identifier(const char *ident);
extern char *quote_qualified_identifier(const char *qualifier,
const char *ident);
+ extern void generate_operator_clause(fmStringInfo buf,
+ const char *leftop, Oid leftoptype,
+ Oid opoid,
+ const char *rightop, Oid rightoptype);
/* varchar.c */
extern int bpchartruelen(char *s, int len);