From 34dce14f94ac8f5c6ff346d2fe530d52f6a2d7c1 Mon Sep 17 00:00:00 2001 From: Roman Eskin Date: Fri, 13 Dec 2024 17:34:18 +1000 Subject: [PATCH 1/3] Fix 'gpconfig' behave test --- src/backend/utils/misc/guc_gp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c index 1850364c89e0..a9cdc4d61ea7 100644 --- a/src/backend/utils/misc/guc_gp.c +++ b/src/backend/utils/misc/guc_gp.c @@ -5167,12 +5167,17 @@ check_optimizer(bool *newval, void **extra, GucSource source) */ if (GP_ROLE_DISPATCH != Gp_role) *newval = false; - else if (NULL == planner_hook) + else if (source > PGC_S_ARGV && NULL == planner_hook) { /* * Assume that, if no planner_hook is registered for the * coordinator, we can use only standard Postgres planner. * Thus, forbid setting the GUC. + * But in case GUC source <= PGC_S_ARGV, this function may be called + * before external planner is loaded from a shared lib. For ex., + * user may set the GUC in 'postgresql.conf', so system will try to + * apply the GUC before the init of shared libs. We need to allow + * such cases, assuming that the user knows what he is doing. */ GUC_check_errmsg("External planner is not registered"); return false; From e116305cad879b14ab023d5d3e6481019de7af3d Mon Sep 17 00:00:00 2001 From: Roman Eskin Date: Mon, 16 Dec 2024 10:01:29 +1000 Subject: [PATCH 2/3] Add log message --- src/backend/utils/misc/guc_gp.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c index a9cdc4d61ea7..30d6149ca1e2 100644 --- a/src/backend/utils/misc/guc_gp.c +++ b/src/backend/utils/misc/guc_gp.c @@ -5173,15 +5173,20 @@ check_optimizer(bool *newval, void **extra, GucSource source) * Assume that, if no planner_hook is registered for the * coordinator, we can use only standard Postgres planner. * Thus, forbid setting the GUC. + */ + GUC_check_errmsg("External planner is not registered"); + return false; + } + else if (NULL == planner_hook) + /* * But in case GUC source <= PGC_S_ARGV, this function may be called * before external planner is loaded from a shared lib. For ex., * user may set the GUC in 'postgresql.conf', so system will try to * apply the GUC before the init of shared libs. We need to allow * such cases, assuming that the user knows what he is doing. */ - GUC_check_errmsg("External planner is not registered"); - return false; - } + elog(LOG, "'optimizer' is explicitly set to 'on'. " + "Please ensure that an external planner is added to 'shared_preload_libraries'"); } if (!optimizer_control) From 405d1911e803a9d2950decf471908dc522c88572 Mon Sep 17 00:00:00 2001 From: Roman Eskin Date: Tue, 17 Dec 2024 13:44:37 +1000 Subject: [PATCH 3/3] Reorganize the condition --- src/backend/utils/misc/guc_gp.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c index 30d6149ca1e2..80b583d0785d 100644 --- a/src/backend/utils/misc/guc_gp.c +++ b/src/backend/utils/misc/guc_gp.c @@ -5167,17 +5167,19 @@ check_optimizer(bool *newval, void **extra, GucSource source) */ if (GP_ROLE_DISPATCH != Gp_role) *newval = false; - else if (source > PGC_S_ARGV && NULL == planner_hook) - { - /* - * Assume that, if no planner_hook is registered for the - * coordinator, we can use only standard Postgres planner. - * Thus, forbid setting the GUC. - */ - GUC_check_errmsg("External planner is not registered"); - return false; - } else if (NULL == planner_hook) + { + if (source > PGC_S_ARGV) + { + /* + * Assume that, if no planner_hook is registered for the + * coordinator, we can use only standard Postgres planner. + * Thus, forbid setting the GUC. + */ + GUC_check_errmsg("External planner is not registered"); + return false; + } + /* * But in case GUC source <= PGC_S_ARGV, this function may be called * before external planner is loaded from a shared lib. For ex., @@ -5187,6 +5189,7 @@ check_optimizer(bool *newval, void **extra, GucSource source) */ elog(LOG, "'optimizer' is explicitly set to 'on'. " "Please ensure that an external planner is added to 'shared_preload_libraries'"); + } } if (!optimizer_control)