[pgpool-hackers: 2805] Re: [New feature] Enable specifying SQL patterns lists that should not be load-balanced.

Tatsuo Ishii ishii at sraoss.co.jp
Wed May 23 09:48:15 JST 2018


Peng,

Thanks for the new patch!

> +特定の SQL をプライマリノードに送信するように <xref linkend="guc-black-query-pattern-list">を設定します。SQL パターンをセミコロン区切りで指定します。マスタースレーブモードのみで動作します。

Please insert a new line between each sentence.

> +static char **get_list_from_string_regex_delim(const char *input, const char *delimi, int *n)
> +{
> +#ifndef POOL_PRIVATE
> +    int j = 0;
> +    char *output;
> +    char *str;
> +    char *buf;
> +    char **tokens;
> +	const int MAXTOKENS = 256;
> +	*n = 0;
> +
> +	if (input == NULL || *input == '\0')
> +		return NULL;
> +
> +	tokens = palloc(MAXTOKENS * sizeof(char *));
> +	if (*(input + strlen(input) - 1) != *delimi)
> +	{
> +		str = palloc(strlen(input) + 1);
> +		sprintf(str, "%s;", input);

Use of sprintf is not always recommended because there could be a
chance for memory overrun. Instead, please use snprintf.

> --- a/src/include/pool_config.h
> +++ b/src/include/pool_config.h
> @@ -181,10 +181,11 @@ typedef struct {
>  	char **reset_query_list;		/* comma separated list of queries to be issued at the end of session */
>  	char **white_function_list;		/* list of functions with no side effects */
>  	char **black_function_list;		/* list of functions with side effects */
> +	char **black_query_pattern_list;	/* list of query patterns that should be sent to primary node */
>  	char *log_line_prefix;			/* printf-style string to output at beginning of each log line */
> -    int log_error_verbosity;		/* controls how much detail about error should be emitted */

Why do you touch irrelevant portion of the existing code? If your
intention is fixing exiting indentation, I recommend you to do it as a
separate commit.

> --- a/src/utils/pool_select_walker.c
> +++ b/src/utils/pool_select_walker.c
> @@ -203,6 +203,11 @@ int pattern_compare(char *str, const int type, const char *param_name)
>  		lists_patterns = pool_config->lists_memqcache_table_patterns;
>  		pattc = &pool_config->memqcache_table_pattc;
>  
> +	} else if (strcmp(param_name, "black_query_pattern_list") == 0)

This is not a recommended coding style in this project. Please write
like below.

}
else if (strcmp(param_name, "black_query_pattern_list") == 0)

> +	{
> +		lists_patterns = pool_config->lists_query_patterns;
> +		pattc = &pool_config->query_pattc;
> +
>  	} else {

This is also needed to be fixed by the same reason above. But since
you did not code it, it's not your fault of course. It is up to you if
you want to fix this as well.

>  		ereport(WARNING,
>  				(errmsg("pattern_compare: unknown paramname %s", param_name)));

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


More information about the pgpool-hackers mailing list