From d0c92dd433175b4a4f5209752e52dfc20a2f4805 Mon Sep 17 00:00:00 2001 From: Dmitry Simonenko Date: Fri, 15 May 2020 12:08:56 +0300 Subject: [PATCH] Fix crash on SELECT WHERE NOT with empty table Modify table state is not created with an empty tables, which lead to NULL pointer evaluation. Starting from PG12 the planner injects a gating plan node above any node that has pseusdo-constant quals. To fix this, we need to check for such a gating node and handle the case. We could optionally prune the extra node, since there's already such a node below ChunkDispatch. Fixes #1883. --- src/hypertable_insert.c | 59 ++++++++++++++++++++++++------------ test/expected/insert-10.out | 50 ++++++++++++++++++++++++++++++ test/expected/insert-11.out | 50 ++++++++++++++++++++++++++++++ test/expected/insert-12.out | 51 +++++++++++++++++++++++++++++++ test/expected/insert-9.6.out | 50 ++++++++++++++++++++++++++++++ test/sql/insert.sql.in | 33 +++++++++++++++++++- 6 files changed, 273 insertions(+), 20 deletions(-) diff --git a/src/hypertable_insert.c b/src/hypertable_insert.c index 7b8f70b9b..94a3334d5 100644 --- a/src/hypertable_insert.c +++ b/src/hypertable_insert.c @@ -21,6 +21,29 @@ #include "chunk_dispatch_plan.h" #include "hypertable_cache.h" +static PlanState * +get_chunk_dispatch_state(ModifyTableState *mtstate, PlanState *substate) +{ + switch (nodeTag(substate)) + { + case T_CustomScanState: + { + CustomScanState *csstate = castNode(CustomScanState, substate); + + if (strcmp(csstate->methods->CustomName, CHUNK_DISPATCH_STATE_NAME) == 0) + return substate; + + break; + } + case T_ResultState: + return get_chunk_dispatch_state(mtstate, castNode(ResultState, substate)->ps.lefttree); + default: + break; + } + + return NULL; +} + /* * HypertableInsert (with corresponding executor node) is a plan node that * implements INSERTs for hypertables. It is mostly a wrapper around the @@ -36,35 +59,33 @@ static void hypertable_insert_begin(CustomScanState *node, EState *estate, int eflags) { HypertableInsertState *state = (HypertableInsertState *) node; + ModifyTableState *mtstate; PlanState *ps; + bool PG_USED_FOR_ASSERTS_ONLY found_cdstate = false; + int i; ps = ExecInitNode(&state->mt->plan, estate, eflags); node->custom_ps = list_make1(ps); + mtstate = castNode(ModifyTableState, ps); - if (IsA(ps, ModifyTableState)) + /* + * Find all ChunkDispatchState subnodes and set their parent + * ModifyTableState node + */ + for (i = 0; i < mtstate->mt_nplans; i++) { - ModifyTableState *mtstate = (ModifyTableState *) ps; - int i; + ChunkDispatchState *cdstate = + (ChunkDispatchState *) get_chunk_dispatch_state(mtstate, mtstate->mt_plans[i]); - /* - * Find all ChunkDispatchState subnodes and set their parent - * ModifyTableState node - */ - for (i = 0; i < mtstate->mt_nplans; i++) + if (cdstate) { - if (IsA(mtstate->mt_plans[i], CustomScanState)) - { - CustomScanState *csstate = (CustomScanState *) mtstate->mt_plans[i]; - - if (strcmp(csstate->methods->CustomName, CHUNK_DISPATCH_STATE_NAME) == 0) - { - ChunkDispatchState *cdstate = (ChunkDispatchState *) mtstate->mt_plans[i]; - - ts_chunk_dispatch_state_set_parent(cdstate, mtstate); - } - } + found_cdstate = true; + ts_chunk_dispatch_state_set_parent(cdstate, mtstate); } } + + /* Ensure that we found at least one ChunkDispatchState node */ + Assert(found_cdstate); } static TupleTableSlot * diff --git a/test/expected/insert-10.out b/test/expected/insert-10.out index 747c46896..109cd5964 100644 --- a/test/expected/insert-10.out +++ b/test/expected/insert-10.out @@ -586,3 +586,53 @@ SELECT insert_test(), insert_test(), insert_test(); | | (1 row) +-- test Postgres crashes on INSERT ... SELECT ... WHERE NOT EXISTS with empty table +-- https://github.com/timescale/timescaledb/issues/1883 +CREATE TABLE readings ( + toe TIMESTAMPTZ NOT NULL, + sensor_id INT NOT NULL, + value INT NOT NULL +); +SELECT create_hypertable( + 'readings', + 'toe', + chunk_time_interval => interval '1 day', + if_not_exists => TRUE, + migrate_data => TRUE +); + create_hypertable +------------------------ + (11,public,readings,t) +(1 row) + +EXPLAIN (costs off) +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); + QUERY PLAN +---------------------------------------------------------- + Custom Scan (HypertableInsert) + -> Insert on readings + -> Custom Scan (ChunkDispatch) + -> Subquery Scan on "*SELECT*" + -> Result + One-Time Filter: (NOT $0) + InitPlan 1 (returns $0) + -> Result + One-Time Filter: false +(9 rows) + +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); +DROP TABLE readings; diff --git a/test/expected/insert-11.out b/test/expected/insert-11.out index 747c46896..109cd5964 100644 --- a/test/expected/insert-11.out +++ b/test/expected/insert-11.out @@ -586,3 +586,53 @@ SELECT insert_test(), insert_test(), insert_test(); | | (1 row) +-- test Postgres crashes on INSERT ... SELECT ... WHERE NOT EXISTS with empty table +-- https://github.com/timescale/timescaledb/issues/1883 +CREATE TABLE readings ( + toe TIMESTAMPTZ NOT NULL, + sensor_id INT NOT NULL, + value INT NOT NULL +); +SELECT create_hypertable( + 'readings', + 'toe', + chunk_time_interval => interval '1 day', + if_not_exists => TRUE, + migrate_data => TRUE +); + create_hypertable +------------------------ + (11,public,readings,t) +(1 row) + +EXPLAIN (costs off) +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); + QUERY PLAN +---------------------------------------------------------- + Custom Scan (HypertableInsert) + -> Insert on readings + -> Custom Scan (ChunkDispatch) + -> Subquery Scan on "*SELECT*" + -> Result + One-Time Filter: (NOT $0) + InitPlan 1 (returns $0) + -> Result + One-Time Filter: false +(9 rows) + +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); +DROP TABLE readings; diff --git a/test/expected/insert-12.out b/test/expected/insert-12.out index e701df1f4..47e4b9acb 100644 --- a/test/expected/insert-12.out +++ b/test/expected/insert-12.out @@ -585,3 +585,54 @@ SELECT insert_test(), insert_test(), insert_test(); | | (1 row) +-- test Postgres crashes on INSERT ... SELECT ... WHERE NOT EXISTS with empty table +-- https://github.com/timescale/timescaledb/issues/1883 +CREATE TABLE readings ( + toe TIMESTAMPTZ NOT NULL, + sensor_id INT NOT NULL, + value INT NOT NULL +); +SELECT create_hypertable( + 'readings', + 'toe', + chunk_time_interval => interval '1 day', + if_not_exists => TRUE, + migrate_data => TRUE +); + create_hypertable +------------------------ + (11,public,readings,t) +(1 row) + +EXPLAIN (costs off) +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); + QUERY PLAN +----------------------------------------------------- + Custom Scan (HypertableInsert) + InitPlan 1 (returns $0) + -> Result + One-Time Filter: false + -> Insert on readings + -> Result + One-Time Filter: (NOT $0) + -> Custom Scan (ChunkDispatch) + -> Result + One-Time Filter: (NOT $0) +(10 rows) + +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); +DROP TABLE readings; diff --git a/test/expected/insert-9.6.out b/test/expected/insert-9.6.out index 747c46896..109cd5964 100644 --- a/test/expected/insert-9.6.out +++ b/test/expected/insert-9.6.out @@ -586,3 +586,53 @@ SELECT insert_test(), insert_test(), insert_test(); | | (1 row) +-- test Postgres crashes on INSERT ... SELECT ... WHERE NOT EXISTS with empty table +-- https://github.com/timescale/timescaledb/issues/1883 +CREATE TABLE readings ( + toe TIMESTAMPTZ NOT NULL, + sensor_id INT NOT NULL, + value INT NOT NULL +); +SELECT create_hypertable( + 'readings', + 'toe', + chunk_time_interval => interval '1 day', + if_not_exists => TRUE, + migrate_data => TRUE +); + create_hypertable +------------------------ + (11,public,readings,t) +(1 row) + +EXPLAIN (costs off) +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); + QUERY PLAN +---------------------------------------------------------- + Custom Scan (HypertableInsert) + -> Insert on readings + -> Custom Scan (ChunkDispatch) + -> Subquery Scan on "*SELECT*" + -> Result + One-Time Filter: (NOT $0) + InitPlan 1 (returns $0) + -> Result + One-Time Filter: false +(9 rows) + +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); +DROP TABLE readings; diff --git a/test/sql/insert.sql.in b/test/sql/insert.sql.in index fa8825aad..a29bf53b4 100644 --- a/test/sql/insert.sql.in +++ b/test/sql/insert.sql.in @@ -148,4 +148,35 @@ $$; SELECT insert_test(), insert_test(), insert_test(); - +-- test Postgres crashes on INSERT ... SELECT ... WHERE NOT EXISTS with empty table +-- https://github.com/timescale/timescaledb/issues/1883 +CREATE TABLE readings ( + toe TIMESTAMPTZ NOT NULL, + sensor_id INT NOT NULL, + value INT NOT NULL +); +SELECT create_hypertable( + 'readings', + 'toe', + chunk_time_interval => interval '1 day', + if_not_exists => TRUE, + migrate_data => TRUE +); +EXPLAIN (costs off) +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); +INSERT INTO readings +SELECT '2020-05-09 10:34:35.296288+00', 1, 0 +WHERE NOT EXISTS ( + SELECT 1 + FROM readings + WHERE sensor_id = 1 + AND toe = '2020-05-09 10:34:35.296288+00' +); +DROP TABLE readings;