Hide `citus.subquery_pushdown` flag and NOTICE when enabled (#4124)

* Hide citus.subquery_pushdown flag

This flag is dangerous and could likely to let queries
return wrong results.

The flag has a very specific purpose for a very specific
data distribution and query structure. In those cases, when
the flag is set, the user can skip recursive planning altogether
*at their own risk*.

The meaning of the flag is that "I know what I'm doing such that
the query structure/data distribution is on my control, so Citus
can skip many correctness checks".

For regular users, enabling this flag is discouraged. We have to
keep the support only for backward compatibility for some users.

In addition to that, give a NOTICE to discourage new users to
use it.
pull/4073/head^2
Önder Kalacı 2020-08-28 14:53:09 +02:00 committed by GitHub
parent 2459ba6eca
commit 983206c5e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 68 additions and 4 deletions

View File

@ -103,6 +103,7 @@ static void RegisterCitusConfigVariables(void);
static bool ErrorIfNotASuitableDeadlockFactor(double *newval, void **extra,
GucSource source);
static bool WarnIfDeprecatedExecutorUsed(int *newval, void **extra, GucSource source);
static bool NoticeIfSubqueryPushdownEnabled(bool *newval, void **extra, GucSource source);
static bool NodeConninfoGucCheckHook(char **newval, void **extra, GucSource source);
static void NodeConninfoGucAssignHook(const char *newval, void *extra);
static const char * MaxSharedPoolSizeGucShowHook(void);
@ -606,13 +607,21 @@ RegisterCitusConfigVariables(void)
DefineCustomBoolVariable(
"citus.subquery_pushdown",
gettext_noop("Enables supported subquery pushdown to workers."),
NULL,
gettext_noop("Usage of this GUC is highly discouraged, please read the long "
"description"),
gettext_noop("When enabled, the planner skips many correctness checks "
"for subqueries and pushes down the queries to shards as-is. "
"It means that the queries are likely to return wrong results "
"unless the user is absolutely sure that pushing down the "
"subquery is safe. This GUC is maintained only for backward "
"compatibility, no new users are supposed to use it. The planner"
"is capable of pushing down as much computation as possible to the "
"shards depending on the query."),
&SubqueryPushdown,
false,
PGC_USERSET,
GUC_STANDARD,
NULL, NULL, NULL);
GUC_NO_SHOW_ALL,
NoticeIfSubqueryPushdownEnabled, NULL, NULL);
DefineCustomBoolVariable(
"citus.log_multi_join_order",
@ -1525,6 +1534,37 @@ WarnIfDeprecatedExecutorUsed(int *newval, void **extra, GucSource source)
}
/*
* NoticeIfSubqueryPushdownEnabled prints a notice when a user sets
* citus.subquery_pushdown to ON. It doesn't print the notice if the
* value is already true.
*/
static bool
NoticeIfSubqueryPushdownEnabled(bool *newval, void **extra, GucSource source)
{
/* notice only when the value changes */
if (*newval == true && SubqueryPushdown == false)
{
ereport(NOTICE, (errcode(ERRCODE_WARNING_DEPRECATED_FEATURE),
errmsg("Setting citus.subquery_pushdown flag is "
"discouraged becuase it forces the planner "
"to pushdown certain queries, skipping "
"relevant correctness checks."),
errdetail(
"When enabled, the planner skips many correctness checks "
"for subqueries and pushes down the queries to shards as-is. "
"It means that the queries are likely to return wrong results "
"unless the user is absolutely sure that pushing down the "
"subquery is safe. This GUC is maintained only for backward "
"compatibility, no new users are supposed to use it. The planner "
"is capable of pushing down as much computation as possible to the "
"shards depending on the query.")));
}
return true;
}
/*
* NodeConninfoGucCheckHook ensures conninfo settings are in the expected form
* and that the keywords of all non-null settings are on a allowlist devised to

View File

@ -829,6 +829,8 @@ Sort
-- Lateral join subquery pushdown
-- set subquery_pushdown due to limit in the query
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
EXPLAIN (COSTS OFF)
SELECT
tenant_id,

View File

@ -155,6 +155,8 @@ FROM
-- Limit is only supported when subquery_pushdown is set
-- Check that we error out if inner query has limit but outer query has not.
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT
avg(o_totalprice/l_quantity)
FROM
@ -1260,6 +1262,8 @@ ORDER BY
-- Lateral join subquery pushdown
-- set subquery_pushdown since there is limit in the query
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT
tenant_id,
user_id,

View File

@ -872,6 +872,8 @@ GROUP BY
-- most queries below has limit clause
-- therefore setting subquery_pushdown flag for all
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
-- multi-subquery-join
-- The first query has filters on partion column to make it router plannable
-- but it is processed by logical planner since we disabled router execution

View File

@ -1154,6 +1154,8 @@ LIMIT 10;
-- Simple LATERAL JOINs with GROUP BYs in each side
-- need to set subquery_pushdown due to limit for next 2 queries
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT *
FROM
(SELECT "some_users_data".user_id, lastseen
@ -1314,6 +1316,8 @@ ERROR: cannot push down this subquery
DETAIL: Limit in subquery is currently unsupported when a subquery references a column from another query
-- LATERAL JOINs used with INNER JOINs
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT user_id, lastseen
FROM
(SELECT
@ -1936,6 +1940,8 @@ SET citus.enable_repartition_joins to OFF;
RESET client_min_messages;
-- nested LATERAL JOINs
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT *
FROM
(SELECT "some_users_data".user_id, "some_recent_users".value_3
@ -2230,6 +2236,8 @@ LIMIT 10;
-- lateral joins in the nested manner
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT *
FROM
(SELECT

View File

@ -650,6 +650,8 @@ ERROR: cannot pushdown the subquery
DETAIL: There exist a reference table in the outer part of the outer join
-- LATERAL JOINs used with INNER JOINs with reference tables
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT user_id, lastseen
FROM
(SELECT
@ -822,6 +824,8 @@ ORDER BY cnt, value_3 DESC LIMIT 10;
-- nested LATERAL JOINs with reference tables
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
SELECT *
FROM
(SELECT "some_users_data".user_id, "some_recent_users".value_3

View File

@ -720,6 +720,8 @@ SELECT * FROM recent_10_users;
(6 rows)
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
-- still not supported since outer query does not have limit
-- it shows a different (subquery with single relation) error message
SELECT * FROM recent_10_users;
@ -856,6 +858,8 @@ EXPLAIN (COSTS FALSE) SELECT et.* FROM recent_10_users JOIN events_table et USIN
(31 rows)
SET citus.subquery_pushdown to ON;
NOTICE: Setting citus.subquery_pushdown flag is discouraged becuase it forces the planner to pushdown certain queries, skipping relevant correctness checks.
DETAIL: When enabled, the planner skips many correctness checks for subqueries and pushes down the queries to shards as-is. It means that the queries are likely to return wrong results unless the user is absolutely sure that pushing down the subquery is safe. This GUC is maintained only for backward compatibility, no new users are supposed to use it. The planner is capable of pushing down as much computation as possible to the shards depending on the query.
EXPLAIN (COSTS FALSE) SELECT et.* FROM recent_10_users JOIN events_table et USING(user_id) ORDER BY et.time DESC LIMIT 10;
QUERY PLAN
---------------------------------------------------------------------