diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 360b26e6fc470de15bba62ea942afe7b9de03c8f..fc9f964bf78887ea08439e60acb548d805dd1848 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -696,7 +696,7 @@ brinbuildempty(PG_FUNCTION_ARGS)
*
* XXX we could mark item tuples as "dirty" (when a minimum or maximum heap
* tuple is deleted), meaning the need to re-run summarization on the affected
- * range. Need to an extra flag in brintuples for that.
+ * range. Would need to add an extra flag in brintuples for that.
*/
Datum
brinbulkdelete(PG_FUNCTION_ARGS)
@@ -951,9 +951,13 @@ summarize_range(IndexInfo *indexInfo, BrinBuildState *state, Relation heapRel,
* Execute the partial heap scan covering the heap blocks in the specified
* page range, summarizing the heap tuples in it. This scan stops just
* short of brinbuildCallback creating the new index entry.
+ *
+ * Note that it is critical we use the "any visible" mode of
+ * IndexBuildHeapRangeScan here: otherwise, we would miss tuples inserted
+ * by transactions that are still in progress, among other corner cases.
*/
state->bs_currRangeStart = heapBlk;
- IndexBuildHeapRangeScan(heapRel, state->bs_irel, indexInfo, false,
+ IndexBuildHeapRangeScan(heapRel, state->bs_irel, indexInfo, false, true,
heapBlk, state->bs_pagesPerRange,
brinbuildCallback, (void *) state);
@@ -1058,36 +1062,6 @@ brinsummarize(Relation index, Relation heapRel, double *numSummarized,
state = initialize_brin_buildstate(index, revmap,
pagesPerRange);
indexInfo = BuildIndexInfo(index);
-
- /*
- * We only have ShareUpdateExclusiveLock on the table, and
- * therefore other sessions may insert tuples into the range
- * we're going to scan. This is okay, because we take
- * additional precautions to avoid losing the additional
- * tuples; see comments in summarize_range. Set the
- * concurrent flag, which causes IndexBuildHeapRangeScan to
- * use a snapshot other than SnapshotAny, and silences
- * warnings emitted there.
- */
- indexInfo->ii_Concurrent = true;
-
- /*
- * If using transaction-snapshot mode, it would be possible
- * for another transaction to insert a tuple that's not
- * visible to our snapshot if we have already acquired one,
- * when in snapshot-isolation mode; therefore, disallow this
- * from running in such a transaction unless a snapshot hasn't
- * been acquired yet.
- *
- * This code is called by VACUUM and
- * brin_summarize_new_values. Have the error message mention
- * the latter because VACUUM cannot run in a transaction and
- * thus cannot cause this issue.
- */
- if (IsolationUsesXactSnapshot() && FirstSnapshotSet)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
- errmsg("brin_summarize_new_values() cannot run in a transaction that has already obtained a snapshot")));
}
summarize_range(indexInfo, state, heapRel, heapBlk);
@@ -1111,7 +1085,10 @@ brinsummarize(Relation index, Relation heapRel, double *numSummarized,
/* free resources */
brinRevmapTerminate(revmap);
if (state)
+ {
terminate_brin_buildstate(state);
+ pfree(indexInfo);
+ }
}
/*
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 69f35c933097920c3a0da919e8a9ef7ff6563fec..e59b163173c611f0605ed8e255debcfec0225f5b 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2161,6 +2161,7 @@ IndexBuildHeapScan(Relation heapRelation,
{
return IndexBuildHeapRangeScan(heapRelation, indexRelation,
indexInfo, allow_sync,
+ false,
0, InvalidBlockNumber,
callback, callback_state);
}
@@ -2170,12 +2171,17 @@ IndexBuildHeapScan(Relation heapRelation,
* number of blocks are scanned. Scan to end-of-rel can be signalled by
* passing InvalidBlockNumber as numblocks. Note that restricting the range
* to scan cannot be done when requesting syncscan.
+ *
+ * When "anyvisible" mode is requested, all tuples visible to any transaction
+ * are considered, including those inserted or deleted by transactions that are
+ * still in progress.
*/
double
IndexBuildHeapRangeScan(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool allow_sync,
+ bool anyvisible,
BlockNumber start_blockno,
BlockNumber numblocks,
IndexBuildCallback callback,
@@ -2209,6 +2215,12 @@ IndexBuildHeapRangeScan(Relation heapRelation,
checking_uniqueness = (indexInfo->ii_Unique ||
indexInfo->ii_ExclusionOps != NULL);
+ /*
+ * "Any visible" mode is not compatible with uniqueness checks; make sure
+ * only one of those is requested.
+ */
+ Assert(!(anyvisible && checking_uniqueness));
+
/*
* Need an EState for evaluation of index expressions and partial-index
* predicates. Also a slot to hold the current tuple.
@@ -2236,6 +2248,9 @@ IndexBuildHeapRangeScan(Relation heapRelation,
{
snapshot = RegisterSnapshot(GetTransactionSnapshot());
OldestXmin = InvalidTransactionId; /* not used */
+
+ /* "any visible" mode is not compatible with this */
+ Assert(!anyvisible);
}
else
{
@@ -2363,6 +2378,17 @@ IndexBuildHeapRangeScan(Relation heapRelation,
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
+ /*
+ * In "anyvisible" mode, this tuple is visible and we don't
+ * need any further checks.
+ */
+ if (anyvisible)
+ {
+ indexIt = true;
+ tupleIsAlive = true;
+ break;
+ }
+
/*
* Since caller should hold ShareLock or better, normally
* the only way to see this is if it was inserted earlier
@@ -2409,8 +2435,16 @@ IndexBuildHeapRangeScan(Relation heapRelation,
/*
* As with INSERT_IN_PROGRESS case, this is unexpected
- * unless it's our own deletion or a system catalog.
+ * unless it's our own deletion or a system catalog;
+ * but in anyvisible mode, this tuple is visible.
*/
+ if (anyvisible)
+ {
+ indexIt = true;
+ tupleIsAlive = false;
+ break;
+ }
+
xwait = HeapTupleHeaderGetUpdateXid(heapTuple->t_data);
if (!TransactionIdIsCurrentTransactionId(xwait))
{
diff --git a/src/include/access/brin.h b/src/include/access/brin.h
index a04a1320b10f48af5975e9488a1c0a8a8415bf57..e72fb2dcfca2a499a493e34c16d1246afada48f1 100644
--- a/src/include/access/brin.h
+++ b/src/include/access/brin.h
@@ -22,7 +22,6 @@ extern Datum brinbuild(PG_FUNCTION_ARGS);
extern Datum brinbuildempty(PG_FUNCTION_ARGS);
extern Datum brininsert(PG_FUNCTION_ARGS);
extern Datum brinbeginscan(PG_FUNCTION_ARGS);
-extern Datum bringettuple(PG_FUNCTION_ARGS);
extern Datum bringetbitmap(PG_FUNCTION_ARGS);
extern Datum brinrescan(PG_FUNCTION_ARGS);
extern Datum brinendscan(PG_FUNCTION_ARGS);
@@ -30,7 +29,6 @@ extern Datum brinmarkpos(PG_FUNCTION_ARGS);
extern Datum brinrestrpos(PG_FUNCTION_ARGS);
extern Datum brinbulkdelete(PG_FUNCTION_ARGS);
extern Datum brinvacuumcleanup(PG_FUNCTION_ARGS);
-extern Datum brincanreturn(PG_FUNCTION_ARGS);
extern Datum brincostestimate(PG_FUNCTION_ARGS);
extern Datum brinoptions(PG_FUNCTION_ARGS);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 8b3b28d954c6d2de9c080a00f1f585f5f0159279..a29174b69057be201c0639c3be7b4c33bb19f343 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -105,6 +105,7 @@ extern double IndexBuildHeapRangeScan(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool allow_sync,
+ bool anyvisible,
BlockNumber start_blockno,
BlockNumber end_blockno,
IndexBuildCallback callback,
diff --git a/src/test/isolation/expected/brin-1.out b/src/test/isolation/expected/brin-1.out
new file mode 100644
index 0000000000000000000000000000000000000000..ddb90f4dba1389d58e53154b198aa12c023b8700
--- /dev/null
+++ b/src/test/isolation/expected/brin-1.out
@@ -0,0 +1,39 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s2check s1b s2b s1i s2summ s1c s2c s2check
+step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
+itemoffset blknum attnum allnulls hasnulls placeholder value
+
+1 0 1 f f f {1 .. 1}
+step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step s2b: BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1;
+?column?
+
+1
+step s1i: INSERT INTO brin_iso VALUES (1000);
+step s2summ: SELECT brin_summarize_new_values('brinidx'::regclass);
+brin_summarize_new_values
+
+1
+step s1c: COMMIT;
+step s2c: COMMIT;
+step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
+itemoffset blknum attnum allnulls hasnulls placeholder value
+
+1 0 1 f f f {1 .. 1}
+2 1 1 f f f {1 .. 1000}
+
+starting permutation: s2check s1b s1i s2vacuum s1c s2check
+step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
+itemoffset blknum attnum allnulls hasnulls placeholder value
+
+1 0 1 f f f {1 .. 1}
+step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step s1i: INSERT INTO brin_iso VALUES (1000);
+step s2vacuum: VACUUM brin_iso;
+step s1c: COMMIT;
+step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
+itemoffset blknum attnum allnulls hasnulls placeholder value
+
+1 0 1 f f f {1 .. 1}
+2 1 1 f f f {1 .. 1000}
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index c0ed637cd2485b7d59aba2608e5b832e1072d1c5..5852c6e51c2ed72fdc270ceddabcc95a63ed9804 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -36,6 +36,7 @@ test: skip-locked
test: skip-locked-2
test: skip-locked-3
test: skip-locked-4
+test: brin-1
test: drop-index-concurrently-1
test: alter-table-1
test: alter-table-2
diff --git a/src/test/isolation/specs/brin-1.spec b/src/test/isolation/specs/brin-1.spec
new file mode 100644
index 0000000000000000000000000000000000000000..19ac18a2e88dc440e63caba08e4ecf2039cc80e6
--- /dev/null
+++ b/src/test/isolation/specs/brin-1.spec
@@ -0,0 +1,44 @@
+# This test verifies that values inserted in transactions still in progress
+# are considered during concurrent range summarization (either using the
+# brin_summarize_new_values function or regular VACUUM).
+
+setup
+{
+ CREATE TABLE brin_iso (
+ value int
+ ) WITH (fillfactor=10);
+ CREATE INDEX brinidx ON brin_iso USING brin (value) WITH (pages_per_range=1);
+ -- this fills the first page
+ DO $$
+ DECLARE curtid tid;
+ BEGIN
+ LOOP
+ INSERT INTO brin_iso VALUES (1) RETURNING ctid INTO curtid;
+ EXIT WHEN curtid > tid '(1, 0)';
+ END LOOP;
+ END;
+ $$;
+ CREATE EXTENSION IF NOT EXISTS pageinspect;
+}
+
+teardown
+{
+ DROP TABLE brin_iso;
+}
+
+session "s1"
+step "s1b" { BEGIN ISOLATION LEVEL REPEATABLE READ; }
+step "s1i" { INSERT INTO brin_iso VALUES (1000); }
+step "s1c" { COMMIT; }
+
+session "s2"
+step "s2b" { BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; }
+step "s2summ" { SELECT brin_summarize_new_values('brinidx'::regclass); }
+step "s2c" { COMMIT; }
+
+step "s2vacuum" { VACUUM brin_iso; }
+
+step "s2check" { SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass); }
+
+permutation "s2check" "s1b" "s2b" "s1i" "s2summ" "s1c" "s2c" "s2check"
+permutation "s2check" "s1b" "s1i" "s2vacuum" "s1c" "s2check"