From 3cf43195e54439433854cb434a27e851b7ac6528 Mon Sep 17 00:00:00 2001 From: Michael Zhilin Date: Thu, 14 Dec 2023 16:08:15 +0300 Subject: [PATCH v4 1/3] contrib/amcheck: must support different header size of short varlena datum --- contrib/amcheck/expected/check_btree.out | 17 +++++++++++ contrib/amcheck/sql/check_btree.sql | 16 +++++++++++ contrib/amcheck/verify_nbtree.c | 36 ++++++++++++++++++++---- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out index 86b38d93f41..6ef7915b319 100644 --- a/contrib/amcheck/expected/check_btree.out +++ b/contrib/amcheck/expected/check_btree.out @@ -1,3 +1,4 @@ +-- directory paths are passed to us in environment variables CREATE TABLE bttest_a(id int8); CREATE TABLE bttest_b(id int8); CREATE TABLE bttest_multi(id int8, data int8); @@ -240,6 +241,21 @@ SELECT bt_index_check('bttest_unique_nulls_b_c_idx', heapallindexed => true, che (1 row) +-- +-- BUG: must support different header size of short varlena datum +-- https://postgr.es/m/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc@postgrespro.ru +-- +CREATE TABLE varlena_bug (v text); +ALTER TABLE varlena_bug ALTER column v SET storage plain; +INSERT INTO varlena_bug VALUES ('x'); +COPY varlena_bug from stdin; +CREATE INDEX varlena_bug_idx on varlena_bug(v); +SELECT bt_index_check('varlena_bug_idx', true); + bt_index_check +---------------- + +(1 row) + -- cleanup DROP TABLE bttest_a; DROP TABLE bttest_b; @@ -250,3 +266,4 @@ DROP FUNCTION ifun(int8); DROP TABLE bttest_unique_nulls; DROP OWNED BY regress_bttest_role; -- permissions DROP ROLE regress_bttest_role; +DROP TABLE varlena_bug; diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql index aa461f7fb97..e17924b1549 100644 --- a/contrib/amcheck/sql/check_btree.sql +++ b/contrib/amcheck/sql/check_btree.sql @@ -1,3 +1,4 @@ +-- directory paths are passed to us in environment variables CREATE TABLE bttest_a(id int8); CREATE TABLE bttest_b(id int8); CREATE TABLE bttest_multi(id int8, data int8); @@ -148,6 +149,20 @@ SELECT bt_index_check('bttest_unique_nulls_c_key', heapallindexed => true, check CREATE INDEX on bttest_unique_nulls (b,c); SELECT bt_index_check('bttest_unique_nulls_b_c_idx', heapallindexed => true, checkunique => true); +-- +-- BUG: must support different header size of short varlena datum +-- https://postgr.es/m/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc@postgrespro.ru +-- + +CREATE TABLE varlena_bug (v text); +ALTER TABLE varlena_bug ALTER column v SET storage plain; +INSERT INTO varlena_bug VALUES ('x'); +COPY varlena_bug from stdin; +x +\. +CREATE INDEX varlena_bug_idx on varlena_bug(v); +SELECT bt_index_check('varlena_bug_idx', true); + -- cleanup DROP TABLE bttest_a; DROP TABLE bttest_b; @@ -158,3 +173,4 @@ DROP FUNCTION ifun(int8); DROP TABLE bttest_unique_nulls; DROP OWNED BY regress_bttest_role; -- permissions DROP ROLE regress_bttest_role; +DROP TABLE varlena_bug; diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 91caa53dd8b..e7f01c2addb 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -2942,7 +2942,7 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup) TupleDesc tupleDescriptor = RelationGetDescr(state->rel); Datum normalized[INDEX_MAX_KEYS]; bool isnull[INDEX_MAX_KEYS]; - bool toast_free[INDEX_MAX_KEYS]; + bool need_free[INDEX_MAX_KEYS]; bool formnewtup = false; IndexTuple reformed; int i; @@ -2961,7 +2961,7 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup) att = TupleDescAttr(tupleDescriptor, i); /* Assume untoasted/already normalized datum initially */ - toast_free[i] = false; + need_free[i] = false; normalized[i] = index_getattr(itup, att->attnum, tupleDescriptor, &isnull[i]); @@ -2973,6 +2973,7 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup) * index without further processing, so an external varlena header * should never be encountered here */ + if (VARATT_IS_EXTERNAL(DatumGetPointer(normalized[i]))) ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), @@ -2984,11 +2985,32 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup) { formnewtup = true; normalized[i] = PointerGetDatum(PG_DETOAST_DATUM(normalized[i])); - toast_free[i] = true; + need_free[i] = true; } + /* + * Short tuples may have 1B or 4B header. Convert 4B header of short + * tuples to 1B + */ + else if (VARATT_CAN_MAKE_SHORT(DatumGetPointer(normalized[i]))) + { + /* convert to short varlena */ + Size len = VARATT_CONVERTED_SHORT_SIZE(DatumGetPointer(normalized[i])); + char *data = palloc(len); + + SET_VARSIZE_SHORT(data, len); + memcpy(data + 1, VARDATA(DatumGetPointer(normalized[i])), len - 1); + + formnewtup = true; + normalized[i] = PointerGetDatum(data); + need_free[i] = true; + } + } - /* Easier case: Tuple has varlena datums, none of which are compressed */ + /* + * Easier case: Tuple has varlena datums, none of which are compressed or + * short with 4B header + */ if (!formnewtup) return itup; @@ -2997,6 +3019,10 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup) * creating normalized version of the tuple from uncompressed input datums * (normalized input datums). This is rather naive, but shouldn't be * necessary too often. + * Also tuple had short varlena datums with 4B header. Actually there is no + * restriction with have heap tuple containing varlena datum with 4B header + * and corresponding index tuple containing varlena datum with 1B header. + * For fingerprinting let's convert heap tuple varlena datum to 1B format. * * Note that we rely on deterministic index_form_tuple() TOAST compression * of normalized input. @@ -3006,7 +3032,7 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup) /* Cannot leak memory here */ for (i = 0; i < tupleDescriptor->natts; i++) - if (toast_free[i]) + if (need_free[i]) pfree(DatumGetPointer(normalized[i])); return reformed; -- 2.43.0