Hi

2014-10-14 22:57 GMT+02:00 Petr Jelinek <p...@2ndquadrant.com>:

> On 09/09/14 17:37, Pavel Stehule wrote:
>
>> Ada is language with strong character, and PLpgSQL is little bit strange
>> fork - so it isn't easy to find some form, how to solve all requirements.
>>
>> My requests:
>>
>> * be consistent with current PLpgSQL syntax and logic
>> * allow some future extensibility
>> * allow a static analyses without hard expression processing
>>
>> But I am thinking so there are some points where can be some agreement -
>> although it is not ASSERT implementation.
>>
>> enhancing RAISE WHEN - please, see attached patch -
>>
>> I prefer RAISE WHEN again RAISE WHERE due consistency with EXIT and
>> CONTINUE [ WHEN ];
>>
>>
> Short review of the patch. Note that this has nothing to do with actual
> assertions, at the moment it's just enhancement of RAISE statement to make
> it optionally conditional. As I was one of the people who voted for it I do
> think we want this and I like the syntax.
>
> Code applies cleanly, seems formatted according to project standards -
> there is unnecessary whitespace added in variable declaration part of
> exec_stmt_raise which should be removed.
>

fixed


>
> Passes make check, I would prefer to have little more complex expression
> than just "true" in the new regression test added for this feature.
>

fixed

>
> Did some manual testing, seems to work as advertised.
>
> There are no docs at the moment which is the only show-stopper that I can
> see so far.
>

fixed

Regards

Pavel


>
> --
>  Petr Jelinek                  http://www.2ndQuadrant.com/
>
>  PostgreSQL Development, 24x7 Support, Training & Services
>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index f195495..eae72f6
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** END LOOP <optional> <replaceable>label</
*** 3369,3379 ****
      raise errors.
  
  <synopsis>
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> '<replaceable class="parameter">format</replaceable>' <optional>, <replaceable class="parameter">expression</replaceable> <optional>, ... </optional></optional> <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional>;
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> <replaceable class="parameter">condition_name</> <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional>;
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> SQLSTATE '<replaceable class="parameter">sqlstate</>' <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional>;
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional>;
! RAISE ;
  </synopsis>
  
      The <replaceable class="parameter">level</replaceable> option specifies
--- 3369,3379 ----
      raise errors.
  
  <synopsis>
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> '<replaceable class="parameter">format</replaceable>' <optional>, <replaceable class="parameter">expression</replaceable> <optional>, ... </optional></optional> <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional> <optional> WHEN <replaceable class="parameter">boolean-expression</replaceable> </optional>;
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> <replaceable class="parameter">condition_name</> <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional> <optional> WHEN <replaceable class="parameter">boolean-expression</replaceable> </optional>;
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> SQLSTATE '<replaceable class="parameter">sqlstate</>' <optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> </optional> <optional> WHEN <replaceable class="parameter">boolean-expression</replaceable> </optional>;
! RAISE <optional> <replaceable class="parameter">level</replaceable> </optional> USING <replaceable class="parameter">option</replaceable> = <replaceable class="parameter">expression</replaceable> <optional>, ... </optional> <optional> WHEN <replaceable class="parameter">boolean-expression</replaceable> </optional>;
! RAISE <optional> WHEN <replaceable class="parameter">boolean-expression</replaceable> </optional>;
  </synopsis>
  
      The <replaceable class="parameter">level</replaceable> option specifies
*************** RAISE unique_violation USING MESSAGE = '
*** 3548,3553 ****
--- 3548,3561 ----
      </para>
     </note>
  
+    <para>
+      If <literal>WHEN</literal> is specified, the message or error is raised
+      only if <replaceable>boolean-expression</> is true.
+ <programlisting>
+ RAISE 'Unexpected NULL in varible: name' WHEN name IS NULL;
+ </programlisting>
+    </para>
+ 
   </sect1>
  
   <sect1 id="plpgsql-trigger">
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
new file mode 100644
index 11cb47b..1c44f85
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** exec_stmt_raise(PLpgSQL_execstate *estat
*** 2892,2897 ****
--- 2892,2908 ----
  	char	   *err_schema = NULL;
  	ListCell   *lc;
  
+ 	if (stmt->cond != NULL)
+ 	{
+ 		bool	value;
+ 		bool	isnull;
+ 
+ 		value = exec_eval_boolean(estate, stmt->cond, &isnull);
+ 		exec_eval_cleanup(estate);
+ 		if (isnull || value == false)
+ 			return PLPGSQL_RC_OK;
+ 	}
+ 
  	/* RAISE with no parameters: re-throw current exception */
  	if (stmt->condname == NULL && stmt->message == NULL &&
  		stmt->options == NIL)
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
new file mode 100644
index 893f3a4..1f0b861
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
*************** static	void			current_token_is_not_varia
*** 63,68 ****
--- 63,69 ----
  static	PLpgSQL_expr	*read_sql_construct(int until,
  											int until2,
  											int until3,
+ 											int until4,
  											const char *expected,
  											const char *sqlstart,
  											bool isexpression,
*************** static	void			 check_labels(const char *
*** 105,111 ****
  									  int end_location);
  static	PLpgSQL_expr	*read_cursor_args(PLpgSQL_var *cursor,
  										  int until, const char *expected);
! static	List			*read_raise_options(void);
  static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
  
  %}
--- 106,112 ----
  									  int end_location);
  static	PLpgSQL_expr	*read_cursor_args(PLpgSQL_var *cursor,
  										  int until, const char *expected);
! static	List			*read_raise_options(int *endtok);
  static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
  
  %}
*************** for_control		: for_variable K_IN
*** 1422,1427 ****
--- 1423,1429 ----
  							expr1 = read_sql_construct(DOT_DOT,
  													   K_LOOP,
  													   0,
+ 													   0,
  													   "LOOP",
  													   "SELECT ",
  													   true,
*************** stmt_raise		: K_RAISE
*** 1716,1721 ****
--- 1718,1724 ----
  					{
  						PLpgSQL_stmt_raise		*new;
  						int	tok;
+ 						int	endtok;
  
  						new = palloc(sizeof(PLpgSQL_stmt_raise));
  
*************** stmt_raise		: K_RAISE
*** 1726,1731 ****
--- 1729,1735 ----
  						new->message	= NULL;
  						new->params		= NIL;
  						new->options	= NIL;
+ 						new->cond = NULL;
  
  						tok = yylex();
  						if (tok == 0)
*************** stmt_raise		: K_RAISE
*** 1796,1817 ****
  								 * or USING to begin the options list.
  								 */
  								tok = yylex();
! 								if (tok != ',' && tok != ';' && tok != K_USING)
  									yyerror("syntax error");
  
  								while (tok == ',')
  								{
  									PLpgSQL_expr *expr;
  
! 									expr = read_sql_construct(',', ';', K_USING,
! 															  ", or ; or USING",
  															  "SELECT ",
  															  true, true, true,
  															  NULL, &tok);
  									new->params = lappend(new->params, expr);
  								}
  							}
! 							else if (tok != K_USING)
  							{
  								/* must be condition name or SQLSTATE */
  								if (tok_is_keyword(tok, &yylval,
--- 1800,1821 ----
  								 * or USING to begin the options list.
  								 */
  								tok = yylex();
! 								if (tok != ',' && tok != ';' && tok != K_USING && tok != K_WHEN)
  									yyerror("syntax error");
  
  								while (tok == ',')
  								{
  									PLpgSQL_expr *expr;
  
! 									expr = read_sql_construct(',', ';', K_USING, K_WHEN,
! 															  ", or ; or USING or WHEN",
  															  "SELECT ",
  															  true, true, true,
  															  NULL, &tok);
  									new->params = lappend(new->params, expr);
  								}
  							}
! 							else if (tok != K_USING && tok != K_WHEN)
  							{
  								/* must be condition name or SQLSTATE */
  								if (tok_is_keyword(tok, &yylval,
*************** stmt_raise		: K_RAISE
*** 1847,1853 ****
  							}
  
  							if (tok == K_USING)
! 								new->options = read_raise_options();
  						}
  
  						check_raise_parameters(new);
--- 1851,1863 ----
  							}
  
  							if (tok == K_USING)
! 							{
! 								new->options = read_raise_options(&endtok);
! 								if (endtok == K_WHEN)
! 									new->cond = read_sql_expression(';', ";");
! 							}
! 							if (tok == K_WHEN)
! 								new->cond = read_sql_expression(';', ";");
  						}
  
  						check_raise_parameters(new);
*************** stmt_dynexecute : K_EXECUTE
*** 1906,1912 ****
  						PLpgSQL_expr *expr;
  						int endtoken;
  
! 						expr = read_sql_construct(K_INTO, K_USING, ';',
  												  "INTO or USING or ;",
  												  "SELECT ",
  												  true, true, true,
--- 1916,1922 ----
  						PLpgSQL_expr *expr;
  						int endtoken;
  
! 						expr = read_sql_construct(K_INTO, K_USING, ';', 0,
  												  "INTO or USING or ;",
  												  "SELECT ",
  												  true, true, true,
*************** stmt_dynexecute : K_EXECUTE
*** 1945,1951 ****
  									yyerror("syntax error");
  								do
  								{
! 									expr = read_sql_construct(',', ';', K_INTO,
  															  ", or ; or INTO",
  															  "SELECT ",
  															  true, true, true,
--- 1955,1961 ----
  									yyerror("syntax error");
  								do
  								{
! 									expr = read_sql_construct(',', ';', K_INTO, 0,
  															  ", or ; or INTO",
  															  "SELECT ",
  															  true, true, true,
*************** current_token_is_not_variable(int tok)
*** 2456,2462 ****
  static PLpgSQL_expr *
  read_sql_expression(int until, const char *expected)
  {
! 	return read_sql_construct(until, 0, 0, expected,
  							  "SELECT ", true, true, true, NULL, NULL);
  }
  
--- 2466,2472 ----
  static PLpgSQL_expr *
  read_sql_expression(int until, const char *expected)
  {
! 	return read_sql_construct(until, 0, 0, 0, expected,
  							  "SELECT ", true, true, true, NULL, NULL);
  }
  
*************** static PLpgSQL_expr *
*** 2465,2471 ****
  read_sql_expression2(int until, int until2, const char *expected,
  					 int *endtoken)
  {
! 	return read_sql_construct(until, until2, 0, expected,
  							  "SELECT ", true, true, true, NULL, endtoken);
  }
  
--- 2475,2481 ----
  read_sql_expression2(int until, int until2, const char *expected,
  					 int *endtoken)
  {
! 	return read_sql_construct(until, until2, 0, 0, expected,
  							  "SELECT ", true, true, true, NULL, endtoken);
  }
  
*************** read_sql_expression2(int until, int unti
*** 2473,2479 ****
  static PLpgSQL_expr *
  read_sql_stmt(const char *sqlstart)
  {
! 	return read_sql_construct(';', 0, 0, ";",
  							  sqlstart, false, true, true, NULL, NULL);
  }
  
--- 2483,2489 ----
  static PLpgSQL_expr *
  read_sql_stmt(const char *sqlstart)
  {
! 	return read_sql_construct(';', 0, 0, 0, ";",
  							  sqlstart, false, true, true, NULL, NULL);
  }
  
*************** static PLpgSQL_expr *
*** 2496,2501 ****
--- 2506,2512 ----
  read_sql_construct(int until,
  				   int until2,
  				   int until3,
+ 				   int until4,
  				   const char *expected,
  				   const char *sqlstart,
  				   bool isexpression,
*************** read_sql_construct(int until,
*** 2529,2534 ****
--- 2540,2547 ----
  			break;
  		if (tok == until3 && parenlevel == 0)
  			break;
+ 		if (tok == until4 && parenlevel == 0)
+ 			break;
  		if (tok == '(' || tok == '[')
  			parenlevel++;
  		else if (tok == ')' || tok == ']')
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3647,3653 ****
  		 * translated into a form where the second parameter is commented
  		 * out.
  		 */
! 		item = read_sql_construct(',', ')', 0,
  								  ",\" or \")",
  								  sqlstart,
  								  true, true,
--- 3660,3666 ----
  		 * translated into a form where the second parameter is commented
  		 * out.
  		 */
! 		item = read_sql_construct(',', ')', 0, 0,
  								  ",\" or \")",
  								  sqlstart,
  								  true, true,
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3711,3724 ****
   * Parse RAISE ... USING options
   */
  static List *
! read_raise_options(void)
  {
  	List	   *result = NIL;
  
  	for (;;)
  	{
  		PLpgSQL_raise_option *opt;
- 		int		tok;
  
  		if ((tok = yylex()) == 0)
  			yyerror("unexpected end of function definition");
--- 3724,3737 ----
   * Parse RAISE ... USING options
   */
  static List *
! read_raise_options(int *endtok)
  {
  	List	   *result = NIL;
+ 	int		tok;
  
  	for (;;)
  	{
  		PLpgSQL_raise_option *opt;
  
  		if ((tok = yylex()) == 0)
  			yyerror("unexpected end of function definition");
*************** read_raise_options(void)
*** 3759,3772 ****
  		if (tok != '=' && tok != COLON_EQUALS)
  			yyerror("syntax error, expected \"=\"");
  
! 		opt->expr = read_sql_expression2(',', ';', ", or ;", &tok);
  
  		result = lappend(result, opt);
  
! 		if (tok == ';')
  			break;
  	}
  
  	return result;
  }
  
--- 3772,3788 ----
  		if (tok != '=' && tok != COLON_EQUALS)
  			yyerror("syntax error, expected \"=\"");
  
! 		opt->expr = read_sql_construct(',', ';', K_WHEN, 0, ", or ; or WHEN",
! 									   "SELECT ", true, true, true, NULL, &tok);
  
  		result = lappend(result, opt);
  
! 		if (tok == ';' || tok == K_WHEN)
  			break;
  	}
  
+ 	*endtok = tok;
+ 
  	return result;
  }
  
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
new file mode 100644
index d6f31ff..3dee7f3
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** typedef struct
*** 621,626 ****
--- 621,627 ----
  	char	   *message;		/* old-style message format literal, or NULL */
  	List	   *params;			/* list of expressions for old-style message */
  	List	   *options;		/* list of PLpgSQL_raise_option */
+ 	PLpgSQL_expr *cond;
  } PLpgSQL_stmt_raise;
  
  typedef struct
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
new file mode 100644
index 983f1b8..c149450
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** NOTICE:  outer_func() done
*** 5351,5353 ****
--- 5351,5388 ----
  drop function outer_outer_func(int);
  drop function outer_func(int);
  drop function inner_func(int);
+ create or replace function test_func(text)
+ returns void as $$
+ begin
+   raise notice 'Hello %', $1;
+   raise notice 'Hello %', $1 when false;
+   raise notice 'Hello % %', $1, $1 when true;
+   raise notice 'Hello % % %', $1, $2, $3 when null;
+   
+   raise notice using message = 'Nazdar';
+   raise notice using message = 'Nazdar ' || $1 when true;
+   raise notice using message = 'Nazdar ' || $1, detail = 'Svete' when true;
+ 
+   for i in 1..8
+   loop
+     raise notice '% is even number', i when i % 2 = 0;
+   end loop;
+ end;
+ $$ language plpgsql;
+ select test_func('world');
+ NOTICE:  Hello world
+ NOTICE:  Hello world world
+ NOTICE:  Nazdar
+ NOTICE:  Nazdar world
+ NOTICE:  Nazdar world
+ DETAIL:  Svete
+ NOTICE:  2 is even number
+ NOTICE:  4 is even number
+ NOTICE:  6 is even number
+ NOTICE:  8 is even number
+  test_func 
+ -----------
+  
+ (1 row)
+ 
+ drop function test_func(text);
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
new file mode 100644
index 2abcbc8..2675da6
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select outer_outer_func(20);
*** 4198,4200 ****
--- 4198,4223 ----
  drop function outer_outer_func(int);
  drop function outer_func(int);
  drop function inner_func(int);
+ 
+ create or replace function test_func(text)
+ returns void as $$
+ begin
+   raise notice 'Hello %', $1;
+   raise notice 'Hello %', $1 when false;
+   raise notice 'Hello % %', $1, $1 when true;
+   raise notice 'Hello % % %', $1, $2, $3 when null;
+   
+   raise notice using message = 'Nazdar';
+   raise notice using message = 'Nazdar ' || $1 when true;
+   raise notice using message = 'Nazdar ' || $1, detail = 'Svete' when true;
+ 
+   for i in 1..8
+   loop
+     raise notice '% is even number', i when i % 2 = 0;
+   end loop;
+ end;
+ $$ language plpgsql;
+ 
+ select test_func('world');
+ 
+ drop function test_func(text);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to