Skip to content

Commit

Permalink
Don't detoast compressed data on Bump memory context
Browse files Browse the repository at this point in the history
PG17 introduced a new Bump allocator for per-tuple memory
contexts. Bump doesn't support pfree(), which caused an error when
detoasting compressed data on the per-tuple memory context since the
detoasting needs to scan some catalog tables.

Make sure Hypercore TAM always detoasts on a temporary memory context
to fix this issue. Also add a test that triggers the failure when the
fix is not present.
  • Loading branch information
erimatnor committed Nov 20, 2024
1 parent ffc1269 commit 53299b9
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
8 changes: 7 additions & 1 deletion tsl/src/hypercore/arrow_array.c
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,13 @@ verify_offsets(const ArrowArray *array)
ArrowArray *
arrow_from_compressed(Datum compressed, Oid typid, MemoryContext dest_mcxt, MemoryContext tmp_mcxt)
{
/*
* Need to detoast on our temporary memory context because
* CurrentMemoryContext can be a per-tuple memory context which uses Bump
* allocator on PG17. The Bump allocator doesn't support pfree(), which is
* needed by detoasting since it does some catalog scans.
*/
MemoryContext oldcxt = MemoryContextSwitchTo(tmp_mcxt);
const CompressedDataHeader *header = (CompressedDataHeader *) PG_DETOAST_DATUM(compressed);
DecompressAllFunction decompress_all =
arrow_get_decompress_all(header->compression_algorithm, typid);
Expand All @@ -364,7 +371,6 @@ arrow_from_compressed(Datum compressed, Oid typid, MemoryContext dest_mcxt, Memo
format_type_be(typid),
NameStr(*compression_get_algorithm_name(header->compression_algorithm)));

MemoryContext oldcxt = MemoryContextSwitchTo(tmp_mcxt);
ArrowArray *array = decompress_all(PointerGetDatum(header), typid, dest_mcxt);

Assert(verify_offsets(array));
Expand Down
23 changes: 23 additions & 0 deletions tsl/test/expected/hypercore_scans.out
Original file line number Diff line number Diff line change
Expand Up @@ -511,3 +511,26 @@ select time, device+device as device_x2 from :chunk limit 1;
Wed Jun 01 00:55:00 2022 PDT | 2
(1 row)

-- Test sort using Bump memory context on PG17. This didn't use to
-- work on PG17 because it introduced a Bump memory context for
-- per-tuple processing on which compressed data was detoasted. This
-- doesn't work because Bump doesn't support pfree(), which is needed
-- by detoasting.
--
-- Need to convert all chunks to Hypercore TAM.
select compress_chunk(ch, hypercore_use_access_method=>true) from show_chunks('readings') ch;
compress_chunk
----------------------------------------
_timescaledb_internal._hyper_1_1_chunk
_timescaledb_internal._hyper_1_2_chunk
_timescaledb_internal._hyper_1_3_chunk
_timescaledb_internal._hyper_1_4_chunk
_timescaledb_internal._hyper_1_5_chunk
_timescaledb_internal._hyper_1_6_chunk
(6 rows)

-- Just test that this query doesn't fail with an error about Bump
-- allocator not supporting pfree. Redirect output to a temporary
-- table.
create temp table test_bump as
select * from readings order by time, device;
14 changes: 14 additions & 0 deletions tsl/test/sql/hypercore_scans.sql
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,17 @@ explain
select time, device+device as device_x2 from :chunk limit 1;
select time, device+device as device_x2 from :chunk limit 1;

-- Test sort using Bump memory context on PG17. This didn't use to
-- work on PG17 because it introduced a Bump memory context for
-- per-tuple processing on which compressed data was detoasted. This
-- doesn't work because Bump doesn't support pfree(), which is needed
-- by detoasting.
--
-- Need to convert all chunks to Hypercore TAM.
select compress_chunk(ch, hypercore_use_access_method=>true) from show_chunks('readings') ch;

-- Just test that this query doesn't fail with an error about Bump
-- allocator not supporting pfree. Redirect output to a temporary
-- table.
create temp table test_bump as
select * from readings order by time, device;

0 comments on commit 53299b9

Please sign in to comment.