From 73058d35ccb05e70c5d839155ea242ba1cc12c5e Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 7 Jul 2021 15:31:34 +0300 Subject: [PATCH] Not free (stripe) chunk buffers after de-serializing Previously, we were only using chunk group reader for sequential scan. However, to support index scans on columnar tables, now we use very same low level functions for index scan too. Since those low-level functions were only used for sequential scan, it was guaranteed that we would never read the same chunk group more than once, so we were freeing chunk buffers after deserializing them into a separate buffer. Now that we use those low level functions for index scan, we cannot free chunk buffers since it's possible to read the same chunk group again, such that: - read chunk group 1 of stripe 5 - read chunk group 2 of stripe 5 - read chunk group 1 of stripe 5 again Here, when we decide to read chunk group 1 for a second time, chunk group 1 is not cached. Plus, before this commit, we were freeing the chunk buffers for chunk group 1 after the first read and then we were getting segfault or errors from low-level de-compression APIs. --- src/backend/columnar/columnar_reader.c | 7 ------- src/test/regress/expected/columnar_indexes.out | 9 +++++++++ src/test/regress/sql/columnar_indexes.sql | 6 ++++++ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/backend/columnar/columnar_reader.c b/src/backend/columnar/columnar_reader.c index 5e686555d..5b5dbe46c 100644 --- a/src/backend/columnar/columnar_reader.c +++ b/src/backend/columnar/columnar_reader.c @@ -1378,13 +1378,6 @@ DeserializeChunkData(StripeBuffers *stripeBuffers, uint64 chunkIndex, chunkBuffers->valueCompressionType, chunkBuffers->decompressedValueSize); - if (chunkBuffers->valueCompressionType != COMPRESSION_NONE) - { - /* compressed data is not needed anymore */ - pfree(chunkBuffers->valueBuffer->data); - pfree(chunkBuffers->valueBuffer); - } - DeserializeBoolArray(chunkBuffers->existsBuffer, chunkData->existsArray[columnIndex], rowCount); diff --git a/src/test/regress/expected/columnar_indexes.out b/src/test/regress/expected/columnar_indexes.out index a61133a68..3789a8403 100644 --- a/src/test/regress/expected/columnar_indexes.out +++ b/src/test/regress/expected/columnar_indexes.out @@ -533,5 +533,14 @@ WHERE t (1 row) +CREATE TABLE revisit_same_cgroup(a INT, b TEXT) USING columnar; +CREATE INDEX ON revisit_same_cgroup USING HASH (b); +INSERT INTO revisit_same_cgroup SELECT random()*500, (random()*500)::INT::TEXT FROM generate_series(1, 100000) i; +SELECT sum(a)>-1 FROM revisit_same_cgroup WHERE b = '1'; + ?column? +--------------------------------------------------------------------- + t +(1 row) + SET client_min_messages TO WARNING; DROP SCHEMA columnar_indexes CASCADE; diff --git a/src/test/regress/sql/columnar_indexes.sql b/src/test/regress/sql/columnar_indexes.sql index d2caa120e..897181645 100644 --- a/src/test/regress/sql/columnar_indexes.sql +++ b/src/test/regress/sql/columnar_indexes.sql @@ -391,5 +391,11 @@ WHERE circle_col >= circle(point(7, 7), 350) AND float_col <= 5.0; +CREATE TABLE revisit_same_cgroup(a INT, b TEXT) USING columnar; +CREATE INDEX ON revisit_same_cgroup USING HASH (b); +INSERT INTO revisit_same_cgroup SELECT random()*500, (random()*500)::INT::TEXT FROM generate_series(1, 100000) i; + +SELECT sum(a)>-1 FROM revisit_same_cgroup WHERE b = '1'; + SET client_min_messages TO WARNING; DROP SCHEMA columnar_indexes CASCADE;