Skip to content

Commit 0c84e99

Browse files
tglsfdctglsfdc
committed
Fix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables.
We implement ON COMMIT DELETE ROWS by truncating tables marked that way, which requires also truncating/rebuilding their indexes. But RelationTruncateIndexes asks the relcache for up-to-date copies of any index expressions, which may cause execution of eval_const_expressions on them, which can result in actual execution of subexpressions. This is a bad thing to have happening during ON COMMIT. Manuel Rigger reported that use of a SQL function resulted in crashes due to expectations that ActiveSnapshot would be set, which it isn't. The most obvious fix perhaps would be to push a snapshot during PreCommit_on_commit_actions, but I think that would just open the door to more problems: CommitTransaction explicitly expects that no user-defined code can be running at this point. Fortunately, since we know that no tuples exist to be indexed, there seems no need to use the real index expressions or predicates during RelationTruncateIndexes. We can set up dummy index expressions instead (we do need something that will expose the right data type, as there are places that build index tupdescs based on this), and just ignore predicates and exclusion constraints. In a green field it'd likely be better to reimplement ON COMMIT DELETE ROWS using the same "init fork" infrastructure used for unlogged relations. That seems impractical without catalog changes though, and even without that it'd be too big a change to back-patch. So for now do it like this. Per private report from Manuel Rigger. This has been broken forever, so back-patch to all supported branches.
1 parent d9b974e commit 0c84e99

File tree

7 files changed

+125
-2
lines changed

7 files changed

+125
-2
lines changed

‎src/backend/catalog/heap.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2709,8 +2709,15 @@ RelationTruncateIndexes(Relation heapRelation)
27092709
/* Open the index relation; use exclusive lock, just to be sure */
27102710
currentIndex = index_open(indexId, AccessExclusiveLock);
27112711

2712-
/* Fetch info needed for index_build */
2713-
indexInfo = BuildIndexInfo(currentIndex);
2712+
/*
2713+
* Fetch info needed for index_build. Since we know there are no
2714+
* tuples that actually need indexing, we can use a dummy IndexInfo.
2715+
* This is slightly cheaper to build, but the real point is to avoid
2716+
* possibly running user-defined code in index expressions or
2717+
* predicates. We might be getting invoked during ON COMMIT
2718+
* processing, and we don't want to run any such code then.
2719+
*/
2720+
indexInfo = BuildDummyIndexInfo(currentIndex);
27142721

27152722
/*
27162723
* Now truncate the actual file (and discard buffers).

‎src/backend/catalog/index.c

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,6 +1660,61 @@ BuildIndexInfo(Relation index)
16601660
return ii;
16611661
}
16621662

1663+
/* ----------------
1664+
* BuildDummyIndexInfo
1665+
* Construct a dummy IndexInfo record for an open index
1666+
*
1667+
* This differs from the real BuildIndexInfo in that it will never run any
1668+
* user-defined code that might exist in index expressions or predicates.
1669+
* Instead of the real index expressions, we return null constants that have
1670+
* the right types/typmods/collations. Predicates and exclusion clauses are
1671+
* just ignored. This is sufficient for the purpose of truncating an index,
1672+
* since we will not need to actually evaluate the expressions or predicates;
1673+
* the only thing that's likely to be done with the data is construction of
1674+
* a tupdesc describing the index's rowtype.
1675+
* ----------------
1676+
*/
1677+
IndexInfo *
1678+
BuildDummyIndexInfo(Relation index)
1679+
{
1680+
IndexInfo *ii = makeNode(IndexInfo);
1681+
Form_pg_index indexStruct = index->rd_index;
1682+
int i;
1683+
int numKeys;
1684+
1685+
/* check the number of keys, and copy attr numbers into the IndexInfo */
1686+
numKeys = indexStruct->indnatts;
1687+
if (numKeys < 1 || numKeys > INDEX_MAX_KEYS)
1688+
elog(ERROR, "invalid indnatts %d for index %u",
1689+
numKeys, RelationGetRelid(index));
1690+
ii->ii_NumIndexAttrs = numKeys;
1691+
for (i = 0; i < numKeys; i++)
1692+
ii->ii_KeyAttrNumbers[i] = indexStruct->indkey.values[i];
1693+
1694+
/* fetch dummy expressions for expressional indexes */
1695+
ii->ii_Expressions = RelationGetDummyIndexExpressions(index);
1696+
ii->ii_ExpressionsState = NIL;
1697+
1698+
/* pretend there is no predicate */
1699+
ii->ii_Predicate = NIL;
1700+
ii->ii_PredicateState = NULL;
1701+
1702+
/* We ignore the exclusion constraint if any */
1703+
ii->ii_ExclusionOps = NULL;
1704+
ii->ii_ExclusionProcs = NULL;
1705+
ii->ii_ExclusionStrats = NULL;
1706+
1707+
/* other info */
1708+
ii->ii_Unique = indexStruct->indisunique;
1709+
ii->ii_ReadyForInserts = IndexIsReady(indexStruct);
1710+
1711+
/* initialize index-build state to default */
1712+
ii->ii_Concurrent = false;
1713+
ii->ii_BrokenHotChain = false;
1714+
1715+
return ii;
1716+
}
1717+
16631718
/* ----------------
16641719
* FormIndexDatum
16651720
* Construct values[] and isnull[] arrays for a new index tuple.

‎src/backend/utils/cache/relcache.c

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
#include "catalog/storage.h"
5757
#include "commands/trigger.h"
5858
#include "miscadmin.h"
59+
#include "nodes/makefuncs.h"
60+
#include "nodes/nodeFuncs.h"
5961
#include "optimizer/clauses.h"
6062
#include "optimizer/planmain.h"
6163
#include "optimizer/prep.h"
@@ -4068,6 +4070,57 @@ RelationGetIndexExpressions(Relation relation)
40684070
return result;
40694071
}
40704072

4073+
/*
4074+
* RelationGetDummyIndexExpressions -- get dummy expressions for an index
4075+
*
4076+
* Return a list of dummy expressions (just Const nodes) with the same
4077+
* types/typmods/collations as the index's real expressions. This is
4078+
* useful in situations where we don't want to run any user-defined code.
4079+
*/
4080+
List *
4081+
RelationGetDummyIndexExpressions(Relation relation)
4082+
{
4083+
List *result;
4084+
Datum exprsDatum;
4085+
bool isnull;
4086+
char *exprsString;
4087+
List *rawExprs;
4088+
ListCell *lc;
4089+
4090+
/* Quick exit if there is nothing to do. */
4091+
if (relation->rd_indextuple == NULL ||
4092+
heap_attisnull(relation->rd_indextuple, Anum_pg_index_indexprs))
4093+
return NIL;
4094+
4095+
/* Extract raw node tree(s) from index tuple. */
4096+
exprsDatum = heap_getattr(relation->rd_indextuple,
4097+
Anum_pg_index_indexprs,
4098+
GetPgIndexDescriptor(),
4099+
&isnull);
4100+
Assert(!isnull);
4101+
exprsString = TextDatumGetCString(exprsDatum);
4102+
rawExprs = (List *) stringToNode(exprsString);
4103+
pfree(exprsString);
4104+
4105+
/* Construct null Consts; the typlen and typbyval are arbitrary. */
4106+
result = NIL;
4107+
foreach(lc, rawExprs)
4108+
{
4109+
Node *rawExpr = (Node *) lfirst(lc);
4110+
4111+
result = lappend(result,
4112+
makeConst(exprType(rawExpr),
4113+
exprTypmod(rawExpr),
4114+
exprCollation(rawExpr),
4115+
1,
4116+
(Datum) 0,
4117+
true,
4118+
true));
4119+
}
4120+
4121+
return result;
4122+
}
4123+
40714124
/*
40724125
* RelationGetIndexPredicate -- get the index predicate for an index
40734126
*

‎src/include/catalog/index.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ extern void index_drop(Oid indexId, bool concurrent);
7979

8080
extern IndexInfo *BuildIndexInfo(Relation index);
8181

82+
extern IndexInfo *BuildDummyIndexInfo(Relation index);
83+
8284
extern void FormIndexDatum(IndexInfo *indexInfo,
8385
TupleTableSlot *slot,
8486
EState *estate,

‎src/include/utils/relcache.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ extern List *RelationGetIndexList(Relation relation);
4141
extern Oid RelationGetOidIndex(Relation relation);
4242
extern Oid RelationGetReplicaIndex(Relation relation);
4343
extern List *RelationGetIndexExpressions(Relation relation);
44+
extern List *RelationGetDummyIndexExpressions(Relation relation);
4445
extern List *RelationGetIndexPredicate(Relation relation);
4546

4647
typedef enum IndexAttrBitmapKind

‎src/test/regress/expected/temp.out

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ LINE 1: SELECT * FROM temptest;
4949
^
5050
-- Test ON COMMIT DELETE ROWS
5151
CREATE TEMP TABLE temptest(col int) ON COMMIT DELETE ROWS;
52+
-- while we're here, verify successful truncation of index with SQL function
53+
CREATE INDEX ON temptest(bit_length(''));
5254
BEGIN;
5355
INSERT INTO temptest VALUES (1);
5456
INSERT INTO temptest VALUES (2);

‎src/test/regress/sql/temp.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ SELECT * FROM temptest;
5555

5656
CREATE TEMP TABLE temptest(col int) ON COMMIT DELETE ROWS;
5757

58+
-- while we're here, verify successful truncation of index with SQL function
59+
CREATE INDEX ON temptest(bit_length(''));
60+
5861
BEGIN;
5962
INSERT INTO temptest VALUES (1);
6063
INSERT INTO temptest VALUES (2);

0 commit comments

Comments
 (0)