From 31debc96e3b8a0ac0a3ccadb04f82b9258b33c7e Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Thu, 6 Jul 2017 16:17:35 +0200 Subject: [PATCH] Handle implicit casts in prepared INSERTs --- src/backend/distributed/utils/citus_clauses.c | 90 ++++++------------- .../regress/expected/multi_prepare_sql.out | 72 +++++++++++++++ src/test/regress/sql/multi_prepare_sql.sql | 47 ++++++++++ 3 files changed, 144 insertions(+), 65 deletions(-) diff --git a/src/backend/distributed/utils/citus_clauses.c b/src/backend/distributed/utils/citus_clauses.c index af90d0b12..2d0a5f92e 100644 --- a/src/backend/distributed/utils/citus_clauses.c +++ b/src/backend/distributed/utils/citus_clauses.c @@ -237,76 +237,36 @@ PartiallyEvaluateExpressionMutator(Node *expression, FunctionEvaluationContext * static Node * EvaluateNodeIfReferencesFunction(Node *expression, PlanState *planState) { - if (IsA(expression, FuncExpr)) + if (expression == NULL || IsA(expression, Const)) { - FuncExpr *expr = (FuncExpr *) expression; - - return (Node *) citus_evaluate_expr((Expr *) expr, - expr->funcresulttype, - exprTypmod((Node *) expr), - expr->funccollid, - planState); + return expression; } - if (IsA(expression, OpExpr) || - IsA(expression, DistinctExpr) || - IsA(expression, NullIfExpr)) + switch (nodeTag(expression)) { - /* structural equivalence */ - OpExpr *expr = (OpExpr *) expression; + case T_FuncExpr: + case T_OpExpr: + case T_DistinctExpr: + case T_NullIfExpr: + case T_CoerceViaIO: + case T_ArrayCoerceExpr: + case T_ScalarArrayOpExpr: + case T_RowCompareExpr: + case T_Param: + case T_RelabelType: + case T_CoerceToDomain: + { + return (Node *) citus_evaluate_expr((Expr *) expression, + exprType(expression), + exprTypmod(expression), + exprCollation(expression), + planState); + } - return (Node *) citus_evaluate_expr((Expr *) expr, - expr->opresulttype, -1, - expr->opcollid, - planState); - } - - if (IsA(expression, CoerceViaIO)) - { - CoerceViaIO *expr = (CoerceViaIO *) expression; - - return (Node *) citus_evaluate_expr((Expr *) expr, - expr->resulttype, -1, - expr->resultcollid, - planState); - } - - if (IsA(expression, ArrayCoerceExpr)) - { - ArrayCoerceExpr *expr = (ArrayCoerceExpr *) expression; - - return (Node *) citus_evaluate_expr((Expr *) expr, - expr->resulttype, - expr->resulttypmod, - expr->resultcollid, - planState); - } - - if (IsA(expression, ScalarArrayOpExpr)) - { - ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) expression; - - return (Node *) citus_evaluate_expr((Expr *) expr, BOOLOID, -1, InvalidOid, - planState); - } - - if (IsA(expression, RowCompareExpr)) - { - RowCompareExpr *expr = (RowCompareExpr *) expression; - - return (Node *) citus_evaluate_expr((Expr *) expr, BOOLOID, -1, InvalidOid, - planState); - } - - if (IsA(expression, Param)) - { - Param *param = (Param *) expression; - - return (Node *) citus_evaluate_expr((Expr *) param, - param->paramtype, - param->paramtypmod, - param->paramcollid, - planState); + default: + { + break; + } } return expression; diff --git a/src/test/regress/expected/multi_prepare_sql.out b/src/test/regress/expected/multi_prepare_sql.out index 9083aca01..fbc951413 100644 --- a/src/test/regress/expected/multi_prepare_sql.out +++ b/src/test/regress/expected/multi_prepare_sql.out @@ -899,6 +899,78 @@ SELECT key, value2 FROM prepare_func_table; (6 rows) DROP TABLE prepare_func_table; +-- Text columns can give issues when there is an implicit cast from varchar +CREATE TABLE text_partition_column_table ( + key text NOT NULL, + value int +); +SELECT create_distributed_table('text_partition_column_table', 'key'); + create_distributed_table +-------------------------- + +(1 row) + +PREPARE prepared_relabel_insert(varchar) AS + INSERT INTO text_partition_column_table VALUES ($1, 1); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +SELECT key, value FROM text_partition_column_table ORDER BY key; + key | value +------+------- + test | 1 + test | 1 + test | 1 + test | 1 + test | 1 + test | 1 +(6 rows) + +DROP TABLE text_partition_column_table; +-- Domain type columns can give issues +CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$'); +SELECT run_command_on_workers($$ + CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$') +$$); + run_command_on_workers +------------------------------------- + (localhost,57637,t,"CREATE DOMAIN") + (localhost,57638,t,"CREATE DOMAIN") +(2 rows) + +CREATE TABLE domain_partition_column_table ( + key test_key NOT NULL, + value int +); +SELECT create_distributed_table('domain_partition_column_table', 'key'); + create_distributed_table +-------------------------- + +(1 row) + +PREPARE prepared_coercion_to_domain_insert(text) AS + INSERT INTO domain_partition_column_table VALUES ($1, 1); +EXECUTE prepared_coercion_to_domain_insert('test-1'); +EXECUTE prepared_coercion_to_domain_insert('test-2'); +EXECUTE prepared_coercion_to_domain_insert('test-3'); +EXECUTE prepared_coercion_to_domain_insert('test-4'); +EXECUTE prepared_coercion_to_domain_insert('test-5'); +EXECUTE prepared_coercion_to_domain_insert('test-6'); +SELECT key, value FROM domain_partition_column_table ORDER BY key; + key | value +--------+------- + test-1 | 1 + test-2 | 1 + test-3 | 1 + test-4 | 1 + test-5 | 1 + test-6 | 1 +(6 rows) + +DROP TABLE domain_partition_column_table; -- verify placement state updates invalidate shard state -- -- We use a immutable function to check for that. The planner will diff --git a/src/test/regress/sql/multi_prepare_sql.sql b/src/test/regress/sql/multi_prepare_sql.sql index a49f154b0..bde59efbf 100644 --- a/src/test/regress/sql/multi_prepare_sql.sql +++ b/src/test/regress/sql/multi_prepare_sql.sql @@ -473,6 +473,53 @@ SELECT key, value2 FROM prepare_func_table; DROP TABLE prepare_func_table; +-- Text columns can give issues when there is an implicit cast from varchar +CREATE TABLE text_partition_column_table ( + key text NOT NULL, + value int +); +SELECT create_distributed_table('text_partition_column_table', 'key'); + +PREPARE prepared_relabel_insert(varchar) AS + INSERT INTO text_partition_column_table VALUES ($1, 1); + +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); +EXECUTE prepared_relabel_insert('test'); + +SELECT key, value FROM text_partition_column_table ORDER BY key; + +DROP TABLE text_partition_column_table; + +-- Domain type columns can give issues +CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$'); +SELECT run_command_on_workers($$ + CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$') +$$); + +CREATE TABLE domain_partition_column_table ( + key test_key NOT NULL, + value int +); +SELECT create_distributed_table('domain_partition_column_table', 'key'); + +PREPARE prepared_coercion_to_domain_insert(text) AS + INSERT INTO domain_partition_column_table VALUES ($1, 1); + +EXECUTE prepared_coercion_to_domain_insert('test-1'); +EXECUTE prepared_coercion_to_domain_insert('test-2'); +EXECUTE prepared_coercion_to_domain_insert('test-3'); +EXECUTE prepared_coercion_to_domain_insert('test-4'); +EXECUTE prepared_coercion_to_domain_insert('test-5'); +EXECUTE prepared_coercion_to_domain_insert('test-6'); + +SELECT key, value FROM domain_partition_column_table ORDER BY key; + +DROP TABLE domain_partition_column_table; + -- verify placement state updates invalidate shard state -- -- We use a immutable function to check for that. The planner will