diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 899005609c2..a584e138ada 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -2819,7 +2819,7 @@ apply_concurrent_changes(BufFile *file, ChangeContext *chgcxt) /* * Adjust spilled_tuple so that it can be used as the new tuple in * the update that we're about to replay. This fixes TOAST - * pointers as well as remove useless values from dropped columns. + * pointers. */ prepare_concurrent_update(spilled_tuple, ondisk_tuple); @@ -2948,6 +2948,7 @@ restore_tuple(BufFile *file, Relation relation, TupleTableSlot *slot) { uint32 t_len; HeapTuple tup; + TupleDesc desc = slot->tts_tupleDescriptor; int natt_ext; /* Read the tuple. */ @@ -2965,6 +2966,16 @@ restore_tuple(BufFile *file, Relation relation, TupleTableSlot *slot) */ ExecForceStoreHeapTuple(tup, slot, false); + /* + * Dropped columns can still have values in some tuples. Null them out. + * This saves space, and the new heap might not even have a TOAST table. + */ + for (int i = 0; i < desc->natts; i++) + { + if (TupleDescCompactAttr(desc, i)->attisdropped) + slot->tts_isnull[i] = true; + } + /* * Next, read any attributes we stored separately into the tts_values * array elements expecting them, if any. This matches @@ -2973,8 +2984,6 @@ restore_tuple(BufFile *file, Relation relation, TupleTableSlot *slot) BufFileReadExact(file, &natt_ext, sizeof(natt_ext)); if (natt_ext > 0) { - TupleDesc desc = slot->tts_tupleDescriptor; - for (int i = 0; i < desc->natts; i++) { CompactAttribute *attr = TupleDescCompactAttr(desc, i); @@ -3017,13 +3026,10 @@ restore_tuple(BufFile *file, Relation relation, TupleTableSlot *slot) * update that we're about to replay. * * We perform the following critical change: - * - Any EXTERNAL_ONDISK toast pointers so that it points to the corresponding - * toast value in 'src' (the transient table) instead. The TOAST storage for - * 'dest' is going to be dropped, so these values cannot be used any longer. - * - * We also apply the following optimization: - * - If any columns are dropped but the slot still contains values, mark them - * as null to avoid uselessly wasting space in the new relation. + * - Update any EXTERNAL_ONDISK toast pointers so that they point to the + * corresponding toast values in 'src' (the transient table) instead. + * The TOAST storage for 'dest' is going to be dropped, so these values + * cannot be used any longer. */ static void prepare_concurrent_update(TupleTableSlot *dest, TupleTableSlot *src) @@ -3036,14 +3042,7 @@ prepare_concurrent_update(TupleTableSlot *dest, TupleTableSlot *src) varlena *varlena_dst; if (attr->attisdropped) - { - if (!slot_attisnull(dest, i + 1)) - { - slot_getsomeattrs(dest, i + 1); - dest->tts_isnull[i] = true; - } continue; - } if (attr->attlen != -1) continue; if (slot_attisnull(dest, i + 1)) diff --git a/src/test/modules/injection_points/expected/repack_dropped.out b/src/test/modules/injection_points/expected/repack_dropped.out index 8aaab73aebb..4186ce4981c 100644 --- a/src/test/modules/injection_points/expected/repack_dropped.out +++ b/src/test/modules/injection_points/expected/repack_dropped.out @@ -1,6 +1,6 @@ Parsed test spec with 3 sessions -starting permutation: s1_size s2_repack s3_updates s1_unlock s2_noop s1_size +starting permutation: s1_size s2_repack s3_updates s3_insert s1_unlock s2_noop s1_size injection_points_attach ----------------------- @@ -23,6 +23,9 @@ step s2_repack: step s3_updates: UPDATE repack_dropped SET a = a || a; +step s3_insert: + INSERT INTO repack_dropped (id, a) VALUES (3, 'three'); + step s1_unlock: SELECT injection_points_wakeup('repack-concurrently-before-lock'); @@ -43,5 +46,6 @@ id|fits_in_one_block --+----------------- 1|t 2|t -(2 rows) + 3|t +(3 rows) diff --git a/src/test/modules/injection_points/specs/repack_dropped.spec b/src/test/modules/injection_points/specs/repack_dropped.spec index 878cdeffc5c..8dabd4fafb4 100644 --- a/src/test/modules/injection_points/specs/repack_dropped.spec +++ b/src/test/modules/injection_points/specs/repack_dropped.spec @@ -13,11 +13,18 @@ setup { INSERT INTO repack_dropped (id, a, b) VALUES (2, 'two', repeat(encode(sha256('2'), 'hex'), current_setting('block_size')::int / 32)); CREATE FUNCTION repack_dropped_f() RETURNS trigger LANGUAGE plpgsql AS - $$ BEGIN + $$ DECLARE r repack_dropped; + BEGIN + IF TG_OP = 'INSERT' THEN + r := (SELECT t FROM repack_dropped t WHERE id = 1); + r.id := NEW.id; + RETURN r; + END IF; IF NEW.id = 1 THEN return OLD; END IF; RETURN NEW; END $$; CREATE TRIGGER repack_dropped_t BEFORE UPDATE ON repack_dropped FOR EACH ROW EXECUTE FUNCTION repack_dropped_f(); + CREATE TRIGGER repack_dropped_ins_t BEFORE INSERT ON repack_dropped FOR EACH ROW EXECUTE FUNCTION repack_dropped_f(); ALTER TABLE repack_dropped DROP COLUMN b; } @@ -63,10 +70,17 @@ step s3_updates UPDATE repack_dropped SET a = a || a; } +# The trigger makes tuple 3 a copy of tuple 1, dropped column included. +step s3_insert +{ + INSERT INTO repack_dropped (id, a) VALUES (3, 'three'); +} + permutation s1_size s2_repack s3_updates + s3_insert s1_unlock s2_noop s1_size