From: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Laurenz Albe <laurenz.albe@cybertec.at>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Simon Riggs <simon@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: Christoph Berg <myon@debian.org>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] Surjective functional indexes
Date: Fri, 9 Nov 2018 20:23:42 +0300
Message-ID: <1268584c-cd53-386d-c519-0681e92532c9@postgrespro.ru> (raw)
In-Reply-To: <20181108190559.ylzufqr2jjvml32f@alvherre.pgsql>
References: <20181108190559.ylzufqr2jjvml32f@alvherre.pgsql>
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
Attachments:projection-11.patch (text/x-patch, 4.0K) downloaddiff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex 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.cindex 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;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: k.knizhnik@postgrespro.ru, alvherre@2ndquadrant.com, laurenz.albe@cybertec.at, tgl@sss.pgh.pa.us, simon@2ndquadrant.com, robertmhaas@gmail.com, andres@anarazel.de, sfrost@snowman.net, michael.paquier@gmail.com, myon@debian.org, thomas.munro@enterprisedb.com, peter.eisentraut@2ndquadrant.com
Subject: Re: [HACKERS] Surjective functional indexes
In-Reply-To: <1268584c-cd53-386d-c519-0681e92532c9@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox