From 08e2820c6762feffebb4559c0b73ab14811f646f Mon Sep 17 00:00:00 2001 From: aykut-bozkurt <51649454+aykut-bozkurt@users.noreply.github.com> Date: Mon, 17 Apr 2023 18:14:01 +0300 Subject: [PATCH] skip restriction clause if it contains placeholdervar (#6857) `PlaceHolderVar` is not relevant to be processed inside a restriction clause. Otherwise, `pull_var_clause_default` would throw error. PG would create the restriction to physical `Var` that `PlaceHolderVar` points to anyway, so it is safe to skip this restriction. DESCRIPTION: Fixes a bug related to WHERE clause list which contains placeholder. Fixes https://github.com/citusdata/citus/issues/6758 --- .../relation_restriction_equivalence.c | 22 ++++++ src/test/regress/expected/issue_6758.out | 69 +++++++++++++++++++ src/test/regress/multi_schedule | 2 +- src/test/regress/sql/issue_6758.sql | 55 +++++++++++++++ 4 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 src/test/regress/expected/issue_6758.out create mode 100644 src/test/regress/sql/issue_6758.sql diff --git a/src/backend/distributed/planner/relation_restriction_equivalence.c b/src/backend/distributed/planner/relation_restriction_equivalence.c index d39c4affb..ac36842de 100644 --- a/src/backend/distributed/planner/relation_restriction_equivalence.c +++ b/src/backend/distributed/planner/relation_restriction_equivalence.c @@ -155,6 +155,7 @@ static bool AllDistributedRelationsInRestrictionContextColocated( RelationRestrictionContext * restrictionContext); static bool IsNotSafeRestrictionToRecursivelyPlan(Node *node); +static bool HasPlaceHolderVar(Node *node); static JoinRestrictionContext * FilterJoinRestrictionContext( JoinRestrictionContext *joinRestrictionContext, Relids queryRteIdentities); @@ -2149,6 +2150,17 @@ GetRestrictInfoListForRelation(RangeTblEntry *rangeTblEntry, continue; } + /* + * PlaceHolderVar is not relevant to be processed inside a restriction clause. + * Otherwise, pull_var_clause_default would throw error. PG would create + * the restriction to physical Var that PlaceHolderVar points anyway, so it is + * safe to skip this restriction. + */ + if (FindNodeMatchingCheckFunction((Node *) restrictionClause, HasPlaceHolderVar)) + { + continue; + } + /* * We're going to add this restriction expression to a subquery * which consists of only one relation in its jointree. Thus, @@ -2214,6 +2226,16 @@ IsNotSafeRestrictionToRecursivelyPlan(Node *node) } +/* + * HasPlaceHolderVar returns true if given node contains any PlaceHolderVar. + */ +static bool +HasPlaceHolderVar(Node *node) +{ + return IsA(node, PlaceHolderVar); +} + + /* * FilterRelationRestrictionContext gets a relation restriction context and * set of rte identities. It returns the relation restrictions that that appear diff --git a/src/test/regress/expected/issue_6758.out b/src/test/regress/expected/issue_6758.out new file mode 100644 index 000000000..34f20593f --- /dev/null +++ b/src/test/regress/expected/issue_6758.out @@ -0,0 +1,69 @@ +CREATE SCHEMA issue_6758; +SET search_path to 'issue_6758'; +CREATE TABLE dist0(id int); +SELECT create_distributed_table('dist0','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE dist1(id int); +SELECT create_distributed_table('dist1','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- added to verify we fixed the issue https://github.com/citusdata/citus/issues/6758 +-- generated by Citus query generator tool +SELECT + avg(avgsub.id) +FROM + ( + SELECT + table_0.id + FROM + ( + SELECT + table_1.id + FROM + ( + SELECT + table_2.id + FROM + ( + SELECT + table_3.id + FROM + ( + VALUES + (838) + ) AS table_3(id) FULL + JOIN dist0 AS table_4 USING (id) + WHERE + table_4.id = 3 + ) AS table_2 + WHERE + table_2.id = 2 + ORDER BY + id + LIMIT + 77 + ) AS table_1 + LEFT JOIN dist0 AS table_5 USING (id) + ORDER BY + id + LIMIT + 44 + ) AS table_0 FULL + JOIN dist1 AS table_6 USING (id) + ) AS avgsub; + avg +--------------------------------------------------------------------- + +(1 row) + +DROP SCHEMA issue_6758 CASCADE; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table dist0 +drop cascades to table dist1 diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 1d5ce0798..a78ee6088 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -95,7 +95,7 @@ test: multi_dropped_column_aliases foreign_key_restriction_enforcement test: binary_protocol test: alter_table_set_access_method test: alter_distributed_table -test: issue_5248 issue_5099 issue_5763 issue_6543 +test: issue_5248 issue_5099 issue_5763 issue_6543 issue_6758 test: object_propagation_debug test: undistribute_table test: run_command_on_all_nodes diff --git a/src/test/regress/sql/issue_6758.sql b/src/test/regress/sql/issue_6758.sql new file mode 100644 index 000000000..dae340526 --- /dev/null +++ b/src/test/regress/sql/issue_6758.sql @@ -0,0 +1,55 @@ +CREATE SCHEMA issue_6758; +SET search_path to 'issue_6758'; + +CREATE TABLE dist0(id int); +SELECT create_distributed_table('dist0','id'); + +CREATE TABLE dist1(id int); +SELECT create_distributed_table('dist1','id'); + +-- added to verify we fixed the issue https://github.com/citusdata/citus/issues/6758 +-- generated by Citus query generator tool +SELECT + avg(avgsub.id) +FROM + ( + SELECT + table_0.id + FROM + ( + SELECT + table_1.id + FROM + ( + SELECT + table_2.id + FROM + ( + SELECT + table_3.id + FROM + ( + VALUES + (838) + ) AS table_3(id) FULL + JOIN dist0 AS table_4 USING (id) + WHERE + table_4.id = 3 + ) AS table_2 + WHERE + table_2.id = 2 + ORDER BY + id + LIMIT + 77 + ) AS table_1 + LEFT JOIN dist0 AS table_5 USING (id) + ORDER BY + id + LIMIT + 44 + ) AS table_0 FULL + JOIN dist1 AS table_6 USING (id) + ) AS avgsub; + +DROP SCHEMA issue_6758 CASCADE;