diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/access/spgist/spgdoinsert.c | 67 | ||||
| -rw-r--r-- | src/test/modules/spgist_name_ops/expected/spgist_name_ops.out | 10 | ||||
| -rw-r--r-- | src/test/modules/spgist_name_ops/sql/spgist_name_ops.sql | 10 |
3 files changed, 76 insertions, 11 deletions
diff --git a/src/backend/access/spgist/spgdoinsert.c b/src/backend/access/spgist/spgdoinsert.c index 88ae2499dd..70557bcf3d 100644 --- a/src/backend/access/spgist/spgdoinsert.c +++ b/src/backend/access/spgist/spgdoinsert.c @@ -669,7 +669,8 @@ checkAllTheSame(spgPickSplitIn *in, spgPickSplitOut *out, bool tooBig, * will eventually terminate if lack of balance is the issue. If the tuple * is too big, we assume that repeated picksplit operations will eventually * make it small enough by repeated prefix-stripping. A broken opclass could - * make this an infinite loop, though. + * make this an infinite loop, though, so spgdoinsert() checks that the + * leaf datums get smaller each time. */ static bool doPickSplit(Relation index, SpGistState *state, @@ -1918,6 +1919,8 @@ spgdoinsert(Relation index, SpGistState *state, int level = 0; Datum leafDatums[INDEX_MAX_KEYS]; int leafSize; + int bestLeafSize; + int numNoProgressCycles = 0; SPPageDesc current, parent; FmgrInfo *procinfo = NULL; @@ -1988,9 +1991,10 @@ spgdoinsert(Relation index, SpGistState *state, /* * If it isn't gonna fit, and the opclass can't reduce the datum size by - * suffixing, bail out now rather than getting into an endless loop. + * suffixing, bail out now rather than doing a lot of useless work. */ - if (leafSize > SPGIST_PAGE_CAPACITY && !state->config.longValuesOK) + if (leafSize > SPGIST_PAGE_CAPACITY && + (isnull || !state->config.longValuesOK)) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("index row size %zu exceeds maximum %zu for index \"%s\"", @@ -1998,6 +2002,7 @@ spgdoinsert(Relation index, SpGistState *state, SPGIST_PAGE_CAPACITY - sizeof(ItemIdData), RelationGetRelationName(index)), errhint("Values larger than a buffer page cannot be indexed."))); + bestLeafSize = leafSize; /* Initialize "current" to the appropriate root page */ current.blkno = isnull ? SPGIST_NULL_BLKNO : SPGIST_ROOT_BLKNO; @@ -2227,18 +2232,58 @@ spgdoinsert(Relation index, SpGistState *state, } /* + * Check new tuple size; fail if it can't fit, unless the + * opclass says it can handle the situation by suffixing. + * + * However, the opclass can only shorten the leaf datum, + * which may not be enough to ever make the tuple fit, + * since INCLUDE columns might alone use more than a page. + * Depending on the opclass' behavior, that could lead to + * an infinite loop --- spgtextproc.c, for example, will + * just repeatedly generate an empty-string leaf datum + * once it runs out of data. Actual bugs in opclasses + * might cause infinite looping, too. To detect such a + * loop, check to see if we are making progress by + * reducing the leafSize in each pass. This is a bit + * tricky though. Because of alignment considerations, + * the total tuple size might not decrease on every pass. + * Also, there are edge cases where the choose method + * might seem to not make progress for a cycle or two. + * Somewhat arbitrarily, we allow up to 10 no-progress + * iterations before failing. (This limit should be more + * than MAXALIGN, to accommodate opclasses that trim one + * byte from the leaf datum per pass.) + */ + if (leafSize > SPGIST_PAGE_CAPACITY) + { + bool ok = false; + + if (state->config.longValuesOK && !isnull) + { + if (leafSize < bestLeafSize) + { + ok = true; + bestLeafSize = leafSize; + numNoProgressCycles = 0; + } + else if (++numNoProgressCycles < 10) + ok = true; + } + if (!ok) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("index row size %zu exceeds maximum %zu for index \"%s\"", + leafSize - sizeof(ItemIdData), + SPGIST_PAGE_CAPACITY - sizeof(ItemIdData), + RelationGetRelationName(index)), + errhint("Values larger than a buffer page cannot be indexed."))); + } + + /* * Loop around and attempt to insert the new leafDatum at * "current" (which might reference an existing child * tuple, or might be invalid to force us to find a new * page for the tuple). - * - * Note: if the opclass sets longValuesOK, we rely on the - * choose function to eventually shorten the leafDatum - * enough to fit on a page. We could add a test here to - * complain if the datum doesn't get visibly shorter each - * time, but that could get in the way of opclasses that - * "simplify" datums in a way that doesn't necessarily - * lead to physical shortening on every cycle. */ break; case spgAddNode: diff --git a/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out b/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out index 0ac99d08fb..ac0ddcecea 100644 --- a/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out +++ b/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out @@ -61,6 +61,12 @@ select * from t binary_upgrade_set_next_toast_pg_class_oid | 1 | binary_upgrade_set_next_toast_pg_class_oid (9 rows) +-- Verify clean failure when INCLUDE'd columns result in overlength tuple +-- The error message details are platform-dependent, so show only SQLSTATE +\set VERBOSITY sqlstate +insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000)); +ERROR: 54000 +\set VERBOSITY default drop index t_f1_f2_f3_idx; create index on t using spgist(f1 name_ops_old) include(f2, f3); \d+ t_f1_f2_f3_idx @@ -100,3 +106,7 @@ select * from t binary_upgrade_set_next_toast_pg_class_oid | 1 | binary_upgrade_set_next_toast_pg_class_oid (9 rows) +\set VERBOSITY sqlstate +insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000)); +ERROR: 54000 +\set VERBOSITY default diff --git a/src/test/modules/spgist_name_ops/sql/spgist_name_ops.sql b/src/test/modules/spgist_name_ops/sql/spgist_name_ops.sql index 76e78ba41c..982f221a8b 100644 --- a/src/test/modules/spgist_name_ops/sql/spgist_name_ops.sql +++ b/src/test/modules/spgist_name_ops/sql/spgist_name_ops.sql @@ -27,6 +27,12 @@ select * from t where f1 > 'binary_upgrade_set_n' and f1 < 'binary_upgrade_set_p' order by 1; +-- Verify clean failure when INCLUDE'd columns result in overlength tuple +-- The error message details are platform-dependent, so show only SQLSTATE +\set VERBOSITY sqlstate +insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000)); +\set VERBOSITY default + drop index t_f1_f2_f3_idx; create index on t using spgist(f1 name_ops_old) include(f2, f3); @@ -39,3 +45,7 @@ select * from t select * from t where f1 > 'binary_upgrade_set_n' and f1 < 'binary_upgrade_set_p' order by 1; + +\set VERBOSITY sqlstate +insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000)); +\set VERBOSITY default |