Thanks for the PR!

Other structures, e.g. RowGroup, ColumnData, etc suffer from the same problem. What we did there was to add a separate constructor that essentially moves all of the data from one entry to another. Could we do the same here? That would simplify the code and allow us to keep ColumnData as a reference in the UpdateSegment.

I'm not entirely sure if I understood what you meant

I tried to propagate the moves to StringHeap but at some point I hit Allocator, which has an explicit copy constructor so it was impossible to make a move constructor there.

Instead the StringHeap is now wrapped in a unique pointer so we can properly move it to the new instance.

The new changes indeed look like what I meant. I believe the StringHeap can be moved though - the ArenaAllocator has an explicit .Move() method. It might just require implementing an explicit move constructor for the ArenaAllocator and StringHeap classes.

Ah you're right, completely missed the StringHeap::Move() method, this is much better 👍

There's one test failing, that I can't reproduce.
I ran the exact same build command, but the test does not fail.

Also looking through the code I can't see how this assertion fails.
The StringHeap gets default constructed, using the DefaultAllocator, this in turns allocates the ArenaAllocator with this Allocator, and that constructor looks like this:

ArenaAllocator::ArenaAllocator(Allocator &allocator, idx_t initial_capacity) : allocator(allocator) {
	head = nullptr;
	tail = nullptr;
	current_capacity = initial_capacity;
}

The failing assertion is this:

TransactionContext Error: Failed to commit: INTERNAL Error: Assertion triggered in file "/home/runner/work/duckdb/duckdb/src/storage/arena_allocator.cpp" on line 104: !other.head
[1965/2192] (89%): test/issues/general/test_2240.test   

EDIT:
I was running the wrong test ;)

Thanks!