Re: [HACKERS] pgbench - allow backslash continuations in \set expressions

[Fabien COELHO]

Hello Rafia,

Well with this new approach, the example you gave previously for better
readability:

\set bid
     CASE WHEN random(0, 99) < 85
       THEN :tbid
       ELSE :abid + (:abid >= :tbid)
     END

will give error at the first line.

Indeed you are right for the patch I sent, but it is ok if the initial state is COEX, i.e. it does not allow an empty expression.

In general, this new approach is likely to create confusions in such cases.

See attached version.

As an end-user one needs to be real careful to check what portions have to split between lines. Keeping this in mind, I'd prefer the previous approach.

--
Fabien.
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 285608d..f066be1 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -826,13 +826,17 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
       <literal>%</>) with their usual precedence and associativity,
       <link linkend="pgbench-builtin-functions">function calls</>, and
       parentheses.
+      Expressions can spead several lines till completed: the parsing is
+      pursued if a token at the end of the line cannot end an expression
+      or if there is an unclosed parenthesis.
      </para>
 
      <para>
       Examples:
 <programlisting>
 \set ntellers 10 * :scale
-\set aid (1021 * random(1, 100000 * :scale)) % (100000 * :scale) + 1
+\set aid (1021 * random(1, 100000 * :scale)) %
+           (100000 * :scale) + 1
 </programlisting></para>
     </listitem>
    </varlistentry>
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 20891a3..0d6fcd6 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -43,6 +43,16 @@ static bool last_was_newline = false;
 extern int	expr_yyget_column(yyscan_t yyscanner);
 extern void expr_yyset_column(int column_no, yyscan_t yyscanner);
 
+/* the expression cannot end on this token */
+#define TO_COEX					   \
+	cur_state->start_state = COEX; \
+	BEGIN(COEX)
+
+/* continuation if unclosed parentheses */
+#define TO_EXPR															\
+	cur_state->start_state = cur_state->paren_depth > 0 ? COEX : EXPR;	\
+	BEGIN(cur_state->start_state)
+
 %}
 
 /* Except for the prefix, these options should match psqlscan.l */
@@ -67,7 +77,7 @@ nonspace		[^ \t\r\f\v\n]
 newline			[\n]
 
 /* Exclusive states */
-%x EXPR
+%x EXPR COEX
 
 %%
 
@@ -104,46 +114,64 @@ newline			[\n]
 					return 0;
 				}
 
+	/* COEX (continued expression) state */
+
+<COEX>{
+
+{newline}		{ /* ignore */ }
+
+}
+
 	/* EXPR state */
 
 <EXPR>{
 
-"+"				{ return '+'; }
-"-"				{ return '-'; }
-"*"				{ return '*'; }
-"/"				{ return '/'; }
-"%"				{ return '%'; }
-"("				{ return '('; }
-")"				{ return ')'; }
-","				{ return ','; }
+{newline}		{
+					/* report end of command */
+					last_was_newline = true;
+					return 0;
+				}
+}
+
+	/* EXPR & COEX states common rules */
+
+<EXPR,COEX>{
+
+"+"				{ TO_COEX; return '+'; }
+"-"				{ TO_COEX; return '-'; }
+"*"				{ TO_COEX; return '*'; }
+"/"				{ TO_COEX; return '/'; }
+"%"				{ TO_COEX; return '%'; }
+"("				{ cur_state->paren_depth++; TO_COEX; return '('; }
+")"				{ cur_state->paren_depth--; TO_EXPR; return ')'; }
+","				{ TO_COEX; return ','; }
 
 :{alnum}+		{
 					yylval->str = pg_strdup(yytext + 1);
+					TO_EXPR;
 					return VARIABLE;
 				}
 {digit}+		{
 					yylval->ival = strtoint64(yytext);
+					TO_EXPR;
 					return INTEGER_CONST;
 				}
 {digit}+(\.{digit}*)?([eE][-+]?{digit}+)?	{
 					yylval->dval = atof(yytext);
+					TO_EXPR;
 					return DOUBLE_CONST;
 				}
 \.{digit}+([eE][-+]?{digit}+)?	{
 					yylval->dval = atof(yytext);
+					TO_EXPR;
 					return DOUBLE_CONST;
 				}
 {alpha}{alnum}*	{
 					yylval->str = pg_strdup(yytext);
+					TO_COEX;
 					return FUNCTION;
 				}
 
-{newline}		{
-					/* report end of command */
-					last_was_newline = true;
-					return 0;
-				}
-
 {space}+		{ /* ignore */ }
 
 .				{
@@ -289,7 +317,7 @@ expr_scanner_init(PsqlScanState state,
 		yy_switch_to_buffer(state->scanbufhandle, state->scanner);
 
 	/* Set start state */
-	state->start_state = EXPR;
+	state->start_state = COEX;
 
 	return state->scanner;
 }

I will not fight over this one. I like it in "scala", though, and I find it rather elegant, especially as backslashes are quite ugly. Another reason not to take it is that it would be much harder to have that in psql as well.

cont2.sql
Description: application/sql

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers