Skip to content

Commit

Permalink
Fix segfault when setting chunk replica identity
Browse files Browse the repository at this point in the history
The segfault happened executing `AlterTableInternal` for setting the
REPLICA IDENTITY during chunk creation (introduced by #5512) that is
dispatched by a trigger invoked by a DDL statement.

Fixed it by using our internal `ts_alter_table_with_event_trigger`
handler function to setup the event trigger context.

Fixes #7406

(cherry picked from commit 5ae1d40)
  • Loading branch information
fabriziomello authored and timescale-automation committed Nov 14, 2024
1 parent 185b83f commit 3232024
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 1 deletion.
2 changes: 2 additions & 0 deletions .unreleased/pr_7434
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixes: #7434 Fixes segfault when internally set the replica identity for a given chunk
Thanks: @bharrisau for reporting the segfault when creating chunks
13 changes: 12 additions & 1 deletion src/chunk.c
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,16 @@ static void
chunk_set_replica_identity(const Chunk *chunk)
{
Relation ht_rel = relation_open(chunk->hypertable_relid, AccessShareLock);
Relation ch_rel = relation_open(chunk->table_id, AccessShareLock);

/* Do nothing if REPLICA IDENTITY of hypertable and chunk are equal */
if (ht_rel->rd_rel->relreplident == ch_rel->rd_rel->relreplident)
{
table_close(ch_rel, NoLock);
table_close(ht_rel, NoLock);
return;
}

ReplicaIdentityStmt stmt = {
.type = T_ReplicaIdentityStmt,
.identity_type = ht_rel->rd_rel->relreplident,
Expand All @@ -922,8 +932,9 @@ chunk_set_replica_identity(const Chunk *chunk)
}

ts_catalog_database_info_become_owner(ts_catalog_database_info_get(), &sec_ctx);
AlterTableInternal(chunk->table_id, list_make1(&cmd), false);
ts_alter_table_with_event_trigger(chunk->table_id, NULL, list_make1(&cmd), false);
ts_catalog_restore_user(&sec_ctx);
table_close(ch_rel, NoLock);
table_close(ht_rel, NoLock);
}

Expand Down
71 changes: 71 additions & 0 deletions test/expected/create_chunks.out
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,74 @@ ERROR: invalid interval: an explicit interval must be specified
SELECT set_chunk_time_interval('chunk_test2', NULL::INTERVAL);
ERROR: invalid interval: an explicit interval must be specified
\set ON_ERROR_STOP 1
-- Issue https://github.com/timescale/timescaledb/issues/7406
CREATE TABLE test_ht (time TIMESTAMPTZ, v1 INTEGER);
SELECT create_hypertable('test_ht', by_range('time', INTERVAL '1 hour'));
NOTICE: adding not-null constraint to column "time"
create_hypertable
-------------------
(4,t)
(1 row)

CREATE TABLE test_tb (time TIMESTAMPTZ, v1 INTEGER);
CREATE OR REPLACE FUNCTION test_tb_trg_insert() RETURNS TRIGGER AS $$
BEGIN
INSERT INTO test_ht VALUES (NEW.time, NEW.v1);
RETURN NEW;
END;
$$ LANGUAGE plpgsql;
CREATE TRIGGER test_tb_trg_insert AFTER
INSERT ON test_tb
FOR EACH ROW EXECUTE FUNCTION test_tb_trg_insert();
-- Creating new chunk inside a trigger called by
-- a DDL statement should not fail.
CREATE TABLE test_output AS
WITH inserted AS (
INSERT INTO test_tb VALUES (NOW(), 1), (NOW(), 2) RETURNING *
)
SELECT * FROM inserted;
-- Check the DEFAULT REPLICA IDENTITY of the chunks
SELECT relname, relreplident FROM show_chunks('test_ht') ch JOIN pg_class c ON (ch = c.oid) ORDER BY relname;
relname | relreplident
-------------------+--------------
_hyper_4_11_chunk | d
(1 row)

-- Clean up
TRUNCATE test_ht, test_tb;
DROP TABLE test_output;
-- Change the DEFAULT REPLICA IDENTITY of the chunks
ALTER TABLE test_ht REPLICA IDENTITY FULL;
-- Internally we force new chunks have the same REPLICA IDENTITY
-- as the parent table.
CREATE TABLE test_output AS
WITH inserted AS (
INSERT INTO test_tb VALUES (NOW(), 1), (NOW(), 2) RETURNING *
)
SELECT * FROM inserted;
-- Check current new REPLICA IDENTITY FULL in the chunks
SELECT relname, relreplident FROM show_chunks('test_ht') ch JOIN pg_class c ON (ch = c.oid) ORDER BY relname;
relname | relreplident
-------------------+--------------
_hyper_4_12_chunk | f
(1 row)

-- All tables should have the same number of rows
SELECT count(*) FROM test_tb;
count
-------
2
(1 row)

SELECT count(*) FROM test_ht;
count
-------
2
(1 row)

SELECT count(*) FROM test_output;
count
-------
2
(1 row)

50 changes: 50 additions & 0 deletions test/sql/create_chunks.sql
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,53 @@ SELECT set_chunk_time_interval('chunk_test', NULL::BIGINT);
SELECT set_chunk_time_interval('chunk_test2', NULL::BIGINT);
SELECT set_chunk_time_interval('chunk_test2', NULL::INTERVAL);
\set ON_ERROR_STOP 1

-- Issue https://github.com/timescale/timescaledb/issues/7406
CREATE TABLE test_ht (time TIMESTAMPTZ, v1 INTEGER);
SELECT create_hypertable('test_ht', by_range('time', INTERVAL '1 hour'));
CREATE TABLE test_tb (time TIMESTAMPTZ, v1 INTEGER);

CREATE OR REPLACE FUNCTION test_tb_trg_insert() RETURNS TRIGGER AS $$
BEGIN
INSERT INTO test_ht VALUES (NEW.time, NEW.v1);
RETURN NEW;
END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER test_tb_trg_insert AFTER
INSERT ON test_tb
FOR EACH ROW EXECUTE FUNCTION test_tb_trg_insert();

-- Creating new chunk inside a trigger called by
-- a DDL statement should not fail.
CREATE TABLE test_output AS
WITH inserted AS (
INSERT INTO test_tb VALUES (NOW(), 1), (NOW(), 2) RETURNING *
)
SELECT * FROM inserted;

-- Check the DEFAULT REPLICA IDENTITY of the chunks
SELECT relname, relreplident FROM show_chunks('test_ht') ch JOIN pg_class c ON (ch = c.oid) ORDER BY relname;

-- Clean up
TRUNCATE test_ht, test_tb;
DROP TABLE test_output;

-- Change the DEFAULT REPLICA IDENTITY of the chunks
ALTER TABLE test_ht REPLICA IDENTITY FULL;

-- Internally we force new chunks have the same REPLICA IDENTITY
-- as the parent table.
CREATE TABLE test_output AS
WITH inserted AS (
INSERT INTO test_tb VALUES (NOW(), 1), (NOW(), 2) RETURNING *
)
SELECT * FROM inserted;

-- Check current new REPLICA IDENTITY FULL in the chunks
SELECT relname, relreplident FROM show_chunks('test_ht') ch JOIN pg_class c ON (ch = c.oid) ORDER BY relname;

-- All tables should have the same number of rows
SELECT count(*) FROM test_tb;
SELECT count(*) FROM test_ht;
SELECT count(*) FROM test_output;

0 comments on commit 3232024

Please sign in to comment.