On 08.11.2018 15:23, Laurenz Albe wrote:
Tom Lane wrote:
I wanted to enumerate my concerns while yesterday's
events are still fresh in mind. (Andres or Robert might have more.)
* I do not understand why this feature is on-by-default in the first
place. It can only be a win for expression indexes that are many-to-one
mappings; for indexes that are one-to-one or few-to-one, it's a pretty
big loss. I see no reason to assume that most expression indexes fall
into the first category. I suggest that the design ought to be to use
this optimization only for indexes for which the user has explicitly
enabled recheck_on_update. That would allow getting rid of the cost check
in IsProjectionFunctionalIndex, about which we'd otherwise have to have
an additional fight: I do not like its ad-hoc-ness, nor the modularity
violation (and potential circularity) involved in having the relcache call
cost_qual_eval.
That was my impression too when I had a closer look at this feature.
What about an option "hot_update_check" with values "off" (default),
"on" and "always"?
Yours,
Laurenz Albe
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).
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 4df3d75..e17dd09 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -365,8 +365,9 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
<listitem>
<para>
Specifies whether to recheck a functional index value to see whether
- we can use a HOT update or not. The default value is on for functional
- indexes with an total expression cost less than 1000, otherwise off.
+ we can use a HOT update or not. Accepted values are "on", "auto" and "off"
+ (default). In case of "auto", recheck is done for indexes with an total expression
+ cost less than 1000.
You might decide to turn this off if you knew that a function used in
an index is unlikely to return the same value when one of the input
columns is updated and so the recheck is not worth the additional cost
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index db84da0..9fb5ebc 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -19,6 +19,7 @@
#include "access/gist_private.h"
#include "access/hash.h"
+#include "access/heapam.h"
#include "access/htup_details.h"
#include "access/nbtree.h"
#include "access/reloptions.h"
@@ -131,15 +132,6 @@ static relopt_bool boolRelOpts[] =
},
{
{
- "recheck_on_update",
- "Recheck functional index expression for changed value after update",
- RELOPT_KIND_INDEX,
- ShareUpdateExclusiveLock /* since only applies to later UPDATEs */
- },
- true
- },
- {
- {
"security_barrier",
"View acts as a row security barrier",
RELOPT_KIND_VIEW,
@@ -448,6 +440,18 @@ static relopt_string stringRelOpts[] =
validateWithCheckOption,
NULL
},
+ {
+ {
+ "recheck_on_update",
+ "Recheck functional index expression for changed value after update",
+ RELOPT_KIND_INDEX,
+ ShareUpdateExclusiveLock /* since only applies to later UPDATEs */
+ },
+ 3,
+ false,
+ validateRecheckOnUpdateOption,
+ "off"
+ },
/* list terminator */
{{NULL}}
};
@@ -1499,7 +1503,7 @@ index_generic_reloptions(Datum reloptions, bool validate)
GenericIndexOpts *idxopts;
relopt_value *options;
static const relopt_parse_elt tab[] = {
- {"recheck_on_update", RELOPT_TYPE_BOOL, offsetof(GenericIndexOpts, recheck_on_update)}
+ {"recheck_on_update", RELOPT_TYPE_STRING, offsetof(GenericIndexOpts, recheck_on_update)}
};
options = parseRelOptions(reloptions, validate,
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index fb63471..21cdc1b 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4491,6 +4491,21 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
}
}
+
+void validateRecheckOnUpdateOption(const char *value)
+{
+ if (value == NULL ||
+ (strcmp(value, "on") != 0 &&
+ strcmp(value, "off") != 0 &&
+ strcmp(value, "auto") != 0))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for \"recheck_on_update\" option"),
+ errdetail("Valid values are \"on\", \"off\", and \"auto\".")));
+ }
+}
+
/*
* Check whether the value is unchanged after update of a projection
* functional index. Compare the new and old values of the indexed
@@ -4546,9 +4561,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..adbdc8a 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4762,24 +4762,6 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
bool isnull;
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));
@@ -4794,7 +4776,26 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
if (idxopts != NULL)
{
- is_projection = idxopts->recheck_on_update;
+ char* recheck_on_update = (char*)idxopts + idxopts->recheck_on_update;
+ if (strcmp(recheck_on_update, "on") == 0)
+ {
+ is_projection = true;
+ }
+ else if (strcmp(recheck_on_update, "auto") == 0)
+ {
+ 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;
+ }
pfree(idxopts);
}
}
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 40e153f..4c6ecc3 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -201,4 +201,6 @@ extern BlockNumber ss_get_location(Relation rel, BlockNumber relnblocks);
extern void SyncScanShmemInit(void);
extern Size SyncScanShmemSize(void);
+extern void validateRecheckOnUpdateOption(const char *value);
+
#endif /* HEAPAM_H */
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 84469f5..b87969a 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -224,7 +224,7 @@ typedef struct ForeignKeyCacheInfo
typedef struct GenericIndexOpts
{
int32 vl_len_;
- bool recheck_on_update;
+ int recheck_on_update;
} GenericIndexOpts;
/*
diff --git a/src/test/regress/expected/func_index.out b/src/test/regress/expected/func_index.out
index 307ac97..1d9f2ce 100644
--- a/src/test/regress/expected/func_index.out
+++ b/src/test/regress/expected/func_index.out
@@ -1,6 +1,6 @@
begin;
create table keyvalue(id integer primary key, info jsonb);
-create index nameindex on keyvalue((info->>'name')) with (recheck_on_update=false);
+create index nameindex on keyvalue((info->>'name')) with (recheck_on_update='off');
insert into keyvalue values (1, '{"name": "john", "data": "some data"}');
update keyvalue set info='{"name": "john", "data": "some other data"}' where id=1;
select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
@@ -12,7 +12,7 @@ select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
rollback;
begin;
create table keyvalue(id integer primary key, info jsonb);
-create index nameindex on keyvalue((info->>'name')) with (recheck_on_update=true);
+create index nameindex on keyvalue((info->>'name')) with (recheck_on_update='on');
insert into keyvalue values (1, '{"name": "john", "data": "some data"}');
update keyvalue set info='{"name": "john", "data": "some other data"}' where id=1;
select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
@@ -38,7 +38,7 @@ select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
rollback;
begin;
create table keyvalue(id integer primary key, info jsonb);
-create index nameindex on keyvalue((info->>'name'));
+create index nameindex on keyvalue((info->>'name')) with (recheck_on_update='auto');
insert into keyvalue values (1, '{"name": "john", "data": "some data"}');
update keyvalue set info='{"name": "john", "data": "some other data"}' where id=1;
select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
diff --git a/src/test/regress/sql/func_index.sql b/src/test/regress/sql/func_index.sql
index c267c93..cb140af 100644
--- a/src/test/regress/sql/func_index.sql
+++ b/src/test/regress/sql/func_index.sql
@@ -1,6 +1,6 @@
begin;
create table keyvalue(id integer primary key, info jsonb);
-create index nameindex on keyvalue((info->>'name')) with (recheck_on_update=false);
+create index nameindex on keyvalue((info->>'name')) with (recheck_on_update='off');
insert into keyvalue values (1, '{"name": "john", "data": "some data"}');
update keyvalue set info='{"name": "john", "data": "some other data"}' where id=1;
select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
@@ -8,7 +8,7 @@ rollback;
begin;
create table keyvalue(id integer primary key, info jsonb);
-create index nameindex on keyvalue((info->>'name')) with (recheck_on_update=true);
+create index nameindex on keyvalue((info->>'name')) with (recheck_on_update='on');
insert into keyvalue values (1, '{"name": "john", "data": "some data"}');
update keyvalue set info='{"name": "john", "data": "some other data"}' where id=1;
select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
@@ -20,7 +20,7 @@ rollback;
begin;
create table keyvalue(id integer primary key, info jsonb);
-create index nameindex on keyvalue((info->>'name'));
+create index nameindex on keyvalue((info->>'name')) with (recheck_on_update='auto');
insert into keyvalue values (1, '{"name": "john", "data": "some data"}');
update keyvalue set info='{"name": "john", "data": "some other data"}' where id=1;
select pg_stat_get_xact_tuples_hot_updated('keyvalue'::regclass);
