Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc3c35a7

Browse files
committed
Prevent infinite insertion loops in spgdoinsert().
Formerly we just relied on operator classes that assert longValuesOKto eventually shorten the leaf value enough to fit on an index page.That fails since the introduction of INCLUDE-column support (commit09c1c6a), because the INCLUDE columns might alone take up morethan a page, meaning no amount of leaf-datum compaction will getthe job done. At least with spgtextproc.c, that leads to an infiniteloop, since spgtextproc.c won't throw an error for not being ableto shorten the leaf datum anymore.To fix without breaking cases that would otherwise work, add logicto spgdoinsert() to verify that the leaf tuple size is decreasingafter each "choose" step. Some opclasses might not decrease thesize on every single cycle, and in any case, alignment roundoffof the tuple size could obscure small gains. Therefore, allowup to 10 cycles without additional savings before throwing anerror. (Perhaps this number will need adjustment, but it seemsquite generous right now.)As long as we've developed this logic, let's back-patch it.The back branches don't have INCLUDE columns to worry about, butthis seems like a good defense against possible bugs in operatorclasses. We already know that an infinite loop here is prettyunpleasant, so having a defense seems to outweigh the risk ofbreaking things. (Note that spgtextproc.c is actually the onlyknown opclass with longValuesOK support, so that this is all mootfor known non-core opclasses anyway.)Per report from Dilip Kumar.Discussion:https://postgr.es/m/CAFiTN-uxP_soPhVG840tRMQTBmtA_f_Y8N51G7DKYYqDh7XN-A@mail.gmail.com
1 parenteb7a6b9 commitc3c35a7

File tree

4 files changed

+88
-11
lines changed

4 files changed

+88
-11
lines changed

‎doc/src/sgml/spgist.sgml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,18 @@ LANGUAGE C STRICT;
978978
fails to do that, the <acronym>SP-GiST</acronym> core resorts to
979979
extraordinary measures described in <xref linkend="spgist-all-the-same"/>.
980980
</para>
981+
982+
<para>
983+
When <structfield>longValuesOK</structfield> is true, it is expected
984+
that successive levels of the <acronym>SP-GiST</acronym> tree will
985+
absorb more and more information into the prefixes and node labels of
986+
the inner tuples, making the required leaf datum smaller and smaller,
987+
so that eventually it will fit on a page.
988+
To prevent bugs in operator classes from causing infinite insertion
989+
loops, the <acronym>SP-GiST</acronym> core will raise an error if the
990+
leaf datum does not become any smaller within ten cycles
991+
of <function>choose</function> method calls.
992+
</para>
981993
</sect2>
982994

983995
<sect2 id="spgist-null-labels">

‎src/backend/access/spgist/spgdoinsert.c

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,8 @@ checkAllTheSame(spgPickSplitIn *in, spgPickSplitOut *out, bool tooBig,
669669
* will eventually terminate if lack of balance is the issue. If the tuple
670670
* is too big, we assume that repeated picksplit operations will eventually
671671
* make it small enough by repeated prefix-stripping. A broken opclass could
672-
* make this an infinite loop, though.
672+
* make this an infinite loop, though, so spgdoinsert() checks that the
673+
* leaf datums get smaller each time.
673674
*/
674675
staticbool
675676
doPickSplit(Relationindex,SpGistState*state,
@@ -1918,6 +1919,8 @@ spgdoinsert(Relation index, SpGistState *state,
19181919
intlevel=0;
19191920
DatumleafDatums[INDEX_MAX_KEYS];
19201921
intleafSize;
1922+
intbestLeafSize;
1923+
intnumNoProgressCycles=0;
19211924
SPPageDesccurrent,
19221925
parent;
19231926
FmgrInfo*procinfo=NULL;
@@ -1988,16 +1991,18 @@ spgdoinsert(Relation index, SpGistState *state,
19881991

19891992
/*
19901993
* If it isn't gonna fit, and the opclass can't reduce the datum size by
1991-
* suffixing, bail out now rather thangetting into an endless loop.
1994+
* suffixing, bail out now rather thandoing a lot of useless work.
19921995
*/
1993-
if (leafSize>SPGIST_PAGE_CAPACITY&& !state->config.longValuesOK)
1996+
if (leafSize>SPGIST_PAGE_CAPACITY&&
1997+
(isnull|| !state->config.longValuesOK))
19941998
ereport(ERROR,
19951999
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
19962000
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
19972001
leafSize-sizeof(ItemIdData),
19982002
SPGIST_PAGE_CAPACITY-sizeof(ItemIdData),
19992003
RelationGetRelationName(index)),
20002004
errhint("Values larger than a buffer page cannot be indexed.")));
2005+
bestLeafSize=leafSize;
20012006

20022007
/* Initialize "current" to the appropriate root page */
20032008
current.blkno=isnull ?SPGIST_NULL_BLKNO :SPGIST_ROOT_BLKNO;
@@ -2226,19 +2231,59 @@ spgdoinsert(Relation index, SpGistState *state,
22262231
leafSize+=sizeof(ItemIdData);
22272232
}
22282233

2234+
/*
2235+
* Check new tuple size; fail if it can't fit, unless the
2236+
* opclass says it can handle the situation by suffixing.
2237+
*
2238+
* However, the opclass can only shorten the leaf datum,
2239+
* which may not be enough to ever make the tuple fit,
2240+
* since INCLUDE columns might alone use more than a page.
2241+
* Depending on the opclass' behavior, that could lead to
2242+
* an infinite loop --- spgtextproc.c, for example, will
2243+
* just repeatedly generate an empty-string leaf datum
2244+
* once it runs out of data. Actual bugs in opclasses
2245+
* might cause infinite looping, too. To detect such a
2246+
* loop, check to see if we are making progress by
2247+
* reducing the leafSize in each pass. This is a bit
2248+
* tricky though. Because of alignment considerations,
2249+
* the total tuple size might not decrease on every pass.
2250+
* Also, there are edge cases where the choose method
2251+
* might seem to not make progress for a cycle or two.
2252+
* Somewhat arbitrarily, we allow up to 10 no-progress
2253+
* iterations before failing. (This limit should be more
2254+
* than MAXALIGN, to accommodate opclasses that trim one
2255+
* byte from the leaf datum per pass.)
2256+
*/
2257+
if (leafSize>SPGIST_PAGE_CAPACITY)
2258+
{
2259+
boolok= false;
2260+
2261+
if (state->config.longValuesOK&& !isnull)
2262+
{
2263+
if (leafSize<bestLeafSize)
2264+
{
2265+
ok= true;
2266+
bestLeafSize=leafSize;
2267+
numNoProgressCycles=0;
2268+
}
2269+
elseif (++numNoProgressCycles<10)
2270+
ok= true;
2271+
}
2272+
if (!ok)
2273+
ereport(ERROR,
2274+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2275+
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
2276+
leafSize-sizeof(ItemIdData),
2277+
SPGIST_PAGE_CAPACITY-sizeof(ItemIdData),
2278+
RelationGetRelationName(index)),
2279+
errhint("Values larger than a buffer page cannot be indexed.")));
2280+
}
2281+
22292282
/*
22302283
* Loop around and attempt to insert the new leafDatum at
22312284
* "current" (which might reference an existing child
22322285
* tuple, or might be invalid to force us to find a new
22332286
* page for the tuple).
2234-
*
2235-
* Note: if the opclass sets longValuesOK, we rely on the
2236-
* choose function to eventually shorten the leafDatum
2237-
* enough to fit on a page. We could add a test here to
2238-
* complain if the datum doesn't get visibly shorter each
2239-
* time, but that could get in the way of opclasses that
2240-
* "simplify" datums in a way that doesn't necessarily
2241-
* lead to physical shortening on every cycle.
22422287
*/
22432288
break;
22442289
casespgAddNode:

‎src/test/modules/spgist_name_ops/expected/spgist_name_ops.out

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ select * from t
6161
binary_upgrade_set_next_toast_pg_class_oid | 1 | binary_upgrade_set_next_toast_pg_class_oid
6262
(9 rows)
6363

64+
-- Verify clean failure when INCLUDE'd columns result in overlength tuple
65+
-- The error message details are platform-dependent, so show only SQLSTATE
66+
\set VERBOSITY sqlstate
67+
insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000));
68+
ERROR: 54000
69+
\set VERBOSITY default
6470
drop index t_f1_f2_f3_idx;
6571
create index on t using spgist(f1 name_ops_old) include(f2, f3);
6672
\d+ t_f1_f2_f3_idx
@@ -100,3 +106,7 @@ select * from t
100106
binary_upgrade_set_next_toast_pg_class_oid | 1 | binary_upgrade_set_next_toast_pg_class_oid
101107
(9 rows)
102108

109+
\set VERBOSITY sqlstate
110+
insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000));
111+
ERROR: 54000
112+
\set VERBOSITY default

‎src/test/modules/spgist_name_ops/sql/spgist_name_ops.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ select * from t
2727
where f1>'binary_upgrade_set_n'and f1<'binary_upgrade_set_p'
2828
order by1;
2929

30+
-- Verify clean failure when INCLUDE'd columns result in overlength tuple
31+
-- The error message details are platform-dependent, so show only SQLSTATE
32+
\set VERBOSITY sqlstate
33+
insert into tvalues(repeat('xyzzy',12),42, repeat('xyzzy',4000));
34+
\set VERBOSITY default
35+
3036
dropindex t_f1_f2_f3_idx;
3137

3238
createindexon t using spgist(f1 name_ops_old) include(f2, f3);
@@ -39,3 +45,7 @@ select * from t
3945
select*from t
4046
where f1>'binary_upgrade_set_n'and f1<'binary_upgrade_set_p'
4147
order by1;
48+
49+
\set VERBOSITY sqlstate
50+
insert into tvalues(repeat('xyzzy',12),42, repeat('xyzzy',4000));
51+
\set VERBOSITY default

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp