mirror of https://github.com/citusdata/citus.git
fix issue #7676: wrong handler around MULTIEXPR
Citus was checking for presence of sublink, but forgot to manage multiexpr while evaluating clauses during planning. At this stage (citus planner), it's not always possible to call PostgreSQL code because the tree is not yet ready for PostgreSQL pure executor. https://github.com/citusdata/citus/issues/7676 Fixed by adding a new function to check sublink or multiexp in the tree.pull/7914/head
parent
1a3316281c
commit
415ebedbaf
|
@ -41,6 +41,7 @@ static bool ShouldEvaluateExpression(Expr *expression);
|
||||||
static bool ShouldEvaluateFunctions(CoordinatorEvaluationContext *evaluationContext);
|
static bool ShouldEvaluateFunctions(CoordinatorEvaluationContext *evaluationContext);
|
||||||
static void FixFunctionArguments(Node *expr);
|
static void FixFunctionArguments(Node *expr);
|
||||||
static bool FixFunctionArgumentsWalker(Node *expr, void *context);
|
static bool FixFunctionArgumentsWalker(Node *expr, void *context);
|
||||||
|
static bool CheckContainsMultiexprOrSublink(Node *expr);
|
||||||
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -99,15 +100,15 @@ PartiallyEvaluateExpression(Node *expression,
|
||||||
}
|
}
|
||||||
|
|
||||||
NodeTag nodeTag = nodeTag(expression);
|
NodeTag nodeTag = nodeTag(expression);
|
||||||
if (nodeTag == T_Param)
|
if (CheckContainsMultiexprOrSublink(expression))
|
||||||
{
|
{
|
||||||
Param *param = (Param *) expression;
|
/* ExecInitExpr cannot handle PARAM_MULTIEXPR and PARAM_SUBLINK */
|
||||||
if (param->paramkind == PARAM_SUBLINK)
|
|
||||||
{
|
|
||||||
/* ExecInitExpr cannot handle PARAM_SUBLINK */
|
|
||||||
return expression;
|
return expression;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* ExecInitExpr cannot handle PARAM_MULTIEXPR and PARAM_SUBLINK but we have guards */
|
||||||
|
else if (nodeTag == T_Param)
|
||||||
|
{
|
||||||
return (Node *) citus_evaluate_expr((Expr *) expression,
|
return (Node *) citus_evaluate_expr((Expr *) expression,
|
||||||
exprType(expression),
|
exprType(expression),
|
||||||
exprTypmod(expression),
|
exprTypmod(expression),
|
||||||
|
@ -537,3 +538,43 @@ FixFunctionArgumentsWalker(Node *expr, void *context)
|
||||||
|
|
||||||
return expression_tree_walker(expr, FixFunctionArgumentsWalker, NULL);
|
return expression_tree_walker(expr, FixFunctionArgumentsWalker, NULL);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Recursively search an expression for a Param with sublink
|
||||||
|
*/
|
||||||
|
static bool
|
||||||
|
CheckContainsMultiexprOrSublink(Node *expr)
|
||||||
|
{
|
||||||
|
if (expr == NULL)
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* If it's a Param, return its attnum */
|
||||||
|
else if (IsA(expr, Param))
|
||||||
|
{
|
||||||
|
Param *param = (Param *) expr;
|
||||||
|
if (param->paramkind == PARAM_MULTIEXPR ||
|
||||||
|
param->paramkind == PARAM_SUBLINK)
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/* If it's a FuncExpr, search in arguments */
|
||||||
|
else if (IsA(expr, FuncExpr))
|
||||||
|
{
|
||||||
|
FuncExpr *func = (FuncExpr *) expr;
|
||||||
|
ListCell *lc;
|
||||||
|
|
||||||
|
foreach(lc, func->args)
|
||||||
|
{
|
||||||
|
if (CheckContainsMultiexprOrSublink((Node *) lfirst(lc)))
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
|
@ -0,0 +1,35 @@
|
||||||
|
-- https://github.com/citusdata/citus/issues/7676
|
||||||
|
CREATE TABLE test_ref_multiexpr (
|
||||||
|
id bigint primary key
|
||||||
|
, col_int integer
|
||||||
|
, col_bool bool
|
||||||
|
, col_text text
|
||||||
|
, col_timestamp timestamp
|
||||||
|
);
|
||||||
|
select create_reference_table('test_ref_multiexpr');
|
||||||
|
create_reference_table
|
||||||
|
---------------------------------------------------------------------
|
||||||
|
|
||||||
|
(1 row)
|
||||||
|
|
||||||
|
/* TODO how to ensure in test that 'now()' is correctly pre-executed */
|
||||||
|
insert into test_ref_multiexpr values (1, 1, true, 'one', now());
|
||||||
|
update test_ref_multiexpr
|
||||||
|
SET (col_timestamp)
|
||||||
|
= (SELECT now())
|
||||||
|
returning id, col_int, col_bool;
|
||||||
|
id | col_int | col_bool
|
||||||
|
---------------------------------------------------------------------
|
||||||
|
1 | 1 | t
|
||||||
|
(1 row)
|
||||||
|
|
||||||
|
update test_ref_multiexpr
|
||||||
|
SET (col_bool, col_timestamp)
|
||||||
|
= (SELECT true, now())
|
||||||
|
returning id, col_int, col_bool;
|
||||||
|
id | col_int | col_bool
|
||||||
|
---------------------------------------------------------------------
|
||||||
|
1 | 1 | t
|
||||||
|
(1 row)
|
||||||
|
|
||||||
|
DROP TABLE test_ref_multiexpr;
|
|
@ -103,7 +103,7 @@ test: multi_dropped_column_aliases foreign_key_restriction_enforcement
|
||||||
test: binary_protocol
|
test: binary_protocol
|
||||||
test: alter_table_set_access_method
|
test: alter_table_set_access_method
|
||||||
test: alter_distributed_table
|
test: alter_distributed_table
|
||||||
test: issue_5248 issue_5099 issue_5763 issue_6543 issue_6758 issue_7477
|
test: issue_5248 issue_5099 issue_5763 issue_6543 issue_6758 issue_7477 issue_7676
|
||||||
test: object_propagation_debug
|
test: object_propagation_debug
|
||||||
test: undistribute_table
|
test: undistribute_table
|
||||||
test: run_command_on_all_nodes
|
test: run_command_on_all_nodes
|
||||||
|
|
|
@ -0,0 +1,24 @@
|
||||||
|
-- https://github.com/citusdata/citus/issues/7676
|
||||||
|
CREATE TABLE test_ref_multiexpr (
|
||||||
|
id bigint primary key
|
||||||
|
, col_int integer
|
||||||
|
, col_bool bool
|
||||||
|
, col_text text
|
||||||
|
, col_timestamp timestamp
|
||||||
|
);
|
||||||
|
select create_reference_table('test_ref_multiexpr');
|
||||||
|
|
||||||
|
/* TODO how to ensure in test that 'now()' is correctly pre-executed */
|
||||||
|
insert into test_ref_multiexpr values (1, 1, true, 'one', now());
|
||||||
|
|
||||||
|
update test_ref_multiexpr
|
||||||
|
SET (col_timestamp)
|
||||||
|
= (SELECT now())
|
||||||
|
returning id, col_int, col_bool;
|
||||||
|
|
||||||
|
update test_ref_multiexpr
|
||||||
|
SET (col_bool, col_timestamp)
|
||||||
|
= (SELECT true, now())
|
||||||
|
returning id, col_int, col_bool;
|
||||||
|
|
||||||
|
DROP TABLE test_ref_multiexpr;
|
Loading…
Reference in New Issue