pgpool2.git / summary / log / commit / refs
commit fc9fa8dbe21dd561b049ad94377a1c0246aec493
Author: Tatsuo Ishii <ishii@sraoss.co.jp>
Date: Sun Jul 26 01:04:36 2020 +0000
Enhance the way getting function names in multiple places.
With query cache enabled, Pgpool-II examines function calls in
SELECTs. However if a function were called with schema qualification,
it was not recognized. This commit fix to allow schema qualifications
in the case.
Also we did not allow schema qualified function names in
black_function_list and white_function_list. The limitation is also
fixed in this commit. Note that if you want to register schema
qualified function names, you have to register function names without
schema qualification as well.
Patch contributed by Hou, Zhijie, Reviewed by me.
Discussion: https://www.pgpool.net/pipermail/pgpool-hackers/2020-July/003718.html
Discussion: https://www.pgpool.net/pipermail/pgpool-hackers/2020-July/003732.html
doc/src/sgml/loadbalance.sgml | 42 ++++--
src/include/utils/pool_relcache.h | 1 +
src/include/utils/pool_select_walker.h | 1 +
src/test/regression/tests/001.load_balance/test.sh | 25 +++-
src/test/regression/tests/006.memqcache/test.sh | 9 ++
src/utils/pool_relcache.c | 154 ++++++++++++++++++--
src/utils/pool_select_walker.c | 161 +++++++++++++++++----
7 files changed, 336 insertions(+), 57 deletions(-)
diff --git a/doc/src/sgml/loadbalance.sgml b/doc/src/sgml/loadbalance.sgml
index be03fdc60..606fa6e63 100644
--- a/doc/src/sgml/loadbalance.sgml
+++ b/doc/src/sgml/loadbalance.sgml
@@ -513,14 +513,19 @@
<note>
<para>
- Schema qualifications can not be used
- in <xref linkend="guc-white-function-list">
- because <productname>Pgpool-II</productname> silently
- ignores a schema qualification in function names appearing
- in an input SQL while comparing the list and the input
- SQL. As a result, a schema qualified function name in the
- list never matches function names appearing in the input
- SQL.
+ If the queries can refer to the function with and without the schema
+ qualification then you must add both entries (with and without
+ schema name) in the list.
+ <programlisting>
+ #For example:
+ #If the queries sometime use "f1()" and other times "public.f1()"
+ #to refer the function f1 then the white_function_list
+ #would be configured as follows.
+
+ white_function_list = "f1,public.f1"
+
+ </programlisting>
+
</para>
</note>
@@ -564,14 +569,19 @@
<note>
<para>
- Schema qualifications can not be used
- in <xref linkend="guc-black-function-list">
- because <productname>Pgpool-II</productname> silently
- ignores a schema qualification in function names appearing
- in an input SQL while comparing the list and the input
- SQL. As a result, a schema qualified function name in the
- list never matches function names appearing in the input
- SQL.
+ If the queries can refer the function with and without the schema
+ qualification then you must add both entries(with and without
+ schema name) in the list.
+ <programlisting>
+ #For example:
+ #If the queries sometime use "f1()" and other times "public.f1()"
+ #to refer the function f1 then the black_function_list
+ #would be configured as follows.
+
+ black_function_list = "f1,public.f1"
+
+ </programlisting>
+
</para>
</note>
diff --git a/src/include/utils/pool_relcache.h b/src/include/utils/pool_relcache.h
index 6b090523a..d4a367857 100644
--- a/src/include/utils/pool_relcache.h
+++ b/src/include/utils/pool_relcache.h
@@ -80,5 +80,6 @@ extern void *int_register_func(POOL_SELECT_RESULT * res);
extern void *int_unregister_func(void *data);
extern void *string_register_func(POOL_SELECT_RESULT * res);
extern void *string_unregister_func(void *data);
+extern bool SplitIdentifierString(char *rawstring, char separator, Node **namelist);
#endif /* POOL_RELCACHE_H */
diff --git a/src/include/utils/pool_select_walker.h b/src/include/utils/pool_select_walker.h
index 10323b08f..eaa0a6fea 100644
--- a/src/include/utils/pool_select_walker.h
+++ b/src/include/utils/pool_select_walker.h
@@ -70,6 +70,7 @@ extern int pool_table_name_to_oid(char *table_name);
extern int pool_extract_table_oids_from_select_stmt(Node *node, SelectContext * ctx);
extern RangeVar *makeRangeVarFromNameList(List *names);
extern char *make_table_name_from_rangevar(RangeVar *rangevar);
+extern char *make_function_name_from_funccall(FuncCall *fcall);
extern int pattern_compare(char *str, const int type, const char *param_name);
extern bool is_unlogged_table(char *table_name);
extern bool is_view(char *table_name);
diff --git a/src/test/regression/tests/001.load_balance/test.sh b/src/test/regression/tests/001.load_balance/test.sh
index 932744105..0f1cf409c 100755
--- a/src/test/regression/tests/001.load_balance/test.sh
+++ b/src/test/regression/tests/001.load_balance/test.sh
@@ -24,7 +24,7 @@ do
echo "backend_weight0 = 0" >> etc/pgpool.conf
echo "backend_weight1 = 1" >> etc/pgpool.conf
- echo "black_function_list = 'f1'" >> etc/pgpool.conf
+ echo "black_function_list = 'f1,public.f2'" >> etc/pgpool.conf
./startall
@@ -36,8 +36,10 @@ do
CREATE TABLE t1(i INTEGER);
CREATE TABLE t2(i INTEGER);
CREATE FUNCTION f1(INTEGER) returns INTEGER AS 'SELECT \$1' LANGUAGE SQL;
+CREATE FUNCTION f2(INTEGER) returns INTEGER AS 'SELECT \$1' LANGUAGE SQL;
SELECT * FROM t1; -- this load balances
-SELECT f1(1); -- this does not load balance
+SELECT f1(1); -- this does not load balance
+SELECT public.f2(1); -- this does not load balance
EOF
# check if simple load balance worked
@@ -58,16 +60,24 @@ EOF
./shutdownall
exit 1
fi
+ fgrep "SELECT public.f2(1);" log/pgpool.log |grep "DB node id: 0">/dev/null 2>&1
+ if [ $? != 0 ];then
+ # expected result not found
+ echo fail: black function is sent to node 1.
+ ./shutdownall
+ exit 1
+ fi
echo ok: black function list works.
- echo "white_function_list = 'f1'" >> etc/pgpool.conf
+ echo "white_function_list = 'f1,public.f2'" >> etc/pgpool.conf
echo "black_function_list = ''" >> etc/pgpool.conf
./pgpool_reload
sleep $st
$PSQL test <<EOF
-SELECT f1(1); -- this does load balance
+SELECT f1(1); -- this does load balance
+SELECT public.f2(1); -- this does load balance
EOF
# check if white function list worked
@@ -78,6 +88,13 @@ EOF
./shutdownall
exit 1
fi
+ fgrep "SELECT public.f2(1);" log/pgpool.log |grep "DB node id: 1">/dev/null 2>&1
+ if [ $? != 0 ];then
+ # expected result not found
+ echo fail: white function is sent to zero-weight node.
+ ./shutdownall
+ exit 1
+ fi
echo ok: white function list works.
# check if black query pattern list worked
diff --git a/src/test/regression/tests/006.memqcache/test.sh b/src/test/regression/tests/006.memqcache/test.sh
index 0272cf4e3..549c8a600 100755
--- a/src/test/regression/tests/006.memqcache/test.sh
+++ b/src/test/regression/tests/006.memqcache/test.sh
@@ -39,11 +39,14 @@ do
wait_for_pgpool_startup
$PSQL test <<EOF
+CREATE SCHEMA other_schema;
CREATE TABLE t1 (i int);
CREATE TABLE black_t (i int);
CREATE TABLE with_modify (i int);
CREATE VIEW normal_v AS SELECT * FROM t1;
CREATE VIEW white_v AS SELECT * FROM t1;
+CREATE FUNCTION public.immutable_func(INTEGER) returns INTEGER AS 'SELECT \$1' LANGUAGE SQL IMMUTABLE;
+CREATE FUNCTION other_schema.volatile_func(INTEGER) returns INTEGER AS 'SELECT \$1' LANGUAGE SQL VOLATILE;
SELECT pg_sleep(2); -- Sleep for a while to make sure object creations are replicated
SELECT * FROM t1;
SELECT * FROM t1;
@@ -57,6 +60,10 @@ SELECT * FROM with_modify;
WITH cte AS (INSERT INTO with_modify values(1) RETURNING *) SELECT * FROM with_modify;
WITH cte AS (INSERT INTO with_modify values(1) RETURNING *) SELECT * FROM with_modify;
SELECT * FROM with_modify;
+select public.immutable_func(1);
+select public.immutable_func(1);
+select other_schema.volatile_func(1);
+select other_schema.volatile_func(1);
EOF
success=true
@@ -65,6 +72,8 @@ EOF
grep "fetched from cache" log/pgpool.log | grep normal_v > /dev/null && success=false
grep "fetched from cache" log/pgpool.log | grep white_v > /dev/null || success=false
grep "fetched from cache" log/pgpool.log | grep with_modify > /dev/null && success=false
+ grep "fetched from cache" log/pgpool.log | grep immutable_func > /dev/null || success=false
+ grep "fetched from cache" log/pgpool.log | grep volatile_func > /dev/null && success=false
if [ $success = false ];then
./shutdownall
exit 1
diff --git a/src/utils/pool_relcache.c b/src/utils/pool_relcache.c
index c801b948a..e4b995f96 100644
--- a/src/utils/pool_relcache.c
+++ b/src/utils/pool_relcache.c
@@ -36,6 +36,7 @@
#include "utils/palloc.h"
#include "utils/memutils.h"
#include "utils/elog.h"
+#include "parser/scansup.h"
static void SearchRelCacheErrorCb(void *arg);
static POOL_SELECT_RESULT *query_cache_to_relation_cache(char *data, size_t size);
@@ -343,25 +344,154 @@ SearchRelCacheErrorCb(void *arg)
}
+/*
+ * SplitIdentifierString --- parse a string containing identifiers
+ *
+ * This is the guts of textToQualifiedNameList, and is exported for use in
+ * other situations such as parsing GUC variables. In the GUC case, it's
+ * important to avoid memory leaks, so the API is designed to minimize the
+ * amount of stuff that needs to be allocated and freed.
+ *
+ * Inputs:
+ * rawstring: the input string; must be overwritable! On return, it's
+ * been modified to contain the separated identifiers.
+ * separator: the separator punctuation expected between identifiers
+ * (typically '.' or ','). Whitespace may also appear around
+ * identifiers.
+ * Outputs:
+ * namelist: filled with a palloc'd list of pointers to identifiers within
+ * rawstring. Caller should list_free() this even on error return.
+ *
+ * Returns true if okay, false if there is a syntax error in the string.
+ *
+ * Note that an empty string is considered okay here, though not in
+ * textToQualifiedNameList.
+ */
+bool
+SplitIdentifierString(char *rawstring, char separator,
+ Node **nlist)
+{
+ char *nextp = rawstring;
+ bool done = false;
+ List **namelist = (List **) nlist;
+
+ *namelist = NIL;
+
+ while (scanner_isspace(*nextp))
+ nextp++; /* skip leading whitespace */
+
+ if (*nextp == '\0')
+ return true; /* allow empty string */
+
+ /* At the top of the loop, we are at start of a new identifier. */
+ do
+ {
+ char *curname;
+ char *endp;
+
+ if (*nextp == '"')
+ {
+ /* Quoted name --- collapse quote-quote pairs, no downcasing */
+ curname = nextp + 1;
+ for (;;)
+ {
+ endp = strchr(nextp + 1, '"');
+ if (endp == NULL)
+ return false; /* mismatched quotes */
+ if (endp[1] != '"')
+ break; /* found end of quoted name */
+ /* Collapse adjacent quotes into one quote, and look again */
+ memmove(endp, endp + 1, strlen(endp));
+ nextp = endp;
+ }
+ /* endp now points at the terminating quote */
+ nextp = endp + 1;
+ }
+ else
+ {
+ /* Unquoted name --- extends to separator or whitespace */
+ char *downname;
+ int len;
+
+ curname = nextp;
+ while (*nextp && *nextp != separator &&
+ !scanner_isspace(*nextp))
+ nextp++;
+ endp = nextp;
+ if (curname == nextp)
+ return false; /* empty unquoted name not allowed */
+
+ /*
+ * Downcase the identifier, using same code as main lexer does.
+ *
+ * XXX because we want to overwrite the input in-place, we cannot
+ * support a downcasing transformation that increases the string
+ * length. This is not a problem given the current implementation
+ * of downcase_truncate_identifier, but we'll probably have to do
+ * something about this someday.
+ */
+ len = endp - curname;
+ downname = downcase_truncate_identifier(curname, len, false);
+ Assert(strlen(downname) <= len);
+ strncpy(curname, downname, len); /* strncpy is required here */
+ pfree(downname);
+ }
+
+ while (scanner_isspace(*nextp))
+ nextp++; /* skip trailing whitespace */
+
+ if (*nextp == separator)
+ {
+ nextp++;
+ while (scanner_isspace(*nextp))
+ nextp++; /* skip leading whitespace for next */
+ /* we expect another name, so done remains false */
+ }
+ else if (*nextp == '\0')
+ done = true;
+ else
+ return false; /* invalid syntax */
+
+ /* Now safe to overwrite separator with a null */
+ *endp = '\0';
+
+ /* Truncate name if it's overlength */
+ truncate_identifier(curname, strlen(curname), false);
+
+ /*
+ * Finished isolating current name --- add it to list
+ */
+ *namelist = lappend(*namelist, curname);
+
+ /* Loop back if we didn't reach end of string */
+ } while (!done);
+
+ return true;
+}
+
char *
remove_quotes_and_schema_from_relname(char *table)
{
static char rel[MAX_ITEM_LENGTH];
- char *p;
- int i = 0;
+ char *rawstring;
+ List *names;
- /* get rid of schema name */
- p = strchr(table, '.');
- if (p)
- table = p + 1;
-
- /* get rid of quotation marks */
- for (i = 0; *table; table++)
+ rawstring = pstrdup(table);
+ if(SplitIdentifierString(rawstring, '.', (Node **) &names) && names != NIL)
+ {
+ /*
+ * Since table name is always the last one in the list,
+ * we use llast() to get table name.
+ */
+ strlcpy(rel, llast(names), sizeof(rel));
+ }
+ else
{
- if (*table != '"')
- rel[i++] = *table;
+ rel[0] = '\0';
}
- rel[i] = '\0';
+
+ pfree(rawstring);
+ list_free(names);
return rel;
}
diff --git a/src/utils/pool_select_walker.c b/src/utils/pool_select_walker.c
index 2765e5609..40b09099d 100644
--- a/src/utils/pool_select_walker.c
+++ b/src/utils/pool_select_walker.c
@@ -339,22 +339,14 @@ function_call_walker(Node *node, void *context)
if (length > 0)
{
- if (length == 1) /* no schema qualification? */
- {
- fname = strVal(linitial(fcall->funcname));
- }
- else
- {
- fname = strVal(lsecond(fcall->funcname)); /* with schema
- * qualification */
- }
+ fname = make_function_name_from_funccall(fcall);
ereport(DEBUG1,
(errmsg("function call walker, function name: \"%s\"", fname)));
- check_object_relationship_list(fname, true);
+ check_object_relationship_list(strVal(llast(fcall->funcname)), true);
- if (ctx->pg_terminate_backend_pid == 0 && strcmp("pg_terminate_backend", fname) == 0)
+ if (ctx->pg_terminate_backend_pid == 0 && strcmp("pg_terminate_backend", strVal(llast(fcall->funcname))) == 0)
{
if (list_length(fcall->args) == 1)
{
@@ -997,15 +989,7 @@ non_immutable_function_call_walker(Node *node, void *context)
if (length > 0)
{
- if (length == 1) /* no schema qualification? */
- {
- fname = strVal(linitial(fcall->funcname));
- }
- else
- {
- fname = strVal(lsecond(fcall->funcname)); /* with schema
- * qualification */
- }
+ fname = make_function_name_from_funccall(fcall);
ereport(DEBUG1,
(errmsg("non immutable function walker. checking function \"%s\"", fname)));
@@ -1047,20 +1031,75 @@ is_immutable_function(char *fname)
/*
* Query to know if the function is IMMUTABLE
*/
-#define IS_STABLE_FUNCTION_QUERY "SELECT count(*) FROM pg_catalog.pg_proc AS p WHERE p.proname = '%s' AND p.provolatile = 'i'"
+#define IS_STABLE_FUNCTION_QUERY "SELECT count(*) FROM pg_catalog.pg_proc AS p, pg_catalog.pg_namespace AS n WHERE p.proname %s '%s' AND n.oid = p.pronamespace AND n.nspname = '%s' AND p.provolatile = 'i'"
bool result;
- static POOL_RELCACHE * relcache;
- POOL_CONNECTION_POOL *backend;
+ char query[1024];
+ char *rawstring = NULL;
+ char *key_str = NULL;
+ List *names = NIL;
+ POOL_CONNECTION_POOL *backend;
+ static POOL_RELCACHE *relcache;
+
+ /* We need a modifiable copy of the input string. */
+ rawstring = pstrdup(fname);
+
+ /* split "schemaname.funcname" */
+ if(!SplitIdentifierString(rawstring, '.', (Node **) &names) ||
+ names == NIL)
+ {
+ if(rawstring)
+ pfree(rawstring);
+ if(names)
+ list_free(names);
+
+ ereport(WARNING,
+ (errmsg("invalid function name %s", fname)));
+
+ return false;
+ }
+
+ /* with schema qualification */
+ if(list_length(names) == 2)
+ {
+ key_str = fname;
+ if(!relcache)
+ {
+ snprintf(query, sizeof(query), IS_STABLE_FUNCTION_QUERY,
+ "=", (char *) lsecond(names), (char *) linitial(names));
+ }
+ else
+ {
+ snprintf(relcache->sql, sizeof(relcache->sql), IS_STABLE_FUNCTION_QUERY,
+ "=", (char *) lsecond(names), (char *) linitial(names));
+ }
+ }
+ else
+ {
+ key_str = (char *) llast(names);
+ if(!relcache)
+ {
+ snprintf(query, sizeof(query), IS_STABLE_FUNCTION_QUERY,
+ "~", ".*", (char *) llast(names));
+ }
+ else
+ {
+ snprintf(relcache->sql, sizeof(relcache->sql), IS_STABLE_FUNCTION_QUERY,
+ "~", ".*", (char *) llast(names));
+ }
+ }
backend = pool_get_session_context(false)->backend;
if (!relcache)
{
- relcache = pool_create_relcache(pool_config->relcache_size, IS_STABLE_FUNCTION_QUERY,
+ relcache = pool_create_relcache(pool_config->relcache_size, query,
int_register_func, int_unregister_func,
false);
if (relcache == NULL)
{
+ pfree(rawstring);
+ list_free(names);
+
ereport(WARNING,
(errmsg("unable to create relcache, while checking if the function is immutable")));
return false;
@@ -1070,7 +1109,10 @@ is_immutable_function(char *fname)
errdetail("relcache created")));
}
- result = (pool_search_relcache(relcache, backend, fname) == 0) ? 0 : 1;
+ result = (pool_search_relcache(relcache, backend, key_str) == 0) ? 0 : 1;
+
+ pfree(rawstring);
+ list_free(names);
ereport(DEBUG1,
(errmsg("checking if the function is IMMUTABLE"),
@@ -1256,6 +1298,75 @@ makeRangeVarFromNameList(List *names)
return rel;
}
+/*
+ * Extract function name from FuncCall. Make schema qualification name if
+ * necessary. The returned function name is in static area. So next
+ * call to this function will break previous result.
+ */
+char *
+make_function_name_from_funccall(FuncCall *fcall)
+{
+ /*
+ * Function name. Max size is calculated as follows: schema
+ * name(POOL_NAMEDATALEN byte) + quotation marks for schmea name(2 byte) +
+ * period(1 byte) + table name (POOL_NAMEDATALEN byte) + quotation marks
+ * for table name(2 byte) + NULL(1 byte)
+ */
+ static char funcname[POOL_NAMEDATALEN * 2 + 1 + 2 * 2 + 1];
+ List *names;
+
+ if(fcall == NULL)
+ {
+ ereport(WARNING,
+ (errmsg("FuncCall argument is NULL, while getting function name from FuncCall")));
+ return "";
+ }
+
+ *funcname = '\0';
+ names = fcall->funcname;
+
+ switch (list_length(names))
+ {
+ case 1:
+ strcat(funcname, "\"");
+ strncat(funcname, strVal(linitial(names)), POOL_NAMEDATALEN);
+ strcat(funcname, "\"");
+ break;
+ case 2:
+ strcat(funcname, "\"");
+ strncat(funcname, strVal(linitial(names)), POOL_NAMEDATALEN);
+ strcat(funcname, "\"");
+
+ strcat(funcname, ".");
+
+ strcat(funcname, "\"");
+ strncat(funcname, strVal(lsecond(names)), POOL_NAMEDATALEN);
+ strcat(funcname, "\"");
+ break;
+ case 3:
+ strcat(funcname, "\"");
+ strncat(funcname, strVal(lsecond(names)), POOL_NAMEDATALEN);
+ strcat(funcname, "\"");
+
+ strcat(funcname, ".");
+
+ strcat(funcname, "\"");
+ strncat(funcname, strVal(lthird(names)), POOL_NAMEDATALEN);
+ strcat(funcname, "\"");
+
+ break;
+ default:
+ ereport(WARNING,
+ (errmsg("invalid function name, too many indirections, while getting function name from FuncCall")));
+ break;
+ }
+
+ ereport(DEBUG1,
+ (errmsg("make function name from funccall: funcname:\"%s\"", funcname)));
+
+ return funcname;
+}
+
/*
* Extract table name from RageVar. Make schema qualification name if
* necessary. The returned table name is in static area. So next
[parent: 4a1af4e3ba31]