From fb7f9311f5436c0043f47b85c653fabbd0ce00a2 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Sat, 1 Feb 2025 14:12:45 +0100 Subject: [PATCH] Fix DELETE on compressed chunk with non-btree operators Fixes #7644 --- .unreleased/pr_7645 | 1 + tsl/src/compression/compression_dml.c | 14 +++++++-- tsl/src/compression/compression_dml.h | 2 +- tsl/src/compression/compression_scankey.c | 32 +++++++++++++------- tsl/test/shared/expected/compression_dml.out | 22 ++++++++++++++ tsl/test/shared/sql/compression_dml.sql | 8 +++++ 6 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 .unreleased/pr_7645 diff --git a/.unreleased/pr_7645 b/.unreleased/pr_7645 new file mode 100644 index 00000000000..c7dae561dd8 --- /dev/null +++ b/.unreleased/pr_7645 @@ -0,0 +1 @@ +Fixes: #7645 Fix DELETE on compressed chunk with non-btree operators diff --git a/tsl/src/compression/compression_dml.c b/tsl/src/compression/compression_dml.c index 573039b3ca5..ba160e7b1c7 100644 --- a/tsl/src/compression/compression_dml.c +++ b/tsl/src/compression/compression_dml.c @@ -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) @@ -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; diff --git a/tsl/src/compression/compression_dml.h b/tsl/src/compression/compression_dml.h index 90b77ce73f0..d30ed3f6610 100644 --- a/tsl/src/compression/compression_dml.h +++ b/tsl/src/compression/compression_dml.h @@ -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); diff --git a/tsl/src/compression/compression_scankey.c b/tsl/src/compression/compression_scankey.c index be5e6ef9652..a816a33ffc8 100644 --- a/tsl/src/compression/compression_scankey.c +++ b/tsl/src/compression/compression_scankey.c @@ -435,7 +435,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; @@ -455,16 +455,26 @@ 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); + int new_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); + /* + * 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 && new_index == key_index) + *delete_only = false; + + key_index = new_index; } *num_scankeys = key_index; return scankeys; diff --git a/tsl/test/shared/expected/compression_dml.out b/tsl/test/shared/expected/compression_dml.out index 38cb54edd5a..042d78d0373 100644 --- a/tsl/test/shared/expected/compression_dml.out +++ b/tsl/test/shared/expected/compression_dml.out @@ -480,6 +480,28 @@ SELECT count(*) FROM direct_delete WHERE reading='r2'; 0 (1 row) +ROLLBACK; +-- issue #7644 +-- make sure non-btree operators don't delete unrelated batches +BEGIN; +:ANALYZE DELETE FROM direct_delete WHERE reading<>'r2'; +QUERY PLAN + Custom Scan (HypertableModify) (actual rows=0 loops=1) + Batches decompressed: 6 + Tuples decompressed: 6 + -> Delete on direct_delete (actual rows=0 loops=1) + Delete on _hyper_X_X_chunk direct_delete_1 + -> Seq Scan on _hyper_X_X_chunk direct_delete_1 (actual rows=4 loops=1) + Filter: (reading <> 'r2'::text) + Rows Removed by Filter: 2 +(8 rows) + +-- 2 tuples should still be there +SELECT count(*) FROM direct_delete; + count + 2 +(1 row) + ROLLBACK; -- combining constraints on segmentby columns should work BEGIN; diff --git a/tsl/test/shared/sql/compression_dml.sql b/tsl/test/shared/sql/compression_dml.sql index 098e4400e7d..433e112f3c1 100644 --- a/tsl/test/shared/sql/compression_dml.sql +++ b/tsl/test/shared/sql/compression_dml.sql @@ -225,6 +225,14 @@ BEGIN; SELECT count(*) FROM direct_delete WHERE reading='r2'; ROLLBACK; +-- issue #7644 +-- make sure non-btree operators don't delete unrelated batches +BEGIN; +:ANALYZE DELETE FROM direct_delete WHERE reading<>'r2'; +-- 2 tuples should still be there +SELECT count(*) FROM direct_delete; +ROLLBACK; + -- combining constraints on segmentby columns should work BEGIN; -- should be 1 batches directly deleted