From ad2f2ca2c94d0172ebaf4eb08a1a0cfe2661d7e8 Mon Sep 17 00:00:00 2001 From: Ashwin Agrawal Date: Wed, 25 Jan 2017 10:48:25 -0800 Subject: [PATCH] Check overflow in delta calculation for delta compression. Delta between two adjacent tuples has fixed size in CO blocks of 29 bits during delta compression. So any value larger than it should be stored natively without applying delta compression to it. In cases where adjacent values are two far part delta calculation missed checking for overflows. Hence adding the check that if its negative don't apply delta compression, as logic always subtracts smaller number from larger number only for overflow case it will go negative. Also, adding validation tests for the same. Signed-off-by: Shreedhar Hardikar --- .../utils/datumstream/datumstreamblock.c | 8 ++++++- src/test/regress/expected/rle.out | 23 +++++++++++++++++++ src/test/regress/sql/rle.sql | 10 ++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/datumstream/datumstreamblock.c b/src/backend/utils/datumstream/datumstreamblock.c index bf7b605886..2074102cc6 100755 --- a/src/backend/utils/datumstream/datumstreamblock.c +++ b/src/backend/utils/datumstream/datumstreamblock.c @@ -3005,7 +3005,13 @@ DatumStreamBlockWrite_PerformDeltaCompression( break; } - if (delta > MAX_DELTA_SUPPORTED_DELTA_COMPRESSION) + /* + * Check if delta value fits the storage reserved for it. Important is to + * also check for overflow case, where delta goes negative. As logic above + * always subtracts smaller number from larger, delta must be positive + * except overflow case. + */ + if (delta < 0 || delta > MAX_DELTA_SUPPORTED_DELTA_COMPRESSION) { return DELTA_COMPRESSION_NOT_APPLIED; } diff --git a/src/test/regress/expected/rle.out b/src/test/regress/expected/rle.out index ff84c6f32b..000e04154f 100644 --- a/src/test/regress/expected/rle.out +++ b/src/test/regress/expected/rle.out @@ -11039,6 +11039,29 @@ select * from rle_runtest; (10000 rows) drop table rle_runtest; +-- This tests the delta compression's delta computation arithmetic. The bigint +-- column is used for the same and values inserted are INT64_MIN to INT64_MAX +-- to check extreme overflow case. +CREATE TABLE rle_overflowtest (a INT, b BIGINT, c char) WITH (appendonly=true, compresstype=rle_type, orientation=column, compresslevel=1); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +INSERT INTO rle_overflowtest VALUES (1,9223372036854775807,'a'), (1,-9,'b'); +INSERT INTO rle_overflowtest VALUES (1,-9,'c'), (1,9223372036854775807,'d'); +INSERT INTO rle_overflowtest VALUES (1,-9223372036854775808,'e'), (1,9223372036854775807,'f'); +INSERT INTO rle_overflowtest VALUES (1,9223372036854775807,'g'), (1,-9223372036854775808,'h'); +SELECT * FROM rle_overflowtest; + a | b | c +---+----------------------+--- + 1 | 9223372036854775807 | a + 1 | -9 | b + 1 | -9 | c + 1 | 9223372036854775807 | d + 1 | -9223372036854775808 | e + 1 | 9223372036854775807 | f + 1 | 9223372036854775807 | g + 1 | -9223372036854775808 | h +(8 rows) + DROP TABLE if exists CO_1_create_table_storage_directive_RLE_TYPE_8192_1; NOTICE: table "co_1_create_table_storage_directive_rle_type_8192_1" does not exist, skipping -- diff --git a/src/test/regress/sql/rle.sql b/src/test/regress/sql/rle.sql index 22aa152532..1a1a723491 100644 --- a/src/test/regress/sql/rle.sql +++ b/src/test/regress/sql/rle.sql @@ -20,6 +20,16 @@ insert into rle_runtest select 1, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' FROM generate select * from rle_runtest; drop table rle_runtest; +-- This tests the delta compression's delta computation arithmetic. The bigint +-- column is used for the same and values inserted are INT64_MIN to INT64_MAX +-- to check extreme overflow case. +CREATE TABLE rle_overflowtest (a INT, b BIGINT, c char) WITH (appendonly=true, compresstype=rle_type, orientation=column, compresslevel=1); +INSERT INTO rle_overflowtest VALUES (1,9223372036854775807,'a'), (1,-9,'b'); +INSERT INTO rle_overflowtest VALUES (1,-9,'c'), (1,9223372036854775807,'d'); +INSERT INTO rle_overflowtest VALUES (1,-9223372036854775808,'e'), (1,9223372036854775807,'f'); +INSERT INTO rle_overflowtest VALUES (1,9223372036854775807,'g'), (1,-9223372036854775808,'h'); +SELECT * FROM rle_overflowtest; + DROP TABLE if exists CO_1_create_table_storage_directive_RLE_TYPE_8192_1; -- -- GitLab