From b8bb22e5cc887969175b0a034ba228e575175b8b Mon Sep 17 00:00:00 2001 From: Metin Doslu Date: Tue, 6 Sep 2016 15:18:20 +0300 Subject: [PATCH] Pass text oid inteads of invalid oid for null values Passing invalid oids even for null values in PQsendQueryParams() causes worker nodes to fail. Therefore, we pass text oid for null values. --- .../executor/multi_router_executor.c | 6 +- .../regress/expected/multi_prepare_plsql.out | 271 ++++++++++++++++++ src/test/regress/sql/multi_prepare_plsql.sql | 114 ++++++++ 3 files changed, 390 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/executor/multi_router_executor.c b/src/backend/distributed/executor/multi_router_executor.c index 19763dca7..7dbc8f0f1 100644 --- a/src/backend/distributed/executor/multi_router_executor.c +++ b/src/backend/distributed/executor/multi_router_executor.c @@ -25,6 +25,7 @@ #include "access/transam.h" #include "access/tupdesc.h" #include "access/xact.h" +#include "catalog/pg_type.h" #include "distributed/citus_clauses.h" #include "distributed/citus_ruleutils.h" #include "distributed/connection_cache.h" @@ -832,11 +833,14 @@ ExtractParametersFromParamListInfo(ParamListInfo paramListInfo, Oid **parameterT /* * If the parameter is NULL, or is not referenced / used (ptype == 0 * would otherwise have errored out inside standard_planner()), - * don't pass a value to the remote side. + * don't pass a value to the remote side, and pass text oid to prevent + * undetermined data type errors on workers. */ if (parameterData->isnull || parameterData->ptype == 0) { (*parameterValues)[parameterIndex] = NULL; + (*parameterTypes)[parameterIndex] = TEXTOID; + continue; } diff --git a/src/test/regress/expected/multi_prepare_plsql.out b/src/test/regress/expected/multi_prepare_plsql.out index 1932ce862..918c5919c 100644 --- a/src/test/regress/expected/multi_prepare_plsql.out +++ b/src/test/regress/expected/multi_prepare_plsql.out @@ -350,6 +350,272 @@ SELECT plpgsql_test_2(); -- FIXME: temporarily disabled, waiting for proper parametrized query support -- SELECT plpgsql_test_6(155); -- SELECT plpgsql_test_6(1555); +-- test router executor parameterized PL/pgsql functions +CREATE TABLE temp_table ( + key int, + value int +); +SELECT master_create_distributed_table('temp_table','key','hash'); + master_create_distributed_table +--------------------------------- + +(1 row) + +SELECT master_create_worker_shards('temp_table',4,1); + master_create_worker_shards +----------------------------- + +(1 row) + +CREATE OR REPLACE FUNCTION no_parameter_insert() RETURNS void as $$ +BEGIN + INSERT INTO temp_table (key) VALUES (0); +END; +$$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage +SELECT no_parameter_insert(); + no_parameter_insert +--------------------- + +(1 row) + +SELECT no_parameter_insert(); + no_parameter_insert +--------------------- + +(1 row) + +SELECT no_parameter_insert(); + no_parameter_insert +--------------------- + +(1 row) + +SELECT no_parameter_insert(); + no_parameter_insert +--------------------- + +(1 row) + +SELECT no_parameter_insert(); + no_parameter_insert +--------------------- + +(1 row) + +SELECT no_parameter_insert(); + no_parameter_insert +--------------------- + +(1 row) + +CREATE OR REPLACE FUNCTION single_parameter_insert(key_arg int) RETURNS void as $$ +BEGIN + INSERT INTO temp_table (key) VALUES (key_arg); +END; +$$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage +SELECT single_parameter_insert(1); + single_parameter_insert +------------------------- + +(1 row) + +SELECT single_parameter_insert(2); + single_parameter_insert +------------------------- + +(1 row) + +SELECT single_parameter_insert(3); + single_parameter_insert +------------------------- + +(1 row) + +SELECT single_parameter_insert(4); + single_parameter_insert +------------------------- + +(1 row) + +SELECT single_parameter_insert(5); + single_parameter_insert +------------------------- + +(1 row) + +SELECT single_parameter_insert(6); +ERROR: values given for the partition column must be constants or constant expressions +CONTEXT: SQL statement "INSERT INTO temp_table (key) VALUES (key_arg)" +PL/pgSQL function single_parameter_insert(integer) line 3 at SQL statement +CREATE OR REPLACE FUNCTION double_parameter_insert(key_arg int, value_arg int) RETURNS void as $$ +BEGIN + INSERT INTO temp_table (key, value) VALUES (key_arg, value_arg); +END; +$$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage +SELECT double_parameter_insert(1, 10); + double_parameter_insert +------------------------- + +(1 row) + +SELECT double_parameter_insert(2, 20); + double_parameter_insert +------------------------- + +(1 row) + +SELECT double_parameter_insert(3, 30); + double_parameter_insert +------------------------- + +(1 row) + +SELECT double_parameter_insert(4, 40); + double_parameter_insert +------------------------- + +(1 row) + +SELECT double_parameter_insert(5, 50); + double_parameter_insert +------------------------- + +(1 row) + +SELECT double_parameter_insert(6, 60); +ERROR: values given for the partition column must be constants or constant expressions +CONTEXT: SQL statement "INSERT INTO temp_table (key, value) VALUES (key_arg, value_arg)" +PL/pgSQL function double_parameter_insert(integer,integer) line 3 at SQL statement +-- check inserted values +SELECT * FROM temp_table ORDER BY key, value; + key | value +-----+------- + 0 | + 0 | + 0 | + 0 | + 0 | + 0 | + 1 | 10 + 1 | + 2 | 20 + 2 | + 3 | 30 + 3 | + 4 | 40 + 4 | + 5 | 50 + 5 | +(16 rows) + +-- check router executor select +CREATE OR REPLACE FUNCTION partition_column_select(key_arg int) RETURNS TABLE(key int, value int) AS $$ +DECLARE +BEGIN + RETURN QUERY + SELECT + temp_table.key, + temp_table.value + FROM + temp_table + WHERE + temp_table.key = key_arg + ORDER BY + key, + value; +END; +$$ LANGUAGE plpgsql; +SELECT partition_column_select(1); + partition_column_select +------------------------- + (1,10) + (1,) +(2 rows) + +SELECT partition_column_select(2); + partition_column_select +------------------------- + (2,20) + (2,) +(2 rows) + +SELECT partition_column_select(3); + partition_column_select +------------------------- + (3,30) + (3,) +(2 rows) + +SELECT partition_column_select(4); + partition_column_select +------------------------- + (4,40) + (4,) +(2 rows) + +SELECT partition_column_select(5); + partition_column_select +------------------------- + (5,50) + (5,) +(2 rows) + +-- 6th execution is failing. We don't want to run the failing test because of +-- changing output. After implementing this feature, uncomment this. +-- SELECT partition_column_select(6); +-- check real-time executor +CREATE OR REPLACE FUNCTION non_partition_column_select(value_arg int) RETURNS TABLE(key int, value int) AS $$ +DECLARE +BEGIN + RETURN QUERY + SELECT + temp_table.key, + temp_table.value + FROM + temp_table + WHERE + temp_table.value = value_arg + ORDER BY + key, + value; +END; +$$ LANGUAGE plpgsql; +SELECT non_partition_column_select(10); + non_partition_column_select +----------------------------- + (1,10) +(1 row) + +SELECT non_partition_column_select(20); + non_partition_column_select +----------------------------- + (2,20) +(1 row) + +SELECT non_partition_column_select(30); + non_partition_column_select +----------------------------- + (3,30) +(1 row) + +SELECT non_partition_column_select(40); + non_partition_column_select +----------------------------- + (4,40) +(1 row) + +SELECT non_partition_column_select(50); + non_partition_column_select +----------------------------- + (5,50) +(1 row) + +-- 6th execution is failing. We don't want to run the failing test because of +-- changing output. After implementing this feature, uncomment this. +-- SELECT partition_column_select(6); -- clean-up functions DROP FUNCTION sql_test_no_1(); DROP FUNCTION sql_test_no_2(); @@ -363,3 +629,8 @@ DROP FUNCTION plpgsql_test_4(); DROP FUNCTION plpgsql_test_5(); DROP FUNCTION plpgsql_test_6(int); DROP FUNCTION plpgsql_test_7(text, text); +DROP FUNCTION no_parameter_insert(); +DROP FUNCTION single_parameter_insert(int); +DROP FUNCTION double_parameter_insert(int, int); +DROP FUNCTION partition_column_select(int); +DROP FUNCTION non_partition_column_select(int); diff --git a/src/test/regress/sql/multi_prepare_plsql.sql b/src/test/regress/sql/multi_prepare_plsql.sql index 8fd1d0bd4..8b9673fe1 100644 --- a/src/test/regress/sql/multi_prepare_plsql.sql +++ b/src/test/regress/sql/multi_prepare_plsql.sql @@ -247,6 +247,115 @@ SELECT plpgsql_test_2(); -- SELECT plpgsql_test_6(155); -- SELECT plpgsql_test_6(1555); +-- test router executor parameterized PL/pgsql functions +CREATE TABLE temp_table ( + key int, + value int +); +SELECT master_create_distributed_table('temp_table','key','hash'); +SELECT master_create_worker_shards('temp_table',4,1); + +CREATE OR REPLACE FUNCTION no_parameter_insert() RETURNS void as $$ +BEGIN + INSERT INTO temp_table (key) VALUES (0); +END; +$$ LANGUAGE plpgsql; + +-- execute 6 times to trigger prepared statement usage +SELECT no_parameter_insert(); +SELECT no_parameter_insert(); +SELECT no_parameter_insert(); +SELECT no_parameter_insert(); +SELECT no_parameter_insert(); +SELECT no_parameter_insert(); + +CREATE OR REPLACE FUNCTION single_parameter_insert(key_arg int) RETURNS void as $$ +BEGIN + INSERT INTO temp_table (key) VALUES (key_arg); +END; +$$ LANGUAGE plpgsql; + +-- execute 6 times to trigger prepared statement usage +SELECT single_parameter_insert(1); +SELECT single_parameter_insert(2); +SELECT single_parameter_insert(3); +SELECT single_parameter_insert(4); +SELECT single_parameter_insert(5); +SELECT single_parameter_insert(6); + +CREATE OR REPLACE FUNCTION double_parameter_insert(key_arg int, value_arg int) RETURNS void as $$ +BEGIN + INSERT INTO temp_table (key, value) VALUES (key_arg, value_arg); +END; +$$ LANGUAGE plpgsql; + +-- execute 6 times to trigger prepared statement usage +SELECT double_parameter_insert(1, 10); +SELECT double_parameter_insert(2, 20); +SELECT double_parameter_insert(3, 30); +SELECT double_parameter_insert(4, 40); +SELECT double_parameter_insert(5, 50); +SELECT double_parameter_insert(6, 60); + +-- check inserted values +SELECT * FROM temp_table ORDER BY key, value; + +-- check router executor select +CREATE OR REPLACE FUNCTION partition_column_select(key_arg int) RETURNS TABLE(key int, value int) AS $$ +DECLARE +BEGIN + RETURN QUERY + SELECT + temp_table.key, + temp_table.value + FROM + temp_table + WHERE + temp_table.key = key_arg + ORDER BY + key, + value; +END; +$$ LANGUAGE plpgsql; + +SELECT partition_column_select(1); +SELECT partition_column_select(2); +SELECT partition_column_select(3); +SELECT partition_column_select(4); +SELECT partition_column_select(5); + +-- 6th execution is failing. We don't want to run the failing test because of +-- changing output. After implementing this feature, uncomment this. +-- SELECT partition_column_select(6); + +-- check real-time executor +CREATE OR REPLACE FUNCTION non_partition_column_select(value_arg int) RETURNS TABLE(key int, value int) AS $$ +DECLARE +BEGIN + RETURN QUERY + SELECT + temp_table.key, + temp_table.value + FROM + temp_table + WHERE + temp_table.value = value_arg + ORDER BY + key, + value; +END; +$$ LANGUAGE plpgsql; + +SELECT non_partition_column_select(10); +SELECT non_partition_column_select(20); +SELECT non_partition_column_select(30); +SELECT non_partition_column_select(40); +SELECT non_partition_column_select(50); + +-- 6th execution is failing. We don't want to run the failing test because of +-- changing output. After implementing this feature, uncomment this. +-- SELECT partition_column_select(6); + -- clean-up functions DROP FUNCTION sql_test_no_1(); DROP FUNCTION sql_test_no_2(); @@ -260,3 +369,8 @@ DROP FUNCTION plpgsql_test_4(); DROP FUNCTION plpgsql_test_5(); DROP FUNCTION plpgsql_test_6(int); DROP FUNCTION plpgsql_test_7(text, text); +DROP FUNCTION no_parameter_insert(); +DROP FUNCTION single_parameter_insert(int); +DROP FUNCTION double_parameter_insert(int, int); +DROP FUNCTION partition_column_select(int); +DROP FUNCTION non_partition_column_select(int);