From 5fe0845d7e397cd1a849a0bb8591c0f5e530c11e Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Tue, 12 Sep 2017 11:38:29 -0700 Subject: [PATCH 1/2] Always copy MultiPlan in GetMultiPlan --- src/backend/distributed/planner/multi_planner.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/planner/multi_planner.c b/src/backend/distributed/planner/multi_planner.c index 0e0b27c48..d020b479b 100644 --- a/src/backend/distributed/planner/multi_planner.c +++ b/src/backend/distributed/planner/multi_planner.c @@ -467,7 +467,14 @@ GetMultiPlan(CustomScan *customScan) node = CheckNodeCopyAndSerialization(node); - multiPlan = (MultiPlan *) node; + /* + * When using prepared statements the same plan gets reused across + * multiple statements and transactions. We make several modifications + * to the MultiPlan during execution such as assigning task placements + * and evaluating functions and parameters. These changes should not + * persist, so we always work on a copy. + */ + multiPlan = (MultiPlan *) copyObject(node); return multiPlan; } From 27da0a29d7eaa5adead8dc94699b8746c977bfdf Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Tue, 12 Sep 2017 13:09:31 -0700 Subject: [PATCH 2/2] Add volatile function in prepared statement regression test --- .../regress/expected/multi_prepare_sql.out | 36 +++++++++++++++++++ src/test/regress/sql/multi_prepare_sql.sql | 31 ++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/test/regress/expected/multi_prepare_sql.out b/src/test/regress/expected/multi_prepare_sql.out index 4da5dfa45..8ff59be54 100644 --- a/src/test/regress/expected/multi_prepare_sql.out +++ b/src/test/regress/expected/multi_prepare_sql.out @@ -998,6 +998,42 @@ SELECT key, value FROM domain_partition_column_table ORDER BY key; (6 rows) DROP TABLE domain_partition_column_table; +-- verify we re-evaluate volatile functions every time +CREATE TABLE http_request ( + site_id INT, + ingest_time TIMESTAMPTZ DEFAULT now(), + url TEXT, + request_country TEXT, + ip_address TEXT, + status_code INT, + response_time_msec INT +); +SELECT create_distributed_table('http_request', 'site_id'); + create_distributed_table +-------------------------- + +(1 row) + +PREPARE FOO AS INSERT INTO http_request ( + site_id, ingest_time, url, request_country, + ip_address, status_code, response_time_msec +) VALUES ( + 1, clock_timestamp(), 'http://example.com/path', 'USA', + inet '88.250.10.123', 200, 10 +); +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; +SELECT count(distinct ingest_time) FROM http_request WHERE site_id = 1; + count +------- + 6 +(1 row) + +DROP TABLE http_request; -- verify placement state updates invalidate shard state -- -- We use a immutable function to check for that. The planner will diff --git a/src/test/regress/sql/multi_prepare_sql.sql b/src/test/regress/sql/multi_prepare_sql.sql index e67d783a9..45bc8e2a1 100644 --- a/src/test/regress/sql/multi_prepare_sql.sql +++ b/src/test/regress/sql/multi_prepare_sql.sql @@ -533,6 +533,37 @@ SELECT key, value FROM domain_partition_column_table ORDER BY key; DROP TABLE domain_partition_column_table; +-- verify we re-evaluate volatile functions every time +CREATE TABLE http_request ( + site_id INT, + ingest_time TIMESTAMPTZ DEFAULT now(), + url TEXT, + request_country TEXT, + ip_address TEXT, + status_code INT, + response_time_msec INT +); + +SELECT create_distributed_table('http_request', 'site_id'); + +PREPARE FOO AS INSERT INTO http_request ( + site_id, ingest_time, url, request_country, + ip_address, status_code, response_time_msec +) VALUES ( + 1, clock_timestamp(), 'http://example.com/path', 'USA', + inet '88.250.10.123', 200, 10 +); +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; +EXECUTE foo; + +SELECT count(distinct ingest_time) FROM http_request WHERE site_id = 1; + +DROP TABLE http_request; + -- verify placement state updates invalidate shard state -- -- We use a immutable function to check for that. The planner will