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

Commitdc714c1

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 parentc1b72bf commitdc714c1

File tree

2 files changed

+62
-11
lines changed

2 files changed

+62
-11
lines changed

‎doc/src/sgml/spgist.sgml

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

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

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

Lines changed: 50 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,
@@ -1895,6 +1896,8 @@ spgdoinsert(Relation index, SpGistState *state,
18951896
intlevel=0;
18961897
DatumleafDatum;
18971898
intleafSize;
1899+
intbestLeafSize;
1900+
intnumNoProgressCycles=0;
18981901
SPPageDesccurrent,
18991902
parent;
19001903
FmgrInfo*procinfo=NULL;
@@ -1944,22 +1947,24 @@ spgdoinsert(Relation index, SpGistState *state,
19441947
* Compute space needed for a leaf tuple containing the given datum.
19451948
*
19461949
* If it isn't gonna fit, and the opclass can't reduce the datum size by
1947-
* suffixing, bail out now rather thangetting into an endless loop.
1950+
* suffixing, bail out now rather thandoing a lot of useless work.
19481951
*/
19491952
if (!isnull)
19501953
leafSize=SGLTHDRSZ+sizeof(ItemIdData)+
19511954
SpGistGetTypeSize(&state->attLeafType,leafDatum);
19521955
else
19531956
leafSize=SGDTSIZE+sizeof(ItemIdData);
19541957

1955-
if (leafSize>SPGIST_PAGE_CAPACITY&& !state->config.longValuesOK)
1958+
if (leafSize>SPGIST_PAGE_CAPACITY&&
1959+
(isnull|| !state->config.longValuesOK))
19561960
ereport(ERROR,
19571961
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
19581962
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
19591963
leafSize-sizeof(ItemIdData),
19601964
SPGIST_PAGE_CAPACITY-sizeof(ItemIdData),
19611965
RelationGetRelationName(index)),
19621966
errhint("Values larger than a buffer page cannot be indexed.")));
1967+
bestLeafSize=leafSize;
19631968

19641969
/* Initialize "current" to the appropriate root page */
19651970
current.blkno=isnull ?SPGIST_NULL_BLKNO :SPGIST_ROOT_BLKNO;
@@ -2187,19 +2192,53 @@ spgdoinsert(Relation index, SpGistState *state,
21872192
SpGistGetTypeSize(&state->attLeafType,leafDatum);
21882193
}
21892194

2195+
/*
2196+
* Check new tuple size; fail if it can't fit, unless the
2197+
* opclass says it can handle the situation by suffixing.
2198+
*
2199+
* A buggy opclass might not ever make the leaf datum
2200+
* small enough, causing an infinite loop. To detect such
2201+
* a loop, check to see if we are making progress by
2202+
* reducing the leafSize in each pass. This is a bit
2203+
* tricky though. Because of alignment considerations,
2204+
* the total tuple size might not decrease on every pass.
2205+
* Also, there are edge cases where the choose method
2206+
* might seem to not make progress for a cycle or two.
2207+
* Somewhat arbitrarily, we allow up to 10 no-progress
2208+
* iterations before failing. (This limit should be more
2209+
* than MAXALIGN, to accommodate opclasses that trim one
2210+
* byte from the leaf datum per pass.)
2211+
*/
2212+
if (leafSize>SPGIST_PAGE_CAPACITY)
2213+
{
2214+
boolok= false;
2215+
2216+
if (state->config.longValuesOK&& !isnull)
2217+
{
2218+
if (leafSize<bestLeafSize)
2219+
{
2220+
ok= true;
2221+
bestLeafSize=leafSize;
2222+
numNoProgressCycles=0;
2223+
}
2224+
elseif (++numNoProgressCycles<10)
2225+
ok= true;
2226+
}
2227+
if (!ok)
2228+
ereport(ERROR,
2229+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2230+
errmsg("index row size %zu exceeds maximum %zu for index \"%s\"",
2231+
leafSize-sizeof(ItemIdData),
2232+
SPGIST_PAGE_CAPACITY-sizeof(ItemIdData),
2233+
RelationGetRelationName(index)),
2234+
errhint("Values larger than a buffer page cannot be indexed.")));
2235+
}
2236+
21902237
/*
21912238
* Loop around and attempt to insert the new leafDatum at
21922239
* "current" (which might reference an existing child
21932240
* tuple, or might be invalid to force us to find a new
21942241
* page for the tuple).
2195-
*
2196-
* Note: if the opclass sets longValuesOK, we rely on the
2197-
* choose function to eventually shorten the leafDatum
2198-
* enough to fit on a page. We could add a test here to
2199-
* complain if the datum doesn't get visibly shorter each
2200-
* time, but that could get in the way of opclasses that
2201-
* "simplify" datums in a way that doesn't necessarily
2202-
* lead to physical shortening on every cycle.
22032242
*/
22042243
break;
22052244
casespgAddNode:

0 commit comments

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