From 8efca3b60ac6a28eaf8787a98996ea47b0d5d633 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 13 Oct 2020 14:19:59 +0300 Subject: [PATCH] Fix a crash with inserting domain composite types in coord. evaluation (#4231) Use short lived per-tuple context in citus_evaluate_expr like (pg) evaluate_expr does. We should not use planState->ExprContext when evaluating expressions as it might lead to freeing the same executor twice (first one happens in citus_evaluate_expr itself and the other one happens when postgres doing clean-up for the top level executor state), which in turn might cause seg.faults. However, now as we don't have necessary planState info to evaluate prepared statements, we also add planState->es_param_list_info to per-tuple ExprContext. --- src/backend/distributed/utils/citus_clauses.c | 21 +++++++----- .../expected/coordinator_evaluation.out | 33 +++++++++++++++++++ .../regress/sql/coordinator_evaluation.sql | 17 ++++++++++ 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/backend/distributed/utils/citus_clauses.c b/src/backend/distributed/utils/citus_clauses.c index 7700364cd..630adaf98 100644 --- a/src/backend/distributed/utils/citus_clauses.c +++ b/src/backend/distributed/utils/citus_clauses.c @@ -309,7 +309,6 @@ citus_evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod, PlanState *planState = NULL; EState *estate; ExprState *exprstate; - ExprContext *econtext; Datum const_val; bool const_is_null; int16 resultTypLen; @@ -357,15 +356,19 @@ citus_evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod, */ exprstate = ExecInitExpr(expr, planState); - if (planState != NULL) + /* + * Get short lived per tuple context as evaluate_expr does. Here we don't + * use planState->ExprContext as it might cause double-free'ing executor + * state. + */ + ExprContext *econtext = GetPerTupleExprContext(estate); + if (planState) { - /* use executor's context to pass down parameters */ - econtext = planState->ps_ExprContext; - } - else - { - /* when called from a function, use a default context */ - econtext = GetPerTupleExprContext(estate); + /* + * If planState exists, then we add es_param_list_info to per tuple + * ExprContext as we need them when evaluating prepared statements. + */ + econtext->ecxt_param_list_info = planState->state->es_param_list_info; } /* diff --git a/src/test/regress/expected/coordinator_evaluation.out b/src/test/regress/expected/coordinator_evaluation.out index 0b972006a..bd309bdb2 100644 --- a/src/test/regress/expected/coordinator_evaluation.out +++ b/src/test/regress/expected/coordinator_evaluation.out @@ -580,5 +580,38 @@ SELECT count(*) FROM coordinator_evaluation_table_2 WHERE key = 101; 7 (1 row) +CREATE TYPE comptype_int as (int_a int); +CREATE DOMAIN domain_comptype_int AS comptype_int CHECK ((VALUE).int_a > 0); +-- citus does not propagate domain types +SELECT run_command_on_workers( +$$ + CREATE DOMAIN coordinator_evaluation.domain_comptype_int AS coordinator_evaluation.comptype_int CHECK ((VALUE).int_a > 0) +$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"CREATE DOMAIN") + (localhost,57638,t,"CREATE DOMAIN") +(2 rows) + +CREATE TABLE reference_table(column_a coordinator_evaluation.domain_comptype_int); +SELECT create_reference_table('reference_table'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO reference_table (column_a) VALUES ('(1)'); +INSERT INTO reference_table (column_a) VALUES ('(2)'), ('(3)'); +INSERT INTO reference_table VALUES ('(4)'), ('(5)'); +SELECT * FROM reference_table ORDER BY 1; + column_a +--------------------------------------------------------------------- + (1) + (2) + (3) + (4) + (5) +(5 rows) + SET client_min_messages TO ERROR; DROP SCHEMA coordinator_evaluation CASCADE; diff --git a/src/test/regress/sql/coordinator_evaluation.sql b/src/test/regress/sql/coordinator_evaluation.sql index 76b1fc47f..e7479fd1a 100644 --- a/src/test/regress/sql/coordinator_evaluation.sql +++ b/src/test/regress/sql/coordinator_evaluation.sql @@ -210,5 +210,22 @@ CALL test_procedure_2(100); CALL test_procedure_2(100); SELECT count(*) FROM coordinator_evaluation_table_2 WHERE key = 101; +CREATE TYPE comptype_int as (int_a int); +CREATE DOMAIN domain_comptype_int AS comptype_int CHECK ((VALUE).int_a > 0); +-- citus does not propagate domain types +SELECT run_command_on_workers( +$$ + CREATE DOMAIN coordinator_evaluation.domain_comptype_int AS coordinator_evaluation.comptype_int CHECK ((VALUE).int_a > 0) +$$); + +CREATE TABLE reference_table(column_a coordinator_evaluation.domain_comptype_int); +SELECT create_reference_table('reference_table'); + +INSERT INTO reference_table (column_a) VALUES ('(1)'); +INSERT INTO reference_table (column_a) VALUES ('(2)'), ('(3)'); +INSERT INTO reference_table VALUES ('(4)'), ('(5)'); + +SELECT * FROM reference_table ORDER BY 1; + SET client_min_messages TO ERROR; DROP SCHEMA coordinator_evaluation CASCADE;