On 08.11.2018 22:05, Alvaro Herrera wrote:
On 2018-Nov-08, Konstantin Knizhnik wrote:
Before doing any other refactoring of projection indexes I want to attach
small bug fix patch which
fixes the original problem (SIGSEGV) and also disables recheck_on_update by
default.
As Laurenz has suggested, I replaced boolean recheck_on_update option with
"on","auto,"off" (default).
I think this causes an ABI break for GenericIndexOpts. Not sure to what
extent that is an actual problem (i.e. how many modules were compiled
with 11.0 that are gonna be reading that struct with later Pg), but I
think it should be avoided anyhow.
Ok, I reverted back my change of reach_on_update option type.
Now if reach_on_update option is not explicitly specified, then
decision is made based on the expression cost.
Patches becomes very small and fix only error in comparison of index
tuple values.
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index fb63471..81ab0ac 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4546,9 +4546,10 @@ ProjIndexIsUnchanged(Relation relation, HeapTuple oldtup, HeapTuple newtup)
}
else if (!old_isnull[i])
{
- Form_pg_attribute att = TupleDescAttr(RelationGetDescr(indexDesc), i);
-
- if (!datumIsEqual(old_values[i], new_values[i], att->attbyval, att->attlen))
+ int16 elmlen;
+ bool elmbyval;
+ get_typlenbyval(indexDesc->rd_opcintype[i], &elmlen, &elmbyval);
+ if (!datumIsEqual(old_values[i], new_values[i], elmbyval, elmlen))
{
equals = false;
break;
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index fd3d010..ed8a01e 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4745,10 +4745,11 @@ RelationGetIndexPredicate(Relation relation)
* for old and new tuple values.
*
* Decision made by this function is based on two sources:
- * 1. Calculated cost of index expression: if greater than some heuristic limit
- then extra comparison of index expression values is expected to be too
- expensive, so we don't attempt it by default.
- * 2. "recheck_on_update" index option explicitly set by user, which overrides 1)
+ * 1. If "recheck_on_update" is true, then index is considered as projection.
+ * 2. If "recheck_on_update" is false, then changing any column on which index depends disables hot update
+ * 3. If "recheck_on_update" is not explicitly specified then recheck on update is
+ * enabled if cost of index expression is not greater than some heuristic limit (1000).
+ * In this case extra comparison of index expression values is expected to be too expensive.
*/
static bool
IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
@@ -4760,26 +4761,9 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
HeapTuple tuple;
Datum reloptions;
bool isnull;
+ bool is_explicitly_set = false;
QualCost index_expr_cost;
- /* by default functional index is considered as non-injective */
- is_projection = true;
-
- cost_qual_eval(&index_expr_cost, ii->ii_Expressions, NULL);
-
- /*
- * If index expression is too expensive, then disable projection
- * optimization, because extra evaluation of index expression is
- * expected to be more expensive than index update. Currently the
- * projection optimization has to calculate index expression twice
- * when the value of index expression has not changed and three times
- * when values differ because the expression is recalculated when
- * inserting a new index entry for the changed value.
- */
- if ((index_expr_cost.startup + index_expr_cost.per_tuple) >
- HEURISTIC_MAX_HOT_RECHECK_EXPR_COST)
- is_projection = false;
-
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(RelationGetRelid(index)));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for relation %u", RelationGetRelid(index));
@@ -4795,9 +4779,25 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
if (idxopts != NULL)
{
is_projection = idxopts->recheck_on_update;
+ is_explicitly_set = true;
pfree(idxopts);
}
}
+ if (!is_explicitly_set)
+ {
+ cost_qual_eval(&index_expr_cost, ii->ii_Expressions, NULL);
+ /*
+ * If index expression is too expensive, then disable projection
+ * optimization, because extra evaluation of index expression is
+ * expected to be more expensive than index update. Currently the
+ * projection optimization has to calculate index expression twice
+ * when the value of index expression has not changed and three times
+ * when values differ because the expression is recalculated when
+ * inserting a new index entry for the changed value.
+ */
+ is_projection = (index_expr_cost.startup + index_expr_cost.per_tuple)
+ <= HEURISTIC_MAX_HOT_RECHECK_EXPR_COST;
+ }
ReleaseSysCache(tuple);
}
return is_projection;
