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

Tatsuo Ishii ishii at sraoss.co.jp
Mon Jun 4 11:26:11 JST 2018


Peng,

Looks good to me. Please go ahead and commit/push it.

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

> Ishii-san,
> 
> Thank you for reviewing my patch.
> I fixed that and attached the new patch.
> 
> On Wed, 23 May 2018 09:48:15 +0900 (JST)
> Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> 
>> 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
>> 
> 
> 
> -- 
> Bo Peng <pengbo at sraoss.co.jp>
> SRA OSS, Inc. Japan


More information about the pgpool-hackers mailing list