Skip to content

Commit 5d28c9b

Browse files
committed
Disable recheck_on_update optimization to avoid crashes.
The code added by commit c203d6c causes a crash in at least one case, where a potentially-optimizable expression index has a storage type different from the input data type. A cursory code review turned up numerous other problems that seem impractical to fix on short notice. Andres argued for revert of that patch some time ago, and if additional senior committers had been paying attention, that's likely what would have happened, but we were not :-( At this point we can't just revert, at least not in v11, because that would mean an ABI break for code touching relcache entries. And we should not remove the (also buggy) support for the recheck_on_update index reloption, since it might already be used in some databases in the field. So this patch just does the as-little-invasive-as-possible measure of disabling the feature as though recheck_on_update were forced off for all indexes. I also removed the related regression tests (which would otherwise fail) and the user-facing documentation of the reloption. We should undertake a more thorough code cleanup if the patch can't be fixed, but not under the extreme time pressure of being already overdue for 11.1 release. Per report from Ondřej Bouda and subsequent private discussion among pgsql-release. Discussion: https://postgr.es/m/20181106185255.776mstcyehnc63ty@alvherre.pgsql
1 parent c4f0876 commit 5d28c9b

File tree

7 files changed

+6
-145
lines changed

7 files changed

+6
-145
lines changed

doc/src/sgml/ref/create_index.sgml

+2-35
Original file line numberDiff line numberDiff line change
@@ -356,41 +356,8 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
356356
<para>
357357
The optional <literal>WITH</literal> clause specifies <firstterm>storage
358358
parameters</firstterm> for the index. Each index method has its own set of allowed
359-
storage parameters. All indexes accept the following parameter:
360-
</para>
361-
362-
<variablelist>
363-
<varlistentry>
364-
<term><literal>recheck_on_update</literal></term>
365-
<listitem>
366-
<para>
367-
Specifies whether to recheck a functional index value to see whether
368-
we can use a HOT update or not. The default value is on for functional
369-
indexes with an total expression cost less than 1000, otherwise off.
370-
You might decide to turn this off if you knew that a function used in
371-
an index is unlikely to return the same value when one of the input
372-
columns is updated and so the recheck is not worth the additional cost
373-
of executing the function.
374-
</para>
375-
376-
<para>
377-
Functional indexes are used frequently for the case where the function
378-
returns a subset of the argument. Examples of this would be accessing
379-
part of a string with <literal>SUBSTR()</literal> or accessing a single
380-
field in a JSON document using an expression such as
381-
<literal>(bookinfo-&gt;&gt;'isbn')</literal>. In this example, the JSON
382-
document might be updated frequently, yet it is uncommon for the ISBN
383-
field for a book to change so we would keep the parameter set to on
384-
for that index. A more frequently changing field might have an index
385-
with this parameter turned off, while very frequently changing fields
386-
might be better to avoid indexing at all under high load.
387-
</para>
388-
</listitem>
389-
</varlistentry>
390-
</variablelist>
391-
392-
<para>
393-
The B-tree, hash, GiST and SP-GiST index methods all accept this parameter:
359+
storage parameters. The B-tree, hash, GiST and SP-GiST index methods all
360+
accept this parameter:
394361
</para>
395362

396363
<variablelist>

doc/src/sgml/release-11.sgml

-13
Original file line numberDiff line numberDiff line change
@@ -1437,19 +1437,6 @@ same commits as above
14371437
</para>
14381438
</listitem>
14391439

1440-
<listitem>
1441-
<!--
1442-
2018-03-27 [c203d6cf8] Allow HOT updates for some expression indexes
1443-
-->
1444-
1445-
<para>
1446-
Allow heap-only-tuple (<acronym>HOT</acronym>) updates for
1447-
expression indexes when the values of the expressions are unchanged
1448-
(Konstantin Knizhnik)
1449-
</para>
1450-
1451-
</listitem>
1452-
14531440
</itemizedlist>
14541441

14551442
<sect5>

src/backend/utils/cache/relcache.c

+3
Original file line numberDiff line numberDiff line change
@@ -4755,6 +4755,7 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
47554755
{
47564756
bool is_projection = false;
47574757

4758+
#ifdef NOT_USED
47584759
if (ii->ii_Expressions)
47594760
{
47604761
HeapTuple tuple;
@@ -4800,6 +4801,8 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
48004801
}
48014802
ReleaseSysCache(tuple);
48024803
}
4804+
#endif
4805+
48034806
return is_projection;
48044807
}
48054808

src/test/regress/expected/func_index.out

-64
This file was deleted.

src/test/regress/parallel_schedule

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ test: select_into select_distinct select_distinct_on select_implicit select_havi
8484
# ----------
8585
# Another group of parallel tests
8686
# ----------
87-
test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator password func_index
87+
test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator password
8888

8989
# ----------
9090
# Another group of parallel tests

src/test/regress/serial_schedule

-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ test: portals
9999
test: arrays
100100
test: btree_index
101101
test: hash_index
102-
test: func_index
103102
test: update
104103
test: delete
105104
test: namespace

src/test/regress/sql/func_index.sql

-31
This file was deleted.

0 commit comments

Comments
 (0)