Skip to content

Commit abaffa9

Browse files
tglsfdctglsfdc
committed
Fix contrib/bloom to work for unlogged indexes.
blbuildempty did not do even approximately the right thing: it tried to add a metapage to the relation's regular data fork, which already has one at that point. It should look like the ambuildempty methods for all the standard index types, ie, initialize a metapage image in some transient storage and then write it directly to the init fork. To support that, refactor BloomInitMetapage into two functions. In passing, fix BloomInitMetapage so it doesn't leave the rd_options field of the index's relcache entry pointing at transient storage. I'm not sure this had any visible consequence, since nothing much else is likely to look at a bloom index's rd_options, but it's certainly poor practice. Per bug #14155 from Zhou Digoal. Report: <20160524144146.22598.42558@wrigleys.postgresql.org>
1 parent 2e8b4bf commit abaffa9

File tree

5 files changed

+144
-25
lines changed

5 files changed

+144
-25
lines changed

‎contrib/bloom/blinsert.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "miscadmin.h"
1919
#include "storage/bufmgr.h"
2020
#include "storage/indexfsm.h"
21+
#include "storage/smgr.h"
2122
#include "utils/memutils.h"
2223
#include "utils/rel.h"
2324

@@ -159,12 +160,26 @@ blbuild(Relation heap, Relation index, IndexInfo *indexInfo)
159160
void
160161
blbuildempty(Relation index)
161162
{
162-
if (RelationGetNumberOfBlocks(index) != 0)
163-
elog(ERROR, "index \"%s\" already contains data",
164-
RelationGetRelationName(index));
163+
Page metapage;
165164

166-
/* Initialize the meta page */
167-
BloomInitMetapage(index);
165+
/* Construct metapage. */
166+
metapage = (Page) palloc(BLCKSZ);
167+
BloomFillMetapage(index, metapage);
168+
169+
/* Write the page. If archiving/streaming, XLOG it. */
170+
PageSetChecksumInplace(metapage, BLOOM_METAPAGE_BLKNO);
171+
smgrwrite(index->rd_smgr, INIT_FORKNUM, BLOOM_METAPAGE_BLKNO,
172+
(char *) metapage, true);
173+
if (XLogIsNeeded())
174+
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
175+
BLOOM_METAPAGE_BLKNO, metapage, false);
176+
177+
/*
178+
* An immediate sync is required even if we xlog'd the page, because the
179+
* write did not go through shared_buffers and therefore a concurrent
180+
* checkpoint may have moved the redo pointer past our xlog record.
181+
*/
182+
smgrimmedsync(index->rd_smgr, INIT_FORKNUM);
168183
}
169184

170185
/*

‎contrib/bloom/bloom.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ typedef BloomScanOpaqueData *BloomScanOpaque;
166166
extern void _PG_init(void);
167167
extern Datum blhandler(PG_FUNCTION_ARGS);
168168
extern void initBloomState(BloomState * state, Relation index);
169+
extern void BloomFillMetapage(Relation index, Page metaPage);
169170
extern void BloomInitMetapage(Relation index);
170171
extern void BloomInitPage(Page page, uint16 flags);
171172
extern Buffer BloomNewBuffer(Relation index);

‎contrib/bloom/blutils.c

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ BloomNewBuffer(Relation index)
346346
}
347347

348348
/*
349-
* Initialize bloom page.
349+
* Initialize any page of a bloom index.
350350
*/
351351
void
352352
BloomInitPage(Page page, uint16 flags)
@@ -363,6 +363,8 @@ BloomInitPage(Page page, uint16 flags)
363363

364364
/*
365365
* Adjust options of bloom index.
366+
*
367+
* This must produce default options when *opts is initially all-zero.
366368
*/
367369
static void
368370
adjustBloomOptions(BloomOptions *opts)
@@ -378,7 +380,7 @@ adjustBloomOptions(BloomOptions *opts)
378380
errmsg("length of bloom signature (%d) is greater than maximum %d",
379381
opts->bloomLength, MAX_BLOOM_LENGTH)));
380382

381-
/* Check singnature length */
383+
/* Check signature length */
382384
for (i = 0; i < INDEX_MAX_KEYS; i++)
383385
{
384386
/*
@@ -392,46 +394,67 @@ adjustBloomOptions(BloomOptions *opts)
392394
}
393395
}
394396

397+
/*
398+
* Fill in metapage for bloom index.
399+
*/
400+
void
401+
BloomFillMetapage(Relation index, Page metaPage)
402+
{
403+
BloomOptions *opts;
404+
BloomMetaPageData *metadata;
405+
406+
/*
407+
* Choose the index's options. If reloptions have been assigned, use
408+
* those, otherwise create default options by applying adjustBloomOptions
409+
* to a zeroed chunk of memory. We apply adjustBloomOptions to existing
410+
* reloptions too, just out of paranoia; they should be valid already.
411+
*/
412+
opts = (BloomOptions *) index->rd_options;
413+
if (!opts)
414+
opts = (BloomOptions *) palloc0(sizeof(BloomOptions));
415+
adjustBloomOptions(opts);
416+
417+
/*
418+
* Initialize contents of meta page, including a copy of the options,
419+
* which are now frozen for the life of the index.
420+
*/
421+
BloomInitPage(metaPage, BLOOM_META);
422+
metadata = BloomPageGetMeta(metaPage);
423+
memset(metadata, 0, sizeof(BloomMetaPageData));
424+
metadata->magickNumber = BLOOM_MAGICK_NUMBER;
425+
metadata->opts = *opts;
426+
((PageHeader) metaPage)->pd_lower += sizeof(BloomMetaPageData);
427+
}
428+
395429
/*
396430
* Initialize metapage for bloom index.
397431
*/
398432
void
399433
BloomInitMetapage(Relation index)
400434
{
401-
Page metaPage;
402435
Buffer metaBuffer;
403-
BloomMetaPageData *metadata;
436+
Page metaPage;
404437
GenericXLogState *state;
405438

406439
/*
407-
* Make a new buffer, since it first buffer it should be associated with
440+
* Make a new page; since it is first page it should be associated with
408441
* block number 0 (BLOOM_METAPAGE_BLKNO).
409442
*/
410443
metaBuffer = BloomNewBuffer(index);
411444
Assert(BufferGetBlockNumber(metaBuffer) == BLOOM_METAPAGE_BLKNO);
412445

413-
/* Initialize bloom index options */
414-
if (!index->rd_options)
415-
index->rd_options = palloc0(sizeof(BloomOptions));
416-
adjustBloomOptions((BloomOptions *) index->rd_options);
417-
418446
/* Initialize contents of meta page */
419447
state = GenericXLogStart(index);
420-
metaPage = GenericXLogRegisterBuffer(state, metaBuffer, GENERIC_XLOG_FULL_IMAGE);
421-
422-
BloomInitPage(metaPage, BLOOM_META);
423-
metadata = BloomPageGetMeta(metaPage);
424-
memset(metadata, 0, sizeof(BloomMetaPageData));
425-
metadata->magickNumber = BLOOM_MAGICK_NUMBER;
426-
metadata->opts = *((BloomOptions *) index->rd_options);
427-
((PageHeader) metaPage)->pd_lower += sizeof(BloomMetaPageData);
428-
448+
metaPage = GenericXLogRegisterBuffer(state, metaBuffer,
449+
GENERIC_XLOG_FULL_IMAGE);
450+
BloomFillMetapage(index, metaPage);
429451
GenericXLogFinish(state);
452+
430453
UnlockReleaseBuffer(metaBuffer);
431454
}
432455

433456
/*
434-
* Initialize options for bloom index.
457+
* Parse reloptions for bloom index, producing a BloomOptions struct.
435458
*/
436459
bytea *
437460
bloptions(Datum reloptions, bool validate)

‎contrib/bloom/expected/bloom.out

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,64 @@ SELECT count(*) FROM tst WHERE i = 7 AND t = '5';
138138
13
139139
(1 row)
140140

141+
-- Try an unlogged table too
142+
CREATE UNLOGGED TABLE tstu (
143+
i int4,
144+
t text
145+
);
146+
INSERT INTO tstu SELECT i%10, substr(md5(i::text), 1, 1) FROM generate_series(1,2000) i;
147+
CREATE INDEX bloomidxu ON tstu USING bloom (i, t) WITH (col2 = 4);
148+
SET enable_seqscan=off;
149+
SET enable_bitmapscan=on;
150+
SET enable_indexscan=on;
151+
EXPLAIN (COSTS OFF) SELECT count(*) FROM tstu WHERE i = 7;
152+
QUERY PLAN
153+
--------------------------------------------
154+
Aggregate
155+
-> Bitmap Heap Scan on tstu
156+
Recheck Cond: (i = 7)
157+
-> Bitmap Index Scan on bloomidxu
158+
Index Cond: (i = 7)
159+
(5 rows)
160+
161+
EXPLAIN (COSTS OFF) SELECT count(*) FROM tstu WHERE t = '5';
162+
QUERY PLAN
163+
--------------------------------------------
164+
Aggregate
165+
-> Bitmap Heap Scan on tstu
166+
Recheck Cond: (t = '5'::text)
167+
-> Bitmap Index Scan on bloomidxu
168+
Index Cond: (t = '5'::text)
169+
(5 rows)
170+
171+
EXPLAIN (COSTS OFF) SELECT count(*) FROM tstu WHERE i = 7 AND t = '5';
172+
QUERY PLAN
173+
---------------------------------------------------------
174+
Aggregate
175+
-> Bitmap Heap Scan on tstu
176+
Recheck Cond: ((i = 7) AND (t = '5'::text))
177+
-> Bitmap Index Scan on bloomidxu
178+
Index Cond: ((i = 7) AND (t = '5'::text))
179+
(5 rows)
180+
181+
SELECT count(*) FROM tstu WHERE i = 7;
182+
count
183+
-------
184+
200
185+
(1 row)
186+
187+
SELECT count(*) FROM tstu WHERE t = '5';
188+
count
189+
-------
190+
112
191+
(1 row)
192+
193+
SELECT count(*) FROM tstu WHERE i = 7 AND t = '5';
194+
count
195+
-------
196+
13
197+
(1 row)
198+
141199
RESET enable_seqscan;
142200
RESET enable_bitmapscan;
143201
RESET enable_indexscan;

‎contrib/bloom/sql/bloom.sql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,28 @@ SELECT count(*) FROM tst WHERE i = 7;
5050
SELECT count(*) FROM tst WHERE t = '5';
5151
SELECT count(*) FROM tst WHERE i = 7 AND t = '5';
5252

53+
-- Try an unlogged table too
54+
55+
CREATE UNLOGGED TABLE tstu (
56+
i int4,
57+
t text
58+
);
59+
60+
INSERT INTO tstu SELECT i%10, substr(md5(i::text), 1, 1) FROM generate_series(1,2000) i;
61+
CREATE INDEX bloomidxu ON tstu USING bloom (i, t) WITH (col2 = 4);
62+
63+
SET enable_seqscan=off;
64+
SET enable_bitmapscan=on;
65+
SET enable_indexscan=on;
66+
67+
EXPLAIN (COSTS OFF) SELECT count(*) FROM tstu WHERE i = 7;
68+
EXPLAIN (COSTS OFF) SELECT count(*) FROM tstu WHERE t = '5';
69+
EXPLAIN (COSTS OFF) SELECT count(*) FROM tstu WHERE i = 7 AND t = '5';
70+
71+
SELECT count(*) FROM tstu WHERE i = 7;
72+
SELECT count(*) FROM tstu WHERE t = '5';
73+
SELECT count(*) FROM tstu WHERE i = 7 AND t = '5';
74+
5375
RESET enable_seqscan;
5476
RESET enable_bitmapscan;
5577
RESET enable_indexscan;

0 commit comments

Comments
 (0)