Skip to content

Commit

Permalink
Fix DELETE on compressed chunk with non-btree operators
Browse files Browse the repository at this point in the history 
When deleting from compressed chunk the direct delete optimization
would ignore constraints that were not using btree operators
leading to constraints of the DELETE not being applied to the
direct delete on the compressed chunk, potentially leading to
data corruption. This patch disables the direct delete optimization
when any of the constraints can not be applied.

Fixes #7644
  • Loading branch information
@svenklemm
svenklemm committed Feb 3, 2025
1 parent c01ad17 commit 253a7cb
Show file tree
Hide file tree
Showing 6 changed files with 249 additions and 75 deletions.
1 change: 1 addition & 0 deletions .unreleased/pr_7645
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #7645 Fix DELETE on compressed chunk with non-btree operators
25 changes: 22 additions & 3 deletions tsl/src/compression/compression_dml.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ decompress_batches_for_update_delete(HypertableModifyState *ht_state, Chunk *chu
scankeys = build_update_delete_scankeys(comp_chunk_rel,
heap_filters,
&num_scankeys,
&null_columns);
&null_columns,
&delete_only);
}

if (matching_index_rel)
Expand Down Expand Up @@ -957,7 +958,16 @@ process_predicates(Chunk *ch, CompressionSettings *settings, List *predicates,
break;
}
default:
/* Do nothing for unknown operator strategies. */
*heap_filters = lappend(*heap_filters,
make_batchfilter(column_name,
op_strategy,
collation,
opcode,
arg_value,
false, /* is_null_check */
false, /* is_null */
false /* is_array_op */
));
break;
}
continue;
Expand Down Expand Up @@ -1109,7 +1119,16 @@ process_predicates(Chunk *ch, CompressionSettings *settings, List *predicates,
break;
}
default:
/* Do nothing on unknown operator strategies. */
*heap_filters = lappend(*heap_filters,
make_batchfilter(column_name,
op_strategy,
collation,
opcode,
arg_value,
false, /* is_null_check */
false, /* is_null */
true /* is_array_op */
));
break;
}
continue;
Expand Down
2 changes: 1 addition & 1 deletion tsl/src/compression/compression_dml.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ ScanKeyData *build_heap_scankeys(Oid hypertable_relid, Relation in_rel, Relation
CompressionSettings *settings, Bitmapset *key_columns,
Bitmapset **null_columns, TupleTableSlot *slot, int *num_scankeys);
ScanKeyData *build_update_delete_scankeys(Relation in_rel, List *heap_filters, int *num_scankeys,
Bitmapset **null_columns);
Bitmapset **null_columns, bool *delete_only);
118 changes: 64 additions & 54 deletions tsl/src/compression/compression_scankey.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
#include "ts_catalog/array_utils.h"

static Oid deduce_filter_subtype(BatchFilter *filter, Oid att_typoid);
static int create_segment_filter_scankey(Relation in_rel, char *segment_filter_col_name,
StrategyNumber strategy, Oid subtype,
ScanKeyData *scankeys, int num_scankeys,
Bitmapset **null_columns, Datum value, bool is_null_check,
bool is_array_op);
static bool create_segment_filter_scankey(Relation in_rel, char *segment_filter_col_name,
StrategyNumber strategy, Oid subtype,
ScanKeyData *scankeys, int *num_scankeys,
Bitmapset **null_columns, Datum value, bool is_null_check,
bool is_array_op);

/*
* Test ScanKey against a slot.
Expand Down Expand Up @@ -187,16 +187,16 @@ build_heap_scankeys(Oid hypertable_relid, Relation in_rel, Relation out_rel,
*/
if (ts_array_is_member(settings->fd.segmentby, attname))
{
key_index = create_segment_filter_scankey(in_rel,
attname,
BTEqualStrategyNumber,
InvalidOid,
scankeys,
key_index,
null_columns,
value,
isnull,
false);
create_segment_filter_scankey(in_rel,
attname,
BTEqualStrategyNumber,
InvalidOid,
scankeys,
&key_index,
null_columns,
value,
isnull,
false);
}
if (ts_array_is_member(settings->fd.orderby, attname))
{
Expand All @@ -208,26 +208,28 @@ build_heap_scankeys(Oid hypertable_relid, Relation in_rel, Relation out_rel,

int16 index = ts_array_position(settings->fd.orderby, attname);

key_index = create_segment_filter_scankey(in_rel,
column_segment_min_name(index),
BTLessEqualStrategyNumber,
InvalidOid,
scankeys,
key_index,
null_columns,
value,
false,
false); /* is_null_check */
key_index = create_segment_filter_scankey(in_rel,
column_segment_max_name(index),
BTGreaterEqualStrategyNumber,
InvalidOid,
scankeys,
key_index,
null_columns,
value,
false,
false); /* is_null_check */
create_segment_filter_scankey(in_rel,
column_segment_min_name(index),
BTLessEqualStrategyNumber,
InvalidOid,
scankeys,
&key_index,
null_columns,
value,
false,
false /* is_null_check */
);
create_segment_filter_scankey(in_rel,
column_segment_max_name(index),
BTGreaterEqualStrategyNumber,
InvalidOid,
scankeys,
&key_index,
null_columns,
value,
false,
false /* is_null_check */
);
}
}
}
Expand Down Expand Up @@ -435,7 +437,7 @@ build_index_scankeys_using_slot(Oid hypertable_relid, Relation in_rel, Relation
*/
ScanKeyData *
build_update_delete_scankeys(Relation in_rel, List *heap_filters, int *num_scankeys,
Bitmapset **null_columns)
Bitmapset **null_columns, bool *delete_only)
{
ListCell *lc;
BatchFilter *filter;
Expand All @@ -455,33 +457,41 @@ build_update_delete_scankeys(Relation in_rel, List *heap_filters, int *num_scank
NameStr(filter->column_name),
RelationGetRelationName(in_rel))));

key_index = create_segment_filter_scankey(in_rel,
NameStr(filter->column_name),
filter->strategy,
deduce_filter_subtype(filter, typoid),
scankeys,
key_index,
null_columns,
filter->value ? filter->value->constvalue : 0,
filter->is_null_check,
filter->is_array_op);
bool added = create_segment_filter_scankey(in_rel,
NameStr(filter->column_name),
filter->strategy,
deduce_filter_subtype(filter, typoid),
scankeys,
&key_index,
null_columns,
filter->value ? filter->value->constvalue : 0,
filter->is_null_check,
filter->is_array_op);
/*
* When we plan to DELETE directly on compressed chunks we
* need to ensure all query constraints could be applied
* to the compressed scan and disable direct DELETE when
* we are skipping filters.
*/
if (*delete_only && !added)
*delete_only = false;
}
*num_scankeys = key_index;
return scankeys;
}

static int
static bool
create_segment_filter_scankey(Relation in_rel, char *segment_filter_col_name,
StrategyNumber strategy, Oid subtype, ScanKeyData *scankeys,
int num_scankeys, Bitmapset **null_columns, Datum value,
int *num_scankeys, Bitmapset **null_columns, Datum value,
bool is_null_check, bool is_array_op)
{
AttrNumber cmp_attno = get_attnum(in_rel->rd_id, segment_filter_col_name);
Assert(cmp_attno != InvalidAttrNumber);
/* This should never happen but if it does happen, we can't generate a scan key for
* the filter column so just skip it */
if (cmp_attno == InvalidAttrNumber)
return num_scankeys;
return false;

int flags = is_array_op ? SK_SEARCHARRAY : 0;

Expand All @@ -497,7 +507,7 @@ create_segment_filter_scankey(Relation in_rel, char *segment_filter_col_name,
if (is_null_check)
{
*null_columns = bms_add_member(*null_columns, cmp_attno);
return num_scankeys;
return false;
}

Oid atttypid = in_rel->rd_att->attrs[AttrNumberGetAttrOffset(cmp_attno)].atttypid;
Expand All @@ -520,15 +530,15 @@ create_segment_filter_scankey(Relation in_rel, char *segment_filter_col_name,

/* No operator could be found so we can't create the scankey. */
if (!OidIsValid(opr))
return num_scankeys;
return false;

opr = get_opcode(opr);
Assert(OidIsValid(opr));
/* We should never end up here but: no opcode, no optimization */
if (!OidIsValid(opr))
return num_scankeys;
return false;

ScanKeyEntryInitialize(&scankeys[num_scankeys++],
ScanKeyEntryInitialize(&scankeys[(*num_scankeys)++],
flags,
cmp_attno,
strategy,
Expand All @@ -537,7 +547,7 @@ create_segment_filter_scankey(Relation in_rel, char *segment_filter_col_name,
opr,
value);

return num_scankeys;
return true;
}

/*
Expand Down
Loading

0 comments on commit 253a7cb

Please sign in to comment.