pgbench - test whether a variable exists

Started by Fabien COELHOalmost 8 years ago18 messages

coelho@cri.ensmp.fr

almost 8 years ago

1 attachment(s)

Hello devs,

While investigating moving pgbench expressions to fe_utils so that they
can be shared with psql (\if ..., \let ?), I figure out that psql's \if
has a syntax to test whether a variable exists, which is not yet available
to pgbench.

This patch adds the same syntax to pgbench expression so that they can
represent this expression, already accepted by psql's \if.

Note that it is not really that useful for benchmarking, although it does
not harm.

--
Fabien.

Attachments:

pgbench-var-exists-1.patchtext/x-diff; name=pgbench-var-exists-1.patchDownload

diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 3dd492c..33d58d5 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -909,6 +909,8 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
       integer constants such as <literal>5432</literal>,
       double constants such as <literal>3.14159</literal>,
       references to variables <literal>:</literal><replaceable>variablename</replaceable>,
+      testing whether a variable exists
+      <literal>:{?</literal><replaceable>variablename</replaceable><literal>}</literal>,
       <link linkend="pgbench-builtin-operators">operators</link>
       with their usual SQL precedence and associativity,
       <link linkend="pgbench-builtin-functions">function calls</link>,
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index e23ca51..ea25b0e 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -24,6 +24,7 @@ static PgBenchExpr *make_boolean_constant(bool bval);
 static PgBenchExpr *make_integer_constant(int64 ival);
 static PgBenchExpr *make_double_constant(double dval);
 static PgBenchExpr *make_variable(char *varname);
+static PgBenchExpr *make_variable_exists(char *varname);
 static PgBenchExpr *make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr);
 static PgBenchExpr *make_uop(yyscan_t yyscanner, const char *operator, PgBenchExpr *expr);
@@ -55,9 +56,10 @@ static PgBenchExpr *make_case(yyscan_t yyscanner, PgBenchExprList *when_then_lis
 %type <ival> INTEGER_CONST function
 %type <dval> DOUBLE_CONST
 %type <bval> BOOLEAN_CONST
-%type <str> VARIABLE FUNCTION
+%type <str> VARIABLE VAREXISTS FUNCTION
 
-%token NULL_CONST INTEGER_CONST DOUBLE_CONST BOOLEAN_CONST VARIABLE FUNCTION
+%token NULL_CONST INTEGER_CONST DOUBLE_CONST BOOLEAN_CONST
+%token VARIABLE VAREXISTS FUNCTION
 %token AND_OP OR_OP NOT_OP NE_OP LE_OP GE_OP LS_OP RS_OP IS_OP
 %token CASE_KW WHEN_KW THEN_KW ELSE_KW END_KW
 
@@ -136,6 +138,7 @@ expr: '(' expr ')'			{ $$ = $2; }
 	| DOUBLE_CONST			{ $$ = make_double_constant($1); }
 	/* misc */
 	| VARIABLE				{ $$ = make_variable($1); }
+	| VAREXISTS				{ $$ = make_variable_exists($1); }
 	| function '(' elist ')' { $$ = make_func(yyscanner, $1, $3); }
 	| case_control			{ $$ = $1; }
 	;
@@ -209,6 +212,16 @@ make_variable(char *varname)
 
 /* binary operators */
 static PgBenchExpr *
+make_variable_exists(char *varname)
+{
+	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
+
+	expr->etype = ENODE_VAREXISTS;
+	expr->u.variable.varname = varname;
+	return expr;
+}
+
+static PgBenchExpr *
 make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr)
 {
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 5c1bd88..ff2b586 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -191,6 +191,13 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 					yylval->bval = false;
 					return BOOLEAN_CONST;
 				}
+:\{\?{alnum}+\}	{
+					/* no pg_strndup? */
+					yylval->str = pg_strdup(yytext + 3);
+					/* scratch final '}' */
+					yylval->str[strlen(yylval->str)-1] = '\0';
+					return VAREXISTS;
+				}
 {digit}+		{
 					yylval->ival = strtoint64(yytext);
 					return INTEGER_CONST;
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d420942..b236116 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2264,6 +2264,10 @@ evaluateExpr(TState *thread, CState *st, PgBenchExpr *expr, PgBenchValue *retval
 				return true;
 			}
 
+		case ENODE_VAREXISTS:
+				setBoolValue(retval, lookupVariable(st, expr->u.variable.varname) != NULL);
+				return true;
+
 		case ENODE_FUNCTION:
 			return evalFunc(thread, st,
 							expr->u.function.function,
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index 0705ccd..0609802 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -58,6 +58,7 @@ typedef enum PgBenchExprType
 {
 	ENODE_CONSTANT,
 	ENODE_VARIABLE,
+	ENODE_VAREXISTS,
 	ENODE_FUNCTION
 } PgBenchExprType;
 
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 99286f6..10c1553 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -259,6 +259,8 @@ pgbench(
 		qr{command=46.: int 46\b},
 		qr{command=47.: boolean true\b},
 		qr{command=48.: boolean true\b},
+		qr{command=58.: boolean false\b}, # var exists
+		qr{command=59.: boolean true\b},
 	],
 	'pgbench expressions',
 	{   '001_pgbench_expressions' => q{-- integer functions
@@ -338,6 +340,9 @@ pgbench(
 \set v2 5432
 \set v3 -54.21E-2
 SELECT :v0, :v1, :v2, :v3;
+-- test variable existence
+\set no debug(:{?no_such_variable})
+\set yes debug(:{?cs})
 } });
 
 # backslash commands

Andres Freund

andres@anarazel.de

almost 8 years ago

In reply to: Fabien COELHO (#1)

Re: pgbench - test whether a variable exists

Hi,

On 2018-02-19 19:23:04 +0100, Fabien COELHO wrote:

Note that it is not really that useful for benchmarking, although it does
not harm.

That seems plenty reason to plainly reject this patch? If we end up
unifying it'll be added via that, no?

- Andres

Fabien COELHO

coelho@cri.ensmp.fr

almost 8 years ago

In reply to: Andres Freund (#2)

Re: pgbench - test whether a variable exists

Note that it is not really that useful for benchmarking, although it does
not harm.

That seems plenty reason to plainly reject this patch? If we end up
unifying it'll be added via that, no?

On the contrary, my point is to add the feature beforehand so that it is
not intermixed in the large refactoring/renamings/interfacing involved in
moving pgbench expression (lexer, parser, execution) engine to front-end
utils.

--
Fabien.

Alvaro Herrera

alvherre@alvh.no-ip.org

almost 8 years ago

In reply to: Fabien COELHO (#1)

Re: pgbench - test whether a variable exists

re. pg_strndup() the following places could use it (skim of a very quick grep
for pg_strdup):

src/bin/pg_waldump/pg_waldump.c: *dir = pg_strdup(path);
src/bin/pg_waldump/pg_waldump.c- (*dir)[(sep - path) + 1] = '\0'; /* no strndup */

src/bin/psql/prompt.c: name = pg_strdup(p + 1);
src/bin/psql/prompt.c- nameend = strcspn(name, ":");
src/bin/psql/prompt.c- name[nameend] = '\0';

src/bin/scripts/common.c: *table = pg_strdup(spec);
src/bin/scripts/common.c- (*table)[cp - spec] = '\0'; /* no strndup */

Not a lot. It takes more code to add pg_strndup() that we would save by
adding it.

.oO(I wonder why strcspn and not strrchr ...)

--
ï¿½lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Fabien COELHO

coelho@cri.ensmp.fr

almost 8 years ago

In reply to: Fabien COELHO (#1)

1 attachment(s)

Re: pgbench - test whether a variable exists

While investigating moving pgbench expressions to fe_utils so that they can
be shared with psql (\if ..., \let ?), I figure out that psql's \if has a
syntax to test whether a variable exists, which is not yet available to
pgbench.

This patch adds the same syntax to pgbench expression so that they can
represent this expression, already accepted by psql's \if.

Note that it is not really that useful for benchmarking, although it does not
harm.

Patch v2 is a rebase.

--
Fabien.

Attachments:

pgbench-var-exists-2.patchtext/plain; name=pgbench-var-exists-2.patchDownload

diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index d52d324..5092628 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -929,6 +929,8 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
       integer constants such as <literal>5432</literal>,
       double constants such as <literal>3.14159</literal>,
       references to variables <literal>:</literal><replaceable>variablename</replaceable>,
+      testing whether a variable exists
+      <literal>:{?</literal><replaceable>variablename</replaceable><literal>}</literal>,
       <link linkend="pgbench-builtin-operators">operators</link>
       with their usual SQL precedence and associativity,
       <link linkend="pgbench-builtin-functions">function calls</link>,
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 8447e14..2f89142 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -28,6 +28,7 @@ static PgBenchExpr *make_boolean_constant(bool bval);
 static PgBenchExpr *make_integer_constant(int64 ival);
 static PgBenchExpr *make_double_constant(double dval);
 static PgBenchExpr *make_variable(char *varname);
+static PgBenchExpr *make_variable_exists(char *varname);
 static PgBenchExpr *make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr);
 static PgBenchExpr *make_uop(yyscan_t yyscanner, const char *operator, PgBenchExpr *expr);
@@ -59,9 +60,10 @@ static PgBenchExpr *make_case(yyscan_t yyscanner, PgBenchExprList *when_then_lis
 %type <ival> INTEGER_CONST function
 %type <dval> DOUBLE_CONST
 %type <bval> BOOLEAN_CONST
-%type <str> VARIABLE FUNCTION
+%type <str> VARIABLE VAREXISTS FUNCTION
 
-%token NULL_CONST INTEGER_CONST DOUBLE_CONST BOOLEAN_CONST VARIABLE FUNCTION
+%token NULL_CONST INTEGER_CONST DOUBLE_CONST BOOLEAN_CONST
+%token VARIABLE VAREXISTS FUNCTION
 %token AND_OP OR_OP NOT_OP NE_OP LE_OP GE_OP LS_OP RS_OP IS_OP
 %token CASE_KW WHEN_KW THEN_KW ELSE_KW END_KW
 
@@ -140,6 +142,7 @@ expr: '(' expr ')'			{ $$ = $2; }
 	| DOUBLE_CONST			{ $$ = make_double_constant($1); }
 	/* misc */
 	| VARIABLE				{ $$ = make_variable($1); }
+	| VAREXISTS				{ $$ = make_variable_exists($1); }
 	| function '(' elist ')' { $$ = make_func(yyscanner, $1, $3); }
 	| case_control			{ $$ = $1; }
 	;
@@ -213,6 +216,16 @@ make_variable(char *varname)
 
 /* binary operators */
 static PgBenchExpr *
+make_variable_exists(char *varname)
+{
+	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
+
+	expr->etype = ENODE_VAREXISTS;
+	expr->u.variable.varname = varname;
+	return expr;
+}
+
+static PgBenchExpr *
 make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr)
 {
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 5c1bd88..ff2b586 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -191,6 +191,13 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 					yylval->bval = false;
 					return BOOLEAN_CONST;
 				}
+:\{\?{alnum}+\}	{
+					/* no pg_strndup? */
+					yylval->str = pg_strdup(yytext + 3);
+					/* scratch final '}' */
+					yylval->str[strlen(yylval->str)-1] = '\0';
+					return VAREXISTS;
+				}
 {digit}+		{
 					yylval->ival = strtoint64(yytext);
 					return INTEGER_CONST;
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 894571e..e08d268 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2356,6 +2356,10 @@ evaluateExpr(TState *thread, CState *st, PgBenchExpr *expr, PgBenchValue *retval
 				return true;
 			}
 
+		case ENODE_VAREXISTS:
+				setBoolValue(retval, lookupVariable(st, expr->u.variable.varname) != NULL);
+				return true;
+
 		case ENODE_FUNCTION:
 			return evalFunc(thread, st,
 							expr->u.function.function,
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index 6983865..284443e 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -58,6 +58,7 @@ typedef enum PgBenchExprType
 {
 	ENODE_CONSTANT,
 	ENODE_VARIABLE,
+	ENODE_VAREXISTS,
 	ENODE_FUNCTION
 } PgBenchExprType;
 
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 7448a96..f1032f2 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -270,6 +270,8 @@ pgbench(
 		qr{command=86.: int 86\b},
 		qr{command=93.: int 93\b},
 		qr{command=95.: int 0\b},
+		qr{command=96.: boolean false\b}, # var exists
+		qr{command=97.: boolean true\b},
 	],
 	'pgbench expressions',
 	{   '001_pgbench_expressions' => q{-- integer functions
@@ -390,6 +392,9 @@ SELECT :v0, :v1, :v2, :v3;
 \endif
 -- must be zero if false branches where skipped
 \set nope debug(:nope)
+-- test variable existence
+\set no debug(:{?no_such_variable})
+\set yes debug(:{?cs})
 } });
 
 # backslash commands

Fabien COELHO

coelho@cri.ensmp.fr

almost 8 years ago

In reply to: Fabien COELHO (#5)

1 attachment(s)

Re: pgbench - test whether a variable exists

Patch v2 is a rebase.

Patch v3 is also a rebase.

--
Fabien.

Attachments:

pgbench-var-exists-3.patchtext/plain; name=pgbench-var-exists-3.patchDownload

diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 41d9030..7b3badf 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -971,6 +971,8 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
       integer constants such as <literal>5432</literal>,
       double constants such as <literal>3.14159</literal>,
       references to variables <literal>:</literal><replaceable>variablename</replaceable>,
+      testing whether a variable exists
+      <literal>:{?</literal><replaceable>variablename</replaceable><literal>}</literal>,
       <link linkend="pgbench-builtin-operators">operators</link>
       with their usual SQL precedence and associativity,
       <link linkend="pgbench-builtin-functions">function calls</link>,
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 8447e14..2f89142 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -28,6 +28,7 @@ static PgBenchExpr *make_boolean_constant(bool bval);
 static PgBenchExpr *make_integer_constant(int64 ival);
 static PgBenchExpr *make_double_constant(double dval);
 static PgBenchExpr *make_variable(char *varname);
+static PgBenchExpr *make_variable_exists(char *varname);
 static PgBenchExpr *make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr);
 static PgBenchExpr *make_uop(yyscan_t yyscanner, const char *operator, PgBenchExpr *expr);
@@ -59,9 +60,10 @@ static PgBenchExpr *make_case(yyscan_t yyscanner, PgBenchExprList *when_then_lis
 %type <ival> INTEGER_CONST function
 %type <dval> DOUBLE_CONST
 %type <bval> BOOLEAN_CONST
-%type <str> VARIABLE FUNCTION
+%type <str> VARIABLE VAREXISTS FUNCTION
 
-%token NULL_CONST INTEGER_CONST DOUBLE_CONST BOOLEAN_CONST VARIABLE FUNCTION
+%token NULL_CONST INTEGER_CONST DOUBLE_CONST BOOLEAN_CONST
+%token VARIABLE VAREXISTS FUNCTION
 %token AND_OP OR_OP NOT_OP NE_OP LE_OP GE_OP LS_OP RS_OP IS_OP
 %token CASE_KW WHEN_KW THEN_KW ELSE_KW END_KW
 
@@ -140,6 +142,7 @@ expr: '(' expr ')'			{ $$ = $2; }
 	| DOUBLE_CONST			{ $$ = make_double_constant($1); }
 	/* misc */
 	| VARIABLE				{ $$ = make_variable($1); }
+	| VAREXISTS				{ $$ = make_variable_exists($1); }
 	| function '(' elist ')' { $$ = make_func(yyscanner, $1, $3); }
 	| case_control			{ $$ = $1; }
 	;
@@ -213,6 +216,16 @@ make_variable(char *varname)
 
 /* binary operators */
 static PgBenchExpr *
+make_variable_exists(char *varname)
+{
+	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
+
+	expr->etype = ENODE_VAREXISTS;
+	expr->u.variable.varname = varname;
+	return expr;
+}
+
+static PgBenchExpr *
 make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr)
 {
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 5c1bd88..ff2b586 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -191,6 +191,13 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 					yylval->bval = false;
 					return BOOLEAN_CONST;
 				}
+:\{\?{alnum}+\}	{
+					/* no pg_strndup? */
+					yylval->str = pg_strdup(yytext + 3);
+					/* scratch final '}' */
+					yylval->str[strlen(yylval->str)-1] = '\0';
+					return VAREXISTS;
+				}
 {digit}+		{
 					yylval->ival = strtoint64(yytext);
 					return INTEGER_CONST;
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 8529e7d..3ddceffe 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2360,6 +2360,10 @@ evaluateExpr(TState *thread, CState *st, PgBenchExpr *expr, PgBenchValue *retval
 				return true;
 			}
 
+		case ENODE_VAREXISTS:
+				setBoolValue(retval, lookupVariable(st, expr->u.variable.varname) != NULL);
+				return true;
+
 		case ENODE_FUNCTION:
 			return evalFunc(thread, st,
 							expr->u.function.function,
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index 6983865..284443e 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -58,6 +58,7 @@ typedef enum PgBenchExprType
 {
 	ENODE_CONSTANT,
 	ENODE_VARIABLE,
+	ENODE_VAREXISTS,
 	ENODE_FUNCTION
 } PgBenchExprType;
 
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 0929418..f6c2f53 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -283,6 +283,8 @@ pgbench(
 		qr{command=96.: int 1\b},    # :scale
 		qr{command=97.: int 0\b},    # :client_id
 		qr{command=98.: int 5432\b}, # :random_seed
+		qr{command=99.: boolean false\b}, # var exists
+		qr{command=100.: boolean true\b},
 	],
 	'pgbench expressions',
 	{   '001_pgbench_expressions' => q{-- integer functions
@@ -407,6 +409,9 @@ SELECT :v0, :v1, :v2, :v3;
 \set sc debug(:scale)
 \set ci debug(:client_id)
 \set rs debug(:random_seed)
+-- test variable existence
+\set no debug(:{?no_such_variable})
+\set yes debug(:{?cs})
 } });
 
 # random determinism when seeded

Michael Paquier

michael@paquier.xyz

over 7 years ago

In reply to: Fabien COELHO (#6)

Re: pgbench - test whether a variable exists

On Tue, Mar 27, 2018 at 02:08:31PM +0200, Fabien COELHO wrote:

Patch v2 is a rebase.

Patch v3 is also a rebase.

This has rotten for half a year, so I am marking it as returned with
feedback. There have been comments from Alvaro and Andres as well...
--
Michael

Fabien COELHO

coelho@cri.ensmp.fr

almost 6 years ago

In reply to: Michael Paquier (#7)

1 attachment(s)

Re: pgbench - test whether a variable exists

Bonjour Michaël,

Patch v3 is also a rebase.

This has rotten for half a year, so I am marking it as returned with
feedback. There have been comments from Alvaro and Andres as well...

Attached a v4. I'm resurrecting this small patch, after "\aset" has been
added to pgbench (9d8ef988).

Alvaro's feedback was about the lack of "pg_strndup" availability, but he
concluded that it would seldom be used if available, so it was not worth
the effort to add it for the sake of this small patch.

About arguments for this patch:

First, this syntax is already available in "psql", and I think that
keeping pgbench/psql in sync is better for users' ease of mind. That is
the initial (weak) argument about which Andres objected.

Second, it is on the path to move pgbench expression as a front-end util
that can be used by psql, which is still a project of mine, although I
have not started much on that yet. For that pgbench expressions must be
able to do what psql can do before merging, including this test. Other
things needed before this is stated are a working free on expression trees
(trivial), merging variable management to some extent (at least the same
API, possibly the same implementation would save quite a few lines of
code), having string values, support for :'var' and :"var" string escapes…
no very big deals, but some work anyway.

Third, I have a practical pgbench-specific use case, which motivates the
resurrection right now: I'd like to be able to run a benchmark with a mix
of SELECT, UPDATE, DELETE and INSERT commands, that is expected in a
normal functioning database system.

For INSERT, I think I have a few ideas for possible and simple solutions,
but it still need some thoughts so this is for later. The key issue is
how to handle a varying number of rows.

Under DELETE, some SELECT and UPDATE scripts may fail because the data are
not there anymore, hence the ability to check whether a variable is empty
comes handy:

Low probability delete script:

\set uid random(...)
DELETE FROM Operations WHERE oid IN (... :uid) \;
DELETE FROM Accounts WHERE aid IN (... :uid) \;
DELETE FROM Client WHERE ... :uid;

Parallel running update script:

-- a pseudo random client arrives
\set uname random(...)
SELECT uid FROM Client WHERE uname = :uname::TEXT \aset
-- remainder must be skipped if no client was found
\if :{?uid}
SELECT SUM(abalance) FROM Account WHERE uid = :uid ...
-- if the balance is right, withdrawing is possible...
...
\else
-- skip silently, the client has left!
\endif

--
Fabien.

Attachments:

pgbench-var-exists-4.patchtext/x-diff; NAME=pgbench-var-exists-4.patchDownload

diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 58a2aa3bf2..1f375c4167 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1133,6 +1133,8 @@ SELECT 4 AS four \; SELECT 5 AS five \aset
       integer constants such as <literal>5432</literal>,
       double constants such as <literal>3.14159</literal>,
       references to variables <literal>:</literal><replaceable>variablename</replaceable>,
+      testing whether a variable exists
+      <literal>:{?</literal><replaceable>variablename</replaceable><literal>}</literal>,
       <link linkend="pgbench-builtin-operators">operators</link>
       with their usual SQL precedence and associativity,
       <link linkend="pgbench-builtin-functions">function calls</link>,
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 85d61caa9f..dc6d419e50 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -28,6 +28,7 @@ static PgBenchExpr *make_boolean_constant(bool bval);
 static PgBenchExpr *make_integer_constant(int64 ival);
 static PgBenchExpr *make_double_constant(double dval);
 static PgBenchExpr *make_variable(char *varname);
+static PgBenchExpr *make_variable_exists(char *varname);
 static PgBenchExpr *make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr);
 static PgBenchExpr *make_uop(yyscan_t yyscanner, const char *operator, PgBenchExpr *expr);
@@ -59,10 +60,10 @@ static PgBenchExpr *make_case(yyscan_t yyscanner, PgBenchExprList *when_then_lis
 %type <ival> INTEGER_CONST function
 %type <dval> DOUBLE_CONST
 %type <bval> BOOLEAN_CONST
-%type <str> VARIABLE FUNCTION
+%type <str> VARIABLE VAREXISTS FUNCTION
 
 %token NULL_CONST INTEGER_CONST MAXINT_PLUS_ONE_CONST DOUBLE_CONST
-%token BOOLEAN_CONST VARIABLE FUNCTION
+%token BOOLEAN_CONST VARIABLE VAREXISTS FUNCTION
 %token AND_OP OR_OP NOT_OP NE_OP LE_OP GE_OP LS_OP RS_OP IS_OP
 %token CASE_KW WHEN_KW THEN_KW ELSE_KW END_KW
 
@@ -144,6 +145,7 @@ expr: '(' expr ')'			{ $$ = $2; }
 	| DOUBLE_CONST			{ $$ = make_double_constant($1); }
 	/* misc */
 	| VARIABLE				{ $$ = make_variable($1); }
+	| VAREXISTS				{ $$ = make_variable_exists($1); }
 	| function '(' elist ')' { $$ = make_func(yyscanner, $1, $3); }
 	| case_control			{ $$ = $1; }
 	;
@@ -216,6 +218,16 @@ make_variable(char *varname)
 }
 
 /* binary operators */
+static PgBenchExpr *
+make_variable_exists(char *varname)
+{
+	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
+
+	expr->etype = ENODE_VAREXISTS;
+	expr->u.variable.varname = varname;
+	return expr;
+}
+
 static PgBenchExpr *
 make_op(yyscan_t yyscanner, const char *operator,
 		PgBenchExpr *lexpr, PgBenchExpr *rexpr)
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 430bff38a6..254bebf300 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -204,6 +204,13 @@ notnull			[Nn][Oo][Tt][Nn][Uu][Ll][Ll]
 					 */
 					return MAXINT_PLUS_ONE_CONST;
 				}
+:\{\?{alnum}+\}	{
+					/* no pg_strndup? */
+					yylval->str = pg_strdup(yytext + 3);
+					/* scratch final '}' */
+					yylval->str[strlen(yylval->str)-1] = '\0';
+					return VAREXISTS;
+				}
 {digit}+		{
 					if (!strtoint64(yytext, true, &yylval->ival))
 						expr_yyerror_more(yyscanner, "bigint constant overflow",
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e99af80167..8cc9e57d9d 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2451,6 +2451,10 @@ evaluateExpr(CState *st, PgBenchExpr *expr, PgBenchValue *retval)
 				return true;
 			}
 
+		case ENODE_VAREXISTS:
+				setBoolValue(retval, lookupVariable(st, expr->u.variable.varname) != NULL);
+				return true;
+
 		case ENODE_FUNCTION:
 			return evalFunc(st,
 							expr->u.function.function,
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index fb2c34f512..18575ea2b6 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -58,6 +58,7 @@ typedef enum PgBenchExprType
 {
 	ENODE_CONSTANT,
 	ENODE_VARIABLE,
+	ENODE_VAREXISTS,
 	ENODE_FUNCTION
 } PgBenchExprType;
 
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index e85728c379..1a75df0c5f 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -459,6 +459,8 @@ pgbench(
 		qr{command=98.: int 5432\b},                    # :random_seed
 		qr{command=99.: int -9223372036854775808\b},    # min int
 		qr{command=100.: int 9223372036854775807\b},    # max int
+		qr{command=101.: boolean false\b}, # var exists
+		qr{command=102.: boolean true\b},
 	],
 	'pgbench expressions',
 	{
@@ -586,6 +588,9 @@ SELECT :v0, :v1, :v2, :v3;
 -- minint constant parsing
 \set min debug(-9223372036854775808)
 \set max debug(-(:min + 1))
+-- test variable existence
+\set no debug(:{?no_such_variable})
+\set yes debug(:{?cs})
 }
 	});

Michael Paquier

michael@paquier.xyz

over 5 years ago

In reply to: Fabien COELHO (#8)

Re: pgbench - test whether a variable exists

On Mon, Apr 13, 2020 at 09:54:01AM +0200, Fabien COELHO wrote:

Attached a v4. I'm resurrecting this small patch, after "\aset" has been
added to pgbench (9d8ef988).

Hmm. It seems to me that this stuff needs to be more careful with the
function handling? For example, all those cases fail but they
directly pass down a variable that may not be defined, so shouldn't
those results be undefined as well instead of failing:
\set g double(:{?no_such_variable})
\set g exp(:{?no_such_variable})
\set g greatest(:{?no_such_variable}, :{?no_such_variable})
\set g int(:{?no_such_variable})

It seems to me that there could be a point in having the result of any
function to become undefined if using at least one undefined argument
(the point could be made as well that things like greatest just ignore
conditioned variables), so I was surprised to not see the logic more
linked with ENODE_VARIABLE. If your intention is to keep this
behavior, it should at least be tested I guess. Please note that this
patch will have to wait until v14 opens for business for more
comments. :p
--
Michael

#10

Fabien COELHO

coelho@cri.ensmp.fr

over 5 years ago

In reply to: Michael Paquier (#9)

Re: pgbench - test whether a variable exists

Bonjour Michaël,

Hmm. It seems to me that this stuff needs to be more careful with the
function handling? For example, all those cases fail but they directly
pass down a variable that may not be defined, so shouldn't those results
be undefined as well instead of failing:

\set g double(:{?no_such_variable})
\set g exp(:{?no_such_variable})
\set g greatest(:{?no_such_variable}, :{?no_such_variable})
\set g int(:{?no_such_variable})

I do not understand: All the above examples are type errors, as ":{?var}"
is a boolean, that cannot be cast to double, be exponentiated etc. So
failing just seems appropriate.

Maybe casting boolean to int should be allowed, though, as pg does it.

It seems to me that there could be a point in having the result of any
function to become undefined if using at least one undefined argument
(the point could be made as well that things like greatest just ignore
conditioned variables), so I was surprised to not see the logic more
linked with ENODE_VARIABLE.

Hmmm… :var (ENODE_VARIABLE) replaces de variable by its value, and it
fails if the variable is not defined, which is the intention, hence the
point of having the ability to test whether a variable exists, and its new
expression node type.

It could replace it by NULL when not existing, but ISTM that a script can
wish to distinguish NULL and undefined, and it departs from SQL which just
fails on a undefined alias or column or whatever.

If someone wants to go back to having a self propagating NULL, they can
simply

\if :{?var}
\set var NULL
\endif

\set var CASE WHEN :{?var} THEN :var ELSE NULL END

to get it.

Having some special UNDEF value looks unattractive, as it would depart for
SQL even further.

If your intention is to keep this behavior, it should at least be tested
I guess.

My intention is to keep failing on type errors, but maybe I'm missing
something of your point.

Please note that this patch will have to wait until v14 opens
for business for more.

Sure. I put it in the July CF, and I do not expect to it to be processed
on the first CF it appears in.

--
Fabien.

#11

Ibrar Ahmed

ibrar.ahmad@gmail.com

about 5 years ago

In reply to: Fabien COELHO (#10)

Re: pgbench - test whether a variable exists

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: tested, failed
Documentation: not tested

I am not very convinced to have that, but I have performed some testing on master ().

I found a crash

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `pgbench postgres -T 60 -f a.sql'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000055bfb572a4b2 in expr_yylex (yylval_param=0x7fff29a60870, yyscanner=0x55bfb6867a90) at exprscan.c:1107
1107 *yy_cp = yyg->yy_hold_char;
(gdb) bt
#0 0x000055bfb572a4b2 in expr_yylex (yylval_param=0x7fff29a60870, yyscanner=0x55bfb6867a90) at exprscan.c:1107
#1 0x000055bfb572cb63 in expr_lex_one_word (state=0x55bfb6867a10, word_buf=0x7fff29a608d0, offset=0x7fff29a608a8) at exprscan.l:340
#2 0x000055bfb57358fd in process_backslash_command (sstate=0x55bfb6867a10, source=0x55bfb68653a0 "a.sql") at pgbench.c:4540
#3 0x000055bfb5736528 in ParseScript (
script=0x55bfb68655f0 "\\set nbranches :scale\n\\set ntellers 10 * :scale\n\\set naccounts 100000 * :scale\n\\setrandom aid 1 :naccounts\n\\setrandom bid 1 :nbranches\n\\setrandom tid 1 :ntellers\n\\setrandom delta -5000 5000\nBEGIN;\nUPD"..., desc=0x55bfb68653a0 "a.sql", weight=1) at pgbench.c:4826
#4 0x000055bfb5736967 in process_file (filename=0x55bfb68653a0 "a.sql", weight=1) at pgbench.c:4962
#5 0x000055bfb5738628 in main (argc=6, argv=0x7fff29a610d8) at pgbench.c:5641

The new status of this patch is: Waiting on Author

#12

Anastasia Lubennikova

a.lubennikova@postgrespro.ru

about 5 years ago

In reply to: Ibrar Ahmed (#11)

Re: pgbench - test whether a variable exists

On 20.10.2020 15:22, Ibrar Ahmed wrote:

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: tested, failed
Documentation: not tested

I am not very convinced to have that, but I have performed some testing on master ().

I found a crash

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `pgbench postgres -T 60 -f a.sql'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000055bfb572a4b2 in expr_yylex (yylval_param=0x7fff29a60870, yyscanner=0x55bfb6867a90) at exprscan.c:1107
1107 *yy_cp = yyg->yy_hold_char;
(gdb) bt
#0 0x000055bfb572a4b2 in expr_yylex (yylval_param=0x7fff29a60870, yyscanner=0x55bfb6867a90) at exprscan.c:1107
#1 0x000055bfb572cb63 in expr_lex_one_word (state=0x55bfb6867a10, word_buf=0x7fff29a608d0, offset=0x7fff29a608a8) at exprscan.l:340
#2 0x000055bfb57358fd in process_backslash_command (sstate=0x55bfb6867a10, source=0x55bfb68653a0 "a.sql") at pgbench.c:4540
#3 0x000055bfb5736528 in ParseScript (
script=0x55bfb68655f0 "\\set nbranches :scale\n\\set ntellers 10 * :scale\n\\set naccounts 100000 * :scale\n\\setrandom aid 1 :naccounts\n\\setrandom bid 1 :nbranches\n\\setrandom tid 1 :ntellers\n\\setrandom delta -5000 5000\nBEGIN;\nUPD"..., desc=0x55bfb68653a0 "a.sql", weight=1) at pgbench.c:4826
#4 0x000055bfb5736967 in process_file (filename=0x55bfb68653a0 "a.sql", weight=1) at pgbench.c:4962
#5 0x000055bfb5738628 in main (argc=6, argv=0x7fff29a610d8) at pgbench.c:5641

The new status of this patch is: Waiting on Author

This patch was inactive during the commitfest, so I am going to mark it
as "Returned with Feedback".
Fabien, are you planning to continue working on it?

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#13

Fabien COELHO

coelho@cri.ensmp.fr

about 5 years ago

In reply to: Anastasia Lubennikova (#12)

Re: pgbench - test whether a variable exists

The new status of this patch is: Waiting on Author

This patch was inactive during the commitfest, so I am going to mark it as
"Returned with Feedback".
Fabien, are you planning to continue working on it?

Not in the short term, but probably for the next CF. Can you park it
there?

--
Fabien.

#14

Michael Paquier

michael@paquier.xyz

about 5 years ago

In reply to: Fabien COELHO (#13)

Re: pgbench - test whether a variable exists

On Mon, Nov 30, 2020 at 12:53:20PM +0100, Fabien COELHO wrote:

Not in the short term, but probably for the next CF. Can you park it there?

IMO, I think that it would be better to leave this item marked as RwF
for now if you are not sure, and register it again in the CF once
there is an actual new patch. This approach makes for less bloat in
the CF app.
--
Michael

#15

Anastasia Lubennikova

a.lubennikova@postgrespro.ru

about 5 years ago

In reply to: Fabien COELHO (#13)

Re: pgbench - test whether a variable exists

On 30.11.2020 14:53, Fabien COELHO wrote:

The new status of this patch is: Waiting on Author

This patch was inactive during the commitfest, so I am going to mark
it as "Returned with Feedback".
Fabien, are you planning to continue working on it?

Not in the short term, but probably for the next CF. Can you park it
there?

Sure, I'll move it to the next CF then.
I also noticed, that the first message mentions the idea of refactoring
to use some code it in both pgbench and psql code. Can you, please,
share a link to the thread, if it exists?

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#16

Fabien COELHO

coelho@cri.ensmp.fr

about 5 years ago

In reply to: Anastasia Lubennikova (#15)

Re: pgbench - test whether a variable exists

Sure, I'll move it to the next CF then. I also noticed, that the first
message mentions the idea of refactoring to use some code it in both
pgbench and psql code. Can you, please, share a link to the thread, if
it exists?

Unsure. I'll try to find it if it exists.

--
Fabien.

#17

Fabien COELHO

coelho@cri.ensmp.fr

about 5 years ago

In reply to: Michael Paquier (#14)

Re: pgbench - test whether a variable exists

Bonjour Michaï¿œl,

On Mon, Nov 30, 2020 at 12:53:20PM +0100, Fabien COELHO wrote:

Not in the short term, but probably for the next CF. Can you park it there?

IMO, I think that it would be better to leave this item marked as RwF
for now if you are not sure, and register it again in the CF once
there is an actual new patch. This approach makes for less bloat in
the CF app.

Yep.

Although I was under water this Fall, I should have more time available in
January, so I think I'll do something about it. Now if it is RWF by then,
I'll resubmit it.

--
Fabien.

#18

Alvaro Herrera

alvherre@alvh.no-ip.org

about 5 years ago

In reply to: Fabien COELHO (#8)

Re: pgbench - test whether a variable exists

On 2020-Apr-13, Fabien COELHO wrote:

Alvaro's feedback was about the lack of "pg_strndup" availability, but he
concluded that it would seldom be used if available, so it was not worth the
effort to add it for the sake of this small patch.

Hello. Please note that as is often the case, I was wrong about this
point; I ended up adding pnstrdup in commit 0b9466fce2cf so you should
be able to use it for this now.