diff --git a/src/chunk_dispatch_plan.c b/src/chunk_dispatch_plan.c index 20ef21550..1505f18ac 100644 --- a/src/chunk_dispatch_plan.c +++ b/src/chunk_dispatch_plan.c @@ -72,12 +72,9 @@ chunk_dispatch_plan_create(PlannerInfo *root, RelOptInfo *relopt, CustomPath *be cscan->custom_plans = custom_plans; cscan->scan.scanrelid = 0; /* Indicate this is not a real relation we are * scanning */ - cscan->scan.plan.targetlist = tlist; - - /* - * We need to set a custom_scan_tlist for EXPLAIN (verbose). - */ + /* The "input" and "output" target lists should be the same */ cscan->custom_scan_tlist = tlist; + cscan->scan.plan.targetlist = tlist; return &cscan->scan.plan; } diff --git a/src/hypertable_insert.c b/src/hypertable_insert.c index 65dde9baf..7b8f70b9b 100644 --- a/src/hypertable_insert.c +++ b/src/hypertable_insert.c @@ -39,7 +39,6 @@ hypertable_insert_begin(CustomScanState *node, EState *estate, int eflags) PlanState *ps; ps = ExecInitNode(&state->mt->plan, estate, eflags); - node->custom_ps = list_make1(ps); if (IsA(ps, ModifyTableState)) @@ -98,16 +97,11 @@ static Node * hypertable_insert_state_create(CustomScan *cscan) { HypertableInsertState *state; + ModifyTable *mt = castNode(ModifyTable, linitial(cscan->custom_plans)); state = (HypertableInsertState *) newNode(sizeof(HypertableInsertState), T_CustomScanState); state->cscan_state.methods = &hypertable_insert_state_methods; - state->mt = (ModifyTable *) linitial(cscan->custom_plans); - - /* - * Restore ModifyTable arbiterIndexes to the original value - * this is necessary in case this plan gets executed multiple - * times in a prepared statement. - */ + state->mt = mt; state->mt->arbiterIndexes = linitial(cscan->custom_private); return (Node *) state; @@ -119,10 +113,45 @@ static CustomScanMethods hypertable_insert_plan_methods = { }; /* - * This copies the target list on the ModifyTable plan node to our wrapping - * HypertableInsert plan node after set_plan_references() has run. This ensures - * that the top-level target list reflects the projection done in a RETURNING - * statement. + * Make a targetlist to meet CustomScan expectations. + * + * When a CustomScan isn't scanning a real relation (scanrelid=0), it will build + * a virtual TupleDesc for the scan "input" based on custom_scan_tlist. The + * "output" targetlist is then expected to reference the attributes of the + * input's TupleDesc. Without projection, the targetlist will be only Vars with + * varno set to INDEX_VAR (to indicate reference to the TupleDesc instead of a + * real relation) and matching the order of the attributes in the TupleDesc. + * + * Any other order, or non-Vars, will lead to the CustomScan performing + * projection. + * + * Since the CustomScan for hypertable insert just wraps ModifyTable, no + * projection is needed, so we'll build a targetlist to avoid this. + */ +static List * +make_var_targetlist(const List *tlist) +{ + List *new_tlist = NIL; + ListCell *lc; + int resno = 1; + + foreach (lc, tlist) + { + TargetEntry *tle = lfirst_node(TargetEntry, lc); + Var *var = makeVarFromTargetEntry(INDEX_VAR, tle); + + var->varattno = resno; + new_tlist = lappend(new_tlist, makeTargetEntry(&var->xpr, resno, tle->resname, false)); + resno++; + } + + return new_tlist; +} + +/* + * Construct the HypertableInsert's target list based on the ModifyTable's + * target list, which now exists after having been created by + * set_plan_references(). */ void ts_hypertable_insert_fixup_tlist(Plan *plan) @@ -133,16 +162,23 @@ ts_hypertable_insert_fixup_tlist(Plan *plan) if (cscan->methods == &hypertable_insert_plan_methods) { - ModifyTable *mt = linitial(cscan->custom_plans); + ModifyTable *mt = linitial_node(ModifyTable, cscan->custom_plans); - Assert(IsA(mt, ModifyTable)); + if (mt->plan.targetlist == NIL) + { + cscan->custom_scan_tlist = NIL; + cscan->scan.plan.targetlist = NIL; + } + else + { + /* The input is the output of the child ModifyTable node */ + cscan->custom_scan_tlist = mt->plan.targetlist; - cscan->scan.plan.targetlist = copyObject(mt->plan.targetlist); - cscan->custom_scan_tlist = NIL; + /* The output is a direct mapping of the input */ + cscan->scan.plan.targetlist = make_var_targetlist(mt->plan.targetlist); + } } } - - return; } static Plan * @@ -164,19 +200,44 @@ hypertable_insert_plan_create(PlannerInfo *root, RelOptInfo *rel, CustomPath *be cscan->scan.plan.plan_rows = mt->plan.plan_rows; cscan->scan.plan.plan_width = mt->plan.plan_width; - /* - * Since this is the top-level plan (above ModifyTable) we need to use the - * same targetlist as ModifyTable. However, that targetlist is not set at - * this point as it is created by setrefs.c at the end of the planning. It - * accounts for things like returning lists that might order attributes in - * a way that does not match the order in the base relation. To get around - * this we use a temporary target list here and later fix it up after the - * standard planner has run. + /* The tlist is always NIL since the ModifyTable subplan doesn't have its + * targetlist set until set_plan_references (setrefs.c) is run */ + Assert(tlist == NIL); + + /* Target list handling here needs special attention. Intuitively, we'd like + * to adopt the target list of the ModifyTable subplan we wrap without + * further projection. For a CustomScan this means setting the "input" + * custom_scan_tlist to the ModifyTable's target list and having an "output" + * targetlist that references the TupleDesc that is created from the + * custom_scan_tlist at execution time. Now, while this seems + * straight-forward, there are several things with how ModifyTable nodes are + * handled in the planner that complicates this: + * + * - First, ModifyTable doesn't have a targetlist set at this point, and + * it is only set later in set_plan_references (setrefs.c) if there's a + * RETURNING clause. + * + * - Second, top-level plan nodes, except for ModifyTable nodes, need to + * have a targetlist matching root->processed_tlist. This is asserted in + * apply_tlist_labeling, which is called in create_plan (createplan.c) + * immediately after this function returns. ModifyTable is exempted + * because it doesn't always have a targetlist that matches + * processed_tlist. So, even if we had access to ModifyTable's + * targetlist here we wouldn't be able to use it since we're a + * CustomScan and thus not exempted. + * + * - Third, a CustomScan's targetlist should reference the attributes of the + * TupleDesc that gets created from the custom_scan_tlist at the start of + * execution. This means we need to make the targetlist into all Vars with + * attribute numbers that correspond to the TupleDesc instead of result + * relation in the ModifyTable. + * + * To get around these issues, we set the targetlist here to + * root->processed_tlist, and at the end of planning when the ModifyTable's + * targetlist is set, we go back and fix up the CustomScan's targetlist. */ cscan->scan.plan.targetlist = copyObject(root->processed_tlist); - - /* Set the custom scan target list for, e.g., explains */ - cscan->custom_scan_tlist = copyObject(cscan->scan.plan.targetlist); + cscan->custom_scan_tlist = cscan->scan.plan.targetlist; /* * we save the original list of arbiter indexes here diff --git a/tsl/test/expected/jit-11.out b/tsl/test/expected/jit-11.out index efb1aefb5..d38df5613 100644 --- a/tsl/test/expected/jit-11.out +++ b/tsl/test/expected/jit-11.out @@ -11,31 +11,23 @@ SELECT format('include/%s_load.sql', :'TEST_BASE_NAME') as "TEST_LOAD_NAME", SELECT format('\! diff -u --label "Unoptimized results" --label "Optimized results" %s %s', :'TEST_RESULTS_UNOPTIMIZED', :'TEST_RESULTS_OPTIMIZED') as "DIFF_CMD" \gset -- enable all jit optimizations --- --- disable jit_tuple_deforming for PG11 as a temporary solution --- until it fixed --- SET jit=on; SET jit_above_cost=0; SET jit_inline_above_cost=0; SET jit_optimize_above_cost=0; -SELECT - CASE WHEN current_setting('server_version_num')::int < 120000 - THEN 0 ELSE 1 - END AS "USE_JIT_DEFORMING" -\gset -SET jit_tuple_deforming=:USE_JIT_DEFORMING; +SET jit_tuple_deforming=on; \ir :TEST_LOAD_NAME -- This file and its contents are licensed under the Timescale License. -- Please see the included NOTICE for copyright information and -- LICENSE-TIMESCALE for a copy of the license. -CREATE TABLE jit_test(time timestamp NOT NULL, temp float); +CREATE TABLE jit_test(time timestamp NOT NULL, device int, temp float); SELECT create_hypertable('jit_test', 'time'); create_hypertable ----------------------- (1,public,jit_test,t) (1 row) +ALTER TABLE jit_test DROP COLUMN device; CREATE TABLE jit_test_interval(id int NOT NULL, temp float); SELECT create_hypertable('jit_test_interval', 'id', chunk_time_interval => 10); create_hypertable @@ -66,7 +58,7 @@ SELECT FROM jit_test_contagg GROUP BY bucket, device_id; -psql:include/jit_load.sql:28: NOTICE: adding index _materialized_hypertable_4_device_id_bucket_idx ON _timescaledb_internal._materialized_hypertable_4 USING BTREE(device_id, bucket) +psql:include/jit_load.sql:29: NOTICE: adding index _materialized_hypertable_4_device_id_bucket_idx ON _timescaledb_internal._materialized_hypertable_4 USING BTREE(device_id, bucket) INSERT INTO jit_test_contagg SELECT ts, 'device_1', (EXTRACT(EPOCH FROM ts)) from generate_series('2018-12-01 00:00'::timestamp, '2018-12-31 00:00'::timestamp, '30 minutes') ts; INSERT INTO jit_test_contagg @@ -83,30 +75,30 @@ REFRESH MATERIALIZED VIEW jit_device_summary; -- :PREFIX INSERT INTO jit_test VALUES('2017-01-20T09:00:01', 22.5) RETURNING *; - QUERY PLAN ---------------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------ Custom Scan (HypertableInsert) Output: "time", temp -> Insert on public.jit_test Output: "time", temp -> Custom Scan (ChunkDispatch) - Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, '22.5'::double precision + Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, NULL::integer, '22.5'::double precision -> Result - Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, '22.5'::double precision + Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, NULL::integer, '22.5'::double precision (8 rows) :PREFIX INSERT INTO jit_test VALUES ('2017-01-20T09:00:02', 2), ('2017-01-20T09:00:03', 5), ('2017-01-20T09:00:04', 10); - QUERY PLAN --------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------------------------- Custom Scan (HypertableInsert) -> Insert on public.jit_test -> Custom Scan (ChunkDispatch) - Output: "*VALUES*".column1, "*VALUES*".column2 + Output: "*VALUES*".column1, NULL::integer, "*VALUES*".column2 -> Values Scan on "*VALUES*" - Output: "*VALUES*".column1, "*VALUES*".column2 + Output: "*VALUES*".column1, NULL::integer, "*VALUES*".column2 (6 rows) :PREFIX diff --git a/tsl/test/expected/jit-12.out b/tsl/test/expected/jit-12.out index 00e960eff..092c0c5ac 100644 --- a/tsl/test/expected/jit-12.out +++ b/tsl/test/expected/jit-12.out @@ -11,31 +11,23 @@ SELECT format('include/%s_load.sql', :'TEST_BASE_NAME') as "TEST_LOAD_NAME", SELECT format('\! diff -u --label "Unoptimized results" --label "Optimized results" %s %s', :'TEST_RESULTS_UNOPTIMIZED', :'TEST_RESULTS_OPTIMIZED') as "DIFF_CMD" \gset -- enable all jit optimizations --- --- disable jit_tuple_deforming for PG11 as a temporary solution --- until it fixed --- SET jit=on; SET jit_above_cost=0; SET jit_inline_above_cost=0; SET jit_optimize_above_cost=0; -SELECT - CASE WHEN current_setting('server_version_num')::int < 120000 - THEN 0 ELSE 1 - END AS "USE_JIT_DEFORMING" -\gset -SET jit_tuple_deforming=:USE_JIT_DEFORMING; +SET jit_tuple_deforming=on; \ir :TEST_LOAD_NAME -- This file and its contents are licensed under the Timescale License. -- Please see the included NOTICE for copyright information and -- LICENSE-TIMESCALE for a copy of the license. -CREATE TABLE jit_test(time timestamp NOT NULL, temp float); +CREATE TABLE jit_test(time timestamp NOT NULL, device int, temp float); SELECT create_hypertable('jit_test', 'time'); create_hypertable ----------------------- (1,public,jit_test,t) (1 row) +ALTER TABLE jit_test DROP COLUMN device; CREATE TABLE jit_test_interval(id int NOT NULL, temp float); SELECT create_hypertable('jit_test_interval', 'id', chunk_time_interval => 10); create_hypertable @@ -66,7 +58,7 @@ SELECT FROM jit_test_contagg GROUP BY bucket, device_id; -psql:include/jit_load.sql:28: NOTICE: adding index _materialized_hypertable_4_device_id_bucket_idx ON _timescaledb_internal._materialized_hypertable_4 USING BTREE(device_id, bucket) +psql:include/jit_load.sql:29: NOTICE: adding index _materialized_hypertable_4_device_id_bucket_idx ON _timescaledb_internal._materialized_hypertable_4 USING BTREE(device_id, bucket) INSERT INTO jit_test_contagg SELECT ts, 'device_1', (EXTRACT(EPOCH FROM ts)) from generate_series('2018-12-01 00:00'::timestamp, '2018-12-31 00:00'::timestamp, '30 minutes') ts; INSERT INTO jit_test_contagg @@ -83,30 +75,30 @@ REFRESH MATERIALIZED VIEW jit_device_summary; -- :PREFIX INSERT INTO jit_test VALUES('2017-01-20T09:00:01', 22.5) RETURNING *; - QUERY PLAN ---------------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------ Custom Scan (HypertableInsert) Output: jit_test."time", jit_test.temp -> Insert on public.jit_test Output: jit_test."time", jit_test.temp -> Custom Scan (ChunkDispatch) - Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, '22.5'::double precision + Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, NULL::integer, '22.5'::double precision -> Result - Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, '22.5'::double precision + Output: 'Fri Jan 20 09:00:01 2017'::timestamp without time zone, NULL::integer, '22.5'::double precision (8 rows) :PREFIX INSERT INTO jit_test VALUES ('2017-01-20T09:00:02', 2), ('2017-01-20T09:00:03', 5), ('2017-01-20T09:00:04', 10); - QUERY PLAN --------------------------------------------------------------------- + QUERY PLAN +----------------------------------------------------------------------------------- Custom Scan (HypertableInsert) -> Insert on public.jit_test -> Custom Scan (ChunkDispatch) - Output: "*VALUES*".column1, "*VALUES*".column2 + Output: "*VALUES*".column1, NULL::integer, "*VALUES*".column2 -> Values Scan on "*VALUES*" - Output: "*VALUES*".column1, "*VALUES*".column2 + Output: "*VALUES*".column1, NULL::integer, "*VALUES*".column2 (6 rows) :PREFIX diff --git a/tsl/test/sql/include/jit_load.sql b/tsl/test/sql/include/jit_load.sql index b966f3bec..3cc1a56ff 100644 --- a/tsl/test/sql/include/jit_load.sql +++ b/tsl/test/sql/include/jit_load.sql @@ -1,8 +1,9 @@ -- This file and its contents are licensed under the Timescale License. -- Please see the included NOTICE for copyright information and -- LICENSE-TIMESCALE for a copy of the license. -CREATE TABLE jit_test(time timestamp NOT NULL, temp float); +CREATE TABLE jit_test(time timestamp NOT NULL, device int, temp float); SELECT create_hypertable('jit_test', 'time'); +ALTER TABLE jit_test DROP COLUMN device; CREATE TABLE jit_test_interval(id int NOT NULL, temp float); SELECT create_hypertable('jit_test_interval', 'id', chunk_time_interval => 10); diff --git a/tsl/test/sql/jit.sql.in b/tsl/test/sql/jit.sql.in index 6e87fae58..13d9d6a87 100644 --- a/tsl/test/sql/jit.sql.in +++ b/tsl/test/sql/jit.sql.in @@ -13,20 +13,11 @@ SELECT format('\! diff -u --label "Unoptimized results" --label "Optimized resul \gset -- enable all jit optimizations --- --- disable jit_tuple_deforming for PG11 as a temporary solution --- until it fixed --- SET jit=on; SET jit_above_cost=0; SET jit_inline_above_cost=0; SET jit_optimize_above_cost=0; -SELECT - CASE WHEN current_setting('server_version_num')::int < 120000 - THEN 0 ELSE 1 - END AS "USE_JIT_DEFORMING" -\gset -SET jit_tuple_deforming=:USE_JIT_DEFORMING; +SET jit_tuple_deforming=on; \ir :TEST_LOAD_NAME \set PREFIX 'EXPLAIN (VERBOSE, TIMING OFF, COSTS OFF, SUMMARY OFF)'