Skip to content

Commit

Permalink
Fix scankey where consttype is different from var
Browse files Browse the repository at this point in the history
When we decide to use an indexscan on the segment-by column for any
query for decompression, then it's possible that the RHS constant type
is not the same as the variable on the LHS of the comparison.

A typical example is "int8 = int4" comparison. While this works ok
on 64 bit instances, it can crash on i386 ones. The issue is in the
scankey that we build for segment-by columns. Even though we specify
the valid "opcode" for cases where the arguments types don't match we
also need to specify the "sk_subtype" appropriately.

Fixes #7039
  • Loading branch information
nikkhils committed Jun 24, 2024
1 parent 86211f5 commit 2710536
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 6 deletions.
1 change: 1 addition & 0 deletions .unreleased/fix_7055
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #7055 Fix scankey where consttype is different from var
46 changes: 40 additions & 6 deletions tsl/src/compression/compression.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ static void row_compressor_flush(RowCompressor *row_compressor, CommandId mycid,

static int create_segment_filter_scankey(RowDecompressor *decompressor,
char *segment_filter_col_name, StrategyNumber strategy,
ScanKeyData *scankeys, int num_scankeys,
Oid subtype, ScanKeyData *scankeys, int num_scankeys,
Bitmapset **null_columns, Datum value, bool is_null_check,
bool is_array_op);
static void create_per_compressed_column(RowDecompressor *decompressor);
Expand Down Expand Up @@ -2059,6 +2059,7 @@ build_scankeys(Oid hypertable_relid, Oid out_rel, RowDecompressor *decompressor,
key_index = create_segment_filter_scankey(decompressor,
attname,
BTEqualStrategyNumber,
InvalidOid,
scankeys,
key_index,
null_columns,
Expand All @@ -2079,6 +2080,7 @@ build_scankeys(Oid hypertable_relid, Oid out_rel, RowDecompressor *decompressor,
key_index = create_segment_filter_scankey(decompressor,
column_segment_min_name(index),
BTLessEqualStrategyNumber,
InvalidOid,
scankeys,
key_index,
null_columns,
Expand All @@ -2088,6 +2090,7 @@ build_scankeys(Oid hypertable_relid, Oid out_rel, RowDecompressor *decompressor,
key_index = create_segment_filter_scankey(decompressor,
column_segment_max_name(index),
BTGreaterEqualStrategyNumber,
InvalidOid,
scankeys,
key_index,
null_columns,
Expand All @@ -2104,9 +2107,9 @@ build_scankeys(Oid hypertable_relid, Oid out_rel, RowDecompressor *decompressor,

static int
create_segment_filter_scankey(RowDecompressor *decompressor, char *segment_filter_col_name,
StrategyNumber strategy, ScanKeyData *scankeys, int num_scankeys,
Bitmapset **null_columns, Datum value, bool is_null_check,
bool is_array_op)
StrategyNumber strategy, Oid subtype, ScanKeyData *scankeys,
int num_scankeys, Bitmapset **null_columns, Datum value,
bool is_null_check, bool is_array_op)
{
AttrNumber cmp_attno = get_attnum(decompressor->in_rel->rd_id, segment_filter_col_name);
Assert(cmp_attno != InvalidAttrNumber);
Expand Down Expand Up @@ -2164,7 +2167,7 @@ create_segment_filter_scankey(RowDecompressor *decompressor, char *segment_filte
flags,
cmp_attno,
strategy,
InvalidOid, /* No strategy subtype. */
subtype,
decompressor->in_desc->attrs[AttrNumberGetAttrOffset(cmp_attno)]
.attcollation,
opr,
Expand Down Expand Up @@ -2765,6 +2768,34 @@ process_predicates(Chunk *ch, CompressionSettings *settings, List *predicates, L
}
}

/*
* Get the subtype for an indexscan from the provided filter. We also
* need to handle array constants appropriately.
*/
static Oid
deduce_filter_subtype(BatchFilter *filter, Oid att_typoid)
{
Oid subtype = InvalidOid;

if (!filter->value)
return InvalidOid;

/*
* Check if the filter type is different from the att type. If yes, the
* subtype needs to be set appropriately.
*/
if (att_typoid != filter->value->consttype)
{
/* For an array type get its element type */
if (filter->is_array_op)
subtype = get_element_type(filter->value->consttype);
else
subtype = filter->value->consttype;
}

return subtype;
}

/*
* This method will build scan keys for predicates including
* SEGMENT BY column with attribute number from compressed chunk
Expand All @@ -2785,6 +2816,7 @@ build_update_delete_scankeys(RowDecompressor *decompressor, List *heap_filters,
{
filter = lfirst(lc);
AttrNumber attno = get_attnum(decompressor->in_rel->rd_id, NameStr(filter->column_name));
Oid typoid = get_atttype(decompressor->in_rel->rd_id, attno);
if (attno == InvalidAttrNumber)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_COLUMN),
Expand All @@ -2795,6 +2827,7 @@ build_update_delete_scankeys(RowDecompressor *decompressor, List *heap_filters,
key_index = create_segment_filter_scankey(decompressor,
NameStr(filter->column_name),
filter->strategy,
deduce_filter_subtype(filter, typoid),
scankeys,
key_index,
null_columns,
Expand Down Expand Up @@ -2979,6 +3012,7 @@ build_index_scankeys(Relation index_rel, List *index_filters, int *num_scankeys)
for (int attno = 1; attno <= index_rel->rd_index->indnatts && idx < *num_scankeys; attno++)
{
char *attname = get_attname(RelationGetRelid(index_rel), attno, false);
Oid typoid = get_atttype(RelationGetRelid(index_rel), attno);
foreach (lc, index_filters)
{
filter = lfirst(lc);
Expand All @@ -2998,7 +3032,7 @@ build_index_scankeys(Relation index_rel, List *index_filters, int *num_scankeys)
flags,
attno,
filter->strategy,
InvalidOid, /* no strategy subtype */
deduce_filter_subtype(filter, typoid), /* subtype */
filter->collation,
filter->opcode,
filter->value ? filter->value->constvalue : 0);
Expand Down
28 changes: 28 additions & 0 deletions tsl/test/expected/compression.out
Original file line number Diff line number Diff line change
Expand Up @@ -2847,3 +2847,31 @@ COPY compressed_table (time,a,b,c) FROM stdin;
ERROR: tuple decompression limit exceeded by operation
\set ON_ERROR_STOP 1
RESET timescaledb.max_tuples_decompressed_per_dml_transaction;
-- Test decompression with DML which compares int8 to int4
CREATE TABLE hyper_84 (time timestamptz, device int8, location int8, temp float8);
SELECT create_hypertable('hyper_84', 'time', create_default_indexes => false);
NOTICE: adding not-null constraint to column "time"
create_hypertable
------------------------
(51,public,hyper_84,t)
(1 row)

INSERT INTO hyper_84 VALUES ('2024-01-01', 1, 1, 1.0);
ALTER TABLE hyper_84 SET (timescaledb.compress, timescaledb.compress_segmentby='device');
NOTICE: default order by for hypertable "hyper_84" is set to ""time" DESC"
SELECT compress_chunk(ch) FROM show_chunks('hyper_84') ch;
compress_chunk
-------------------------------------------
_timescaledb_internal._hyper_51_110_chunk
(1 row)

-- indexscan for decompression: UPDATE
UPDATE hyper_84 SET temp = 100 where device = 1;
SELECT compress_chunk(ch) FROM show_chunks('hyper_84') ch;
compress_chunk
-------------------------------------------
_timescaledb_internal._hyper_51_110_chunk
(1 row)

-- indexscan for decompression: DELETE
DELETE FROM hyper_84 WHERE device = 1;
12 changes: 12 additions & 0 deletions tsl/test/sql/compression.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1159,3 +1159,15 @@ COPY compressed_table (time,a,b,c) FROM stdin;
\.
\set ON_ERROR_STOP 1
RESET timescaledb.max_tuples_decompressed_per_dml_transaction;

-- Test decompression with DML which compares int8 to int4
CREATE TABLE hyper_84 (time timestamptz, device int8, location int8, temp float8);
SELECT create_hypertable('hyper_84', 'time', create_default_indexes => false);
INSERT INTO hyper_84 VALUES ('2024-01-01', 1, 1, 1.0);
ALTER TABLE hyper_84 SET (timescaledb.compress, timescaledb.compress_segmentby='device');
SELECT compress_chunk(ch) FROM show_chunks('hyper_84') ch;
-- indexscan for decompression: UPDATE
UPDATE hyper_84 SET temp = 100 where device = 1;
SELECT compress_chunk(ch) FROM show_chunks('hyper_84') ch;
-- indexscan for decompression: DELETE
DELETE FROM hyper_84 WHERE device = 1;

0 comments on commit 2710536

Please sign in to comment.