From 5c62f9935a8f597614ff9b1ebf5858d2c6f1cf6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Mon, 24 Jun 2019 22:31:22 +0200 Subject: [PATCH] Router planner: reject SELECT FOR UPDATE ctes --- .../executor/multi_router_executor.c | 2 +- .../planner/multi_router_planner.c | 7 ++++++ .../expected/isolation_select_for_update.out | 25 +++++++++++++++++++ src/test/regress/pg_regress_multi.pl | 12 ++++----- .../specs/isolation_select_for_update.spec | 7 ++++++ 5 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/backend/distributed/executor/multi_router_executor.c b/src/backend/distributed/executor/multi_router_executor.c index db5078336..dac21c1e9 100644 --- a/src/backend/distributed/executor/multi_router_executor.c +++ b/src/backend/distributed/executor/multi_router_executor.c @@ -272,7 +272,7 @@ AcquireExecutorShardLock(Task *task, CmdType commandType) * must be conflict with each other modify command. By getting ExlcusiveLock * we guarantee that. Note that, getting ExlusiveLock does not mimic the * behaviour of Postgres exactly. Getting row lock with FOR NO KEY UPDATE and - * FOR KEY SHARE do not conflicts in Postgres, yet they block each other in + * FOR KEY SHARE do not conflict in Postgres, yet they block each other in * our implementation. Since FOR SHARE and FOR KEY SHARE does not conflict * with each other but conflicts with modify commands, we get ShareLock for * them. diff --git a/src/backend/distributed/planner/multi_router_planner.c b/src/backend/distributed/planner/multi_router_planner.c index f43f40530..d399045cd 100644 --- a/src/backend/distributed/planner/multi_router_planner.c +++ b/src/backend/distributed/planner/multi_router_planner.c @@ -612,6 +612,13 @@ ModifyQuerySupported(Query *queryTree, Query *originalQuery, bool multiShardQuer NULL, NULL); } + if (cteQuery->hasForUpdate) + { + return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, + "Router planner doesn't support SELECT FOR UPDATE in common table expressions.", + NULL, NULL); + } + cteError = MultiRouterPlannableQuery(cteQuery); if (cteError) { diff --git a/src/test/regress/expected/isolation_select_for_update.out b/src/test/regress/expected/isolation_select_for_update.out index 0587b37ea..d986c2d19 100644 --- a/src/test/regress/expected/isolation_select_for_update.out +++ b/src/test/regress/expected/isolation_select_for_update.out @@ -374,3 +374,28 @@ step s2-finish: restore_isolation_tester_func + +starting permutation: s1-begin s1-update-rt-with-cte-select-from-rt s2-begin s2-update-rt s1-finish s2-finish +step s1-begin: + BEGIN; + +step s1-update-rt-with-cte-select-from-rt: + WITH foo AS (SELECT * FROM ref_table FOR UPDATE) + UPDATE ref_table SET val_1 = 4 FROM foo WHERE ref_table.id = foo.id; + +step s2-begin: + BEGIN; + +step s2-update-rt: + UPDATE ref_table SET val_1 = 5 WHERE id = 1; + +step s1-finish: + COMMIT; + +step s2-update-rt: <... completed> +step s2-finish: + COMMIT; + +restore_isolation_tester_func + + diff --git a/src/test/regress/pg_regress_multi.pl b/src/test/regress/pg_regress_multi.pl index f7e5bac3b..e153909b1 100755 --- a/src/test/regress/pg_regress_multi.pl +++ b/src/test/regress/pg_regress_multi.pl @@ -111,7 +111,7 @@ GetOptions( # # XXX: There's some issues with el capitan's SIP here, causing # DYLD_LIBRARY_PATH not being inherited if SIP is enabled. That's a -# know problem, present in postgres itself as well. +# known problem, present in postgres itself as well. if (defined $libdir) { $ENV{LD_LIBRARY_PATH} = "$libdir:".($ENV{LD_LIBRARY_PATH} || ''); @@ -559,11 +559,11 @@ sub ShutdownServers() or warn "Could not shutdown worker server"; } } - if ($mitmPid != 0) - { - # '-' means signal the process group, 2 is SIGINT - kill(-2, $mitmPid) or warn "could not interrupt mitmdump"; - } + if ($mitmPid != 0) + { + # '-' means signal the process group, 2 is SIGINT + kill(-2, $mitmPid) or warn "could not interrupt mitmdump"; + } $serversAreShutdown = "TRUE"; } } diff --git a/src/test/regress/specs/isolation_select_for_update.spec b/src/test/regress/specs/isolation_select_for_update.spec index 517259cb7..658a6c614 100644 --- a/src/test/regress/specs/isolation_select_for_update.spec +++ b/src/test/regress/specs/isolation_select_for_update.spec @@ -80,6 +80,12 @@ step "s1-select-from-t1-within-cte" SELECT * FROM first_value; } +step "s1-update-rt-with-cte-select-from-rt" +{ + WITH foo AS (SELECT * FROM ref_table FOR UPDATE) + UPDATE ref_table SET val_1 = 4 FROM foo WHERE ref_table.id = foo.id; +} + step "s1-select-from-t1-with-subquery" { SELECT * FROM (SELECT * FROM test_table_1_rf1 FOR UPDATE) foo WHERE id = 1; @@ -157,3 +163,4 @@ permutation "s1-begin" "s1-select-from-t1-within-cte" "s2-begin" "s2-update-t1" permutation "s1-begin" "s1-select-from-t1-with-subquery" "s2-begin" "s2-update-t1" "s1-finish" "s2-finish" permutation "s1-begin" "s1-select-from-rt-with-subquery" "s2-begin" "s2-update-rt" "s1-finish" "s2-finish" permutation "s1-begin" "s1-select-from-t1-with-view" "s2-begin" "s2-update-t1" "s1-finish" "s2-finish" +permutation "s1-begin" "s1-update-rt-with-cte-select-from-rt" "s2-begin" "s2-update-rt" "s1-finish" "s2-finish"