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.

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index b7bcdd9d0f7b2f8af6671d4ce3a24e86cf0dbf46..8404904710929e785ce7ff9ffbda26350088d569 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3148,8 +3148,15 @@ RelationTruncateIndexes(Relation heapRelation)
                /* Open the index relation; use exclusive lock, just to be sure */
                currentIndex = index_open(indexId, AccessExclusiveLock);
 
-               /* Fetch info needed for index_build */
-               indexInfo = BuildIndexInfo(currentIndex);
+               /*
+                * Fetch info needed for index_build.  Since we know there are no
+                * tuples that actually need indexing, we can use a dummy IndexInfo.
+                * This is slightly cheaper to build, but the real point is to avoid
+                * possibly running user-defined code in index expressions or
+                * predicates.  We might be getting invoked during ON COMMIT
+                * processing, and we don't want to run any such code then.
+                */
+               indexInfo = BuildDummyIndexInfo(currentIndex);
 
                /*
                 * Now truncate the actual file (and discard buffers).

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 67f637de11d99d368ec872c0cff2203c1d5b3fcf..e9955707fa7a6624cf7cdfed2bdf29623d6b1930 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2324,6 +2324,56 @@ BuildIndexInfo(Relation index)
        return ii;
 }
 
+/* ----------------
+ *             BuildDummyIndexInfo
+ *                     Construct a dummy IndexInfo record for an open index
+ *
+ * This differs from the real BuildIndexInfo in that it will never run any
+ * user-defined code that might exist in index expressions or predicates.
+ * Instead of the real index expressions, we return null constants that have
+ * the right types/typmods/collations.  Predicates and exclusion clauses are
+ * just ignored.  This is sufficient for the purpose of truncating an index,
+ * since we will not need to actually evaluate the expressions or predicates;
+ * the only thing that's likely to be done with the data is construction of
+ * a tupdesc describing the index's rowtype.
+ * ----------------
+ */
+IndexInfo *
+BuildDummyIndexInfo(Relation index)
+{
+       IndexInfo  *ii;
+       Form_pg_index indexStruct = index->rd_index;
+       int                     i;
+       int                     numAtts;
+
+       /* check the number of keys, and copy attr numbers into the IndexInfo */
+       numAtts = indexStruct->indnatts;
+       if (numAtts < 1 || numAtts > INDEX_MAX_KEYS)
+               elog(ERROR, "invalid indnatts %d for index %u",
+                        numAtts, RelationGetRelid(index));
+
+       /*
+        * Create the node, using dummy index expressions, and pretending there is
+        * no predicate.
+        */
+       ii = makeIndexInfo(indexStruct->indnatts,
+                                          indexStruct->indnkeyatts,
+                                          index->rd_rel->relam,
+                                          RelationGetDummyIndexExpressions(index),
+                                          NIL,
+                                          indexStruct->indisunique,
+                                          indexStruct->indisready,
+                                          false);
+
+       /* fill in attribute numbers */
+       for (i = 0; i < numAtts; i++)
+               ii->ii_IndexAttrNumbers[i] = indexStruct->indkey.values[i];
+
+       /* We ignore the exclusion constraint if any */
+
+       return ii;
+}
+
 /*
  * CompareIndexInfo
  *             Return whether the properties of two indexes (in different tables)

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index ad1ff01b3208ed9932fc69bf3d73fed76094c824..50f8912c13ae5373d69d51e611caafd6bee19906 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4629,6 +4629,57 @@ RelationGetIndexExpressions(Relation relation)
        return result;
 }
 
+/*
+ * RelationGetDummyIndexExpressions -- get dummy expressions for an index
+ *
+ * Return a list of dummy expressions (just Const nodes) with the same
+ * types/typmods/collations as the index's real expressions.  This is
+ * useful in situations where we don't want to run any user-defined code.
+ */
+List *
+RelationGetDummyIndexExpressions(Relation relation)
+{
+       List       *result;
+       Datum           exprsDatum;
+       bool            isnull;
+       char       *exprsString;
+       List       *rawExprs;
+       ListCell   *lc;
+
+       /* Quick exit if there is nothing to do. */
+       if (relation->rd_indextuple == NULL ||
+               heap_attisnull(relation->rd_indextuple, Anum_pg_index_indexprs, NULL))
+               return NIL;
+
+       /* Extract raw node tree(s) from index tuple. */
+       exprsDatum = heap_getattr(relation->rd_indextuple,
+                                                         Anum_pg_index_indexprs,
+                                                         GetPgIndexDescriptor(),
+                                                         &isnull);
+       Assert(!isnull);
+       exprsString = TextDatumGetCString(exprsDatum);
+       rawExprs = (List *) stringToNode(exprsString);
+       pfree(exprsString);
+
+       /* Construct null Consts; the typlen and typbyval are arbitrary. */
+       result = NIL;
+       foreach(lc, rawExprs)
+       {
+               Node       *rawExpr = (Node *) lfirst(lc);
+
+               result = lappend(result,
+                                                makeConst(exprType(rawExpr),
+                                                                  exprTypmod(rawExpr),
+                                                                  exprCollation(rawExpr),
+                                                                  1,
+                                                                  (Datum) 0,
+                                                                  true,
+                                                                  true));
+       }
+
+       return result;
+}
+
 /*
  * RelationGetIndexPredicate -- get the index predicate for an index
  *

diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 1113d25b2d8246ced702d6dadc3cff1d099ab101..27d9e537d31c68201d0a8282116f93cd0debd66d 100644 (file)
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -106,6 +106,8 @@ extern void index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode);
 
 extern IndexInfo *BuildIndexInfo(Relation index);
 
+extern IndexInfo *BuildDummyIndexInfo(Relation index);
+
 extern bool CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
                                                         Oid *collations1, Oid *collations2,
                                                         Oid *opfamilies1, Oid *opfamilies2,

diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 2f2ace35b051649cfd36e0bbf8c5cc2e161405e5..90487b2b2eb372aa8fdbf14dbb0620acef0dbc43 100644 (file)
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -48,6 +48,7 @@ extern List *RelationGetStatExtList(Relation relation);
 extern Oid     RelationGetPrimaryKeyIndex(Relation relation);
 extern Oid     RelationGetReplicaIndex(Relation relation);
 extern List *RelationGetIndexExpressions(Relation relation);
+extern List *RelationGetDummyIndexExpressions(Relation relation);
 extern List *RelationGetIndexPredicate(Relation relation);
 
 typedef enum IndexAttrBitmapKind

diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index b1d2ffdef3d8be794494d338c629140062b583ef..a5b3ed34a367aafe8f430401eccb4c0cbb2e0384 100644 (file)
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -49,6 +49,8 @@ LINE 1: SELECT * FROM temptest;
                       ^
 -- Test ON COMMIT DELETE ROWS
 CREATE TEMP TABLE temptest(col int) ON COMMIT DELETE ROWS;
+-- while we're here, verify successful truncation of index with SQL function
+CREATE INDEX ON temptest(bit_length(''));
 BEGIN;
 INSERT INTO temptest VALUES (1);
 INSERT INTO temptest VALUES (2);

diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql
index b636b33dcacafbd8f8b6d598a1972eab7de1765a..424d12b2833f05ea9c671f45a6beae8b56fd7a81 100644 (file)
--- a/src/test/regress/sql/temp.sql
+++ b/src/test/regress/sql/temp.sql
@@ -55,6 +55,9 @@ SELECT * FROM temptest;
 
 CREATE TEMP TABLE temptest(col int) ON COMMIT DELETE ROWS;
 
+-- while we're here, verify successful truncation of index with SQL function
+CREATE INDEX ON temptest(bit_length(''));
+
 BEGIN;
 INSERT INTO temptest VALUES (1);
 INSERT INTO temptest VALUES (2);