Skip to content

Commit a5718cb

Browse files
dbeckdbeck
committed
Fixing NULL compression dump/restore issue
As reported in the bug report 2912, dumping and restoring a column using NULL compression failed. This change fixes this issue.
1 parent 4800693 commit a5718cb

File tree

8 files changed

+160
-18
lines changed

8 files changed

+160
-18
lines changed

‎.unreleased/pr_8153‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixes: #8153 Restoring a database having NULL compressed data

‎test/sql/utils/pg_dump_aux_plain_dump.sh‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
DUMPFILE=${DUMPFILE:-$1}
2+
EXTRA_PGOPTIONS=${EXTRA_PGOPTIONS:-$2}
3+
DUMP_OPTIONS=${DUMP_OPTIONS:-$3}
4+
# Override PGOPTIONS to remove verbose output
5+
PGOPTIONS="--client-min-messages=warning $EXTRA_PGOPTIONS"
6+
7+
export PGOPTIONS
8+
echo ${DUMP_OPTIONS}
9+
echo $DUMP_OPTIONS
10+
echo $(echo $DUMP_OPTIONS)
11+
12+
${PG_BINDIR}/pg_dump -h ${PGHOST} -U ${TEST_ROLE_SUPERUSER} ${DUMP_OPTIONS} -Fp ${TEST_DBNAME} -f ${DUMPFILE}
13+
# ${PG_BINDIR}/pg_dump -h ${PGHOST} -U ${TEST_ROLE_SUPERUSER} ${DUMP_OPTIONS} -Fp ${TEST_DBNAME} > /dev/null 2>&1 -f ${DUMPFILE}
14+
${PG_BINDIR}/dropdb -h ${PGHOST} -U ${TEST_ROLE_SUPERUSER} ${TEST_DBNAME}
15+
${PG_BINDIR}/createdb -h ${PGHOST} -U ${TEST_ROLE_SUPERUSER} ${TEST_DBNAME}

‎tsl/src/compression/algorithms/null.c‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,15 @@ null_compressed_send(CompressedDataHeader *header, StringInfo buffer)
3535
extern Datum
3636
null_compressed_recv(StringInfo buffer)
3737
{
38-
elog(ERROR, "null compression doesn't implement recv");
39-
PG_RETURN_VOID();
38+
/* Sanity checks for invalid buffer */
39+
if (buffer->len == 0)
40+
ereport(ERROR,
41+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
42+
errmsg("compressed data is invalid to be a null compressed block")));
43+
if (buffer->data == NULL)
44+
ereport(ERROR,
45+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("compressed data is NULL")));
46+
PG_RETURN_POINTER(null_compressor_get_dummy_block());
4047
}
4148

4249
extern Compressor *

‎tsl/src/compression/compression.c‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2015,14 +2015,7 @@ tsl_compressed_data_recv(PG_FUNCTION_ARGS)
20152015
if (header.compression_algorithm >= _END_COMPRESSION_ALGORITHMS)
20162016
elog(ERROR, "invalid compression algorithm %d", header.compression_algorithm);
20172017

2018-
if (header.compression_algorithm == COMPRESSION_ALGORITHM_NULL)
2019-
{
2020-
PG_RETURN_NULL();
2021-
}
2022-
else
2023-
{
2024-
return definitions[header.compression_algorithm].compressed_data_recv(buf);
2025-
}
2018+
return definitions[header.compression_algorithm].compressed_data_recv(buf);
20262019
}
20272020

20282021
extern Datum

‎tsl/test/expected/compression_null_dump_restore.out‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
-- This file and its contents are licensed under the Timescale License.
2+
-- Please see the included NOTICE for copyright information and
3+
-- LICENSE-TIMESCALE for a copy of the license.
4+
-- This test is to verify fixing the behaviour reported in: SDC #2912
5+
-- Create destination database
6+
\set TEST_DBNAME_3 :TEST_DBNAME _3
7+
\c postgres :ROLE_SUPERUSER
8+
CREATE DATABASE :TEST_DBNAME_3;
9+
\c :TEST_DBNAME_3 :ROLE_SUPERUSER
10+
create extension timescaledb CASCADE;
11+
-- Create source database
12+
\set TEST_DBNAME_2 :TEST_DBNAME _2
13+
\c postgres :ROLE_SUPERUSER
14+
CREATE DATABASE :TEST_DBNAME_2;
15+
\c :TEST_DBNAME_2 :ROLE_SUPERUSER
16+
create extension timescaledb CASCADE;
17+
-- Create compressed table with one column having all NULL
18+
-- values, so it will be compressed with the NULL algorithm
19+
--
20+
create table null_dump (ts int primary key, n int);
21+
select create_hypertable('null_dump', 'ts');
22+
create_hypertable
23+
------------------------
24+
(1,public,null_dump,t)
25+
(1 row)
26+
27+
insert into null_dump values (1), (2), (3), (4);
28+
alter table null_dump set (timescaledb.compress, timescaledb.compress_orderby = 'ts');
29+
select compress_chunk(show_chunks('null_dump'));
30+
compress_chunk
31+
----------------------------------------
32+
_timescaledb_internal._hyper_1_1_chunk
33+
(1 row)
34+
35+
-- Dump the content of the source database
36+
\setenv TEST_DBNAME :TEST_DBNAME_2
37+
\setenv DUMP_OPTIONS '--quote-all-identifiers --no-tablespaces --no-owner --no-privileges --exclude-schema=test'
38+
\c postgres :ROLE_SUPERUSER
39+
\! utils/pg_dump_aux_plain_dump.sh dump/null-compress-dump.sql
40+
--quote-all-identifiers --no-tablespaces --no-owner --no-privileges --exclude-schema=test
41+
--quote-all-identifiers --no-tablespaces --no-owner --no-privileges --exclude-schema=test
42+
--quote-all-identifiers --no-tablespaces --no-owner --no-privileges --exclude-schema=test
43+
pg_dump: warning: there are circular foreign-key constraints on this table:
44+
pg_dump: detail: hypertable
45+
pg_dump: hint: You might not be able to restore the dump without using --disable-triggers or temporarily dropping the constraints.
46+
pg_dump: hint: Consider using a full dump instead of a --data-only dump to avoid this problem.
47+
pg_dump: warning: there are circular foreign-key constraints on this table:
48+
pg_dump: detail: chunk
49+
pg_dump: hint: You might not be able to restore the dump without using --disable-triggers or temporarily dropping the constraints.
50+
pg_dump: hint: Consider using a full dump instead of a --data-only dump to avoid this problem.
51+
pg_dump: warning: there are circular foreign-key constraints on this table:
52+
pg_dump: detail: continuous_agg
53+
pg_dump: hint: You might not be able to restore the dump without using --disable-triggers or temporarily dropping the constraints.
54+
pg_dump: hint: Consider using a full dump instead of a --data-only dump to avoid this problem.
55+
-- Restore the data into the destination database
56+
\c :TEST_DBNAME_3 :ROLE_SUPERUSER
57+
select public.timescaledb_pre_restore();
58+
timescaledb_pre_restore
59+
-------------------------
60+
t
61+
(1 row)
62+
63+
\set ECHO none
64+
select public.timescaledb_post_restore();
65+
timescaledb_post_restore
66+
--------------------------
67+
t
68+
(1 row)
69+
70+
select * from public.null_dump order by 1;
71+
ts | n
72+
----+---
73+
1 |
74+
2 |
75+
3 |
76+
4 |
77+
(4 rows)
78+
79+
drop database db_compression_null_dump_restore_2;
80+
drop database db_compression_null_dump_restore_3;

‎tsl/test/sql/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ set(TEST_FILES
3333
compression_fks.sql
3434
compression_indexcreate.sql
3535
compression_insert.sql
36+
compression_null_dump_restore.sql
3637
compression_nulls_and_defaults.sql
3738
compression_policy.sql
3839
compression_qualpushdown.sql

‎tsl/test/sql/compression_null_dump_restore.sql‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
-- This file and its contents are licensed under the Timescale License.
2+
-- Please see the included NOTICE for copyright information and
3+
-- LICENSE-TIMESCALE for a copy of the license.
4+
5+
-- This test is to verify fixing the behaviour reported in: SDC #2912
6+
7+
-- Create destination database
8+
\set TEST_DBNAME_3 :TEST_DBNAME _3
9+
\c postgres :ROLE_SUPERUSER
10+
CREATE DATABASE :TEST_DBNAME_3;
11+
\c :TEST_DBNAME_3 :ROLE_SUPERUSER
12+
create extension timescaledb CASCADE;
13+
14+
-- Create source database
15+
\set TEST_DBNAME_2 :TEST_DBNAME _2
16+
\c postgres :ROLE_SUPERUSER
17+
CREATE DATABASE :TEST_DBNAME_2;
18+
\c :TEST_DBNAME_2 :ROLE_SUPERUSER
19+
create extension timescaledb CASCADE;
20+
21+
-- Create compressed table with one column having all NULL
22+
-- values, so it will be compressed with the NULL algorithm
23+
--
24+
create table null_dump (ts int primary key, n int);
25+
select create_hypertable('null_dump', 'ts');
26+
insert into null_dump values (1), (2), (3), (4);
27+
alter table null_dump set (timescaledb.compress, timescaledb.compress_orderby = 'ts');
28+
select compress_chunk(show_chunks('null_dump'));
29+
30+
-- Dump the content of the source database
31+
\setenv TEST_DBNAME :TEST_DBNAME_2
32+
\setenv DUMP_OPTIONS '--quote-all-identifiers --no-tablespaces --no-owner --no-privileges --exclude-schema=test'
33+
\c postgres :ROLE_SUPERUSER
34+
\! utils/pg_dump_aux_plain_dump.sh dump/null-compress-dump.sql
35+
36+
-- Restore the data into the destination database
37+
\c :TEST_DBNAME_3 :ROLE_SUPERUSER
38+
select public.timescaledb_pre_restore();
39+
\set ECHO none
40+
\o /dev/null
41+
SET client_min_messages = 'error';
42+
\i dump/null-compress-dump.sql
43+
\o
44+
\set ECHO queries
45+
select public.timescaledb_post_restore();
46+
47+
select * from public.null_dump order by 1;
48+
49+
\c postgres :ROLE_SUPERUSER
50+
drop database :TEST_DBNAME_2;
51+
drop database :TEST_DBNAME_3;

‎tsl/test/src/compression_unit_test.c‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -999,14 +999,8 @@ test_null()
999999
TestAssertTrue(transmission.len > 0);
10001000
TestAssertTrue(transmission.data != NULL);
10011001

1002-
LOCAL_FCINFO(local_fcinfo, 1);
1003-
local_fcinfo->args[0] =
1004-
(NullableDatum){ .value = PointerGetDatum(&transmission), .isnull = false };
1005-
1006-
// Call the function directly
1007-
tsl_compressed_data_recv(local_fcinfo);
1008-
1009-
TestAssertTrue(local_fcinfo->isnull);
1002+
Datum res = DirectFunctionCall1(tsl_compressed_data_recv, PointerGetDatum(&transmission));
1003+
TestAssertTrue(DatumGetPointer(res) != NULL);
10101004
}
10111005
{
10121006
void *compressed = null_compressor_get_dummy_block();

0 commit comments

Comments
 (0)