[ new subject, new thread, new patch ]
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2021-Jun-16, Tom Lane wrote:
>> BTW, as long as we're thinking of back-patching nontrivial specfile
>> changes, I have another modest proposal. What do people think of
>> removing the requirement for step/session names to be double-quoted,
>> and instead letting them work like SQL identifiers?
> Yes *please*.
Here's a draft patch for that. I converted one specfile just as
proof-of-concept, but I don't want to touch the rest until the other
patch has gone in, or I'll have merge problems. (This'll have some
merge problems with that anyway I fear, but they'll be minor.)
I decided to follow the standard SQL rule that you can use "foo""bar"
to include a double-quote in a quoted identifier. This broke one
place in test_decoding's oldest_xmin.spec where somebody had left out
a space. So maybe there's an argument for not doing that --- but I'd
rather not document more inconsistencies than I have to.
regards, tom lane
diff --git a/contrib/test_decoding/specs/oldest_xmin.spec b/contrib/test_decoding/specs/oldest_xmin.spec
index da3a8cd512..88bd30f5ff 100644
--- a/contrib/test_decoding/specs/oldest_xmin.spec
+++ b/contrib/test_decoding/specs/oldest_xmin.spec
@@ -39,4 +39,4 @@ step "s1_commit" { COMMIT; }
# composite type is a rare form of DDL which allows T1 to see the tuple which
# will be removed (xmax set) before T1 commits. That is, interlocking doesn't
# forbid modifying catalog after someone read it (and didn't commit yet).
-permutation "s0_begin" "s0_getxid" "s1_begin" "s1_insert" "s0_alter" "s0_commit" "s0_checkpoint" "s0_get_changes"
"s0_get_changes""s1_commit""s0_vacuum" "s0_get_changes"
+permutation "s0_begin" "s0_getxid" "s1_begin" "s1_insert" "s0_alter" "s0_commit" "s0_checkpoint" "s0_get_changes"
"s0_get_changes""s1_commit" "s0_vacuum" "s0_get_changes"
diff --git a/src/test/isolation/README b/src/test/isolation/README
index 6ae7152325..439b1d32b2 100644
--- a/src/test/isolation/README
+++ b/src/test/isolation/README
@@ -75,7 +75,7 @@ teardown { <SQL> }
this to clean up in preparation for the next permutation, e.g dropping
any test tables created by setup. This part is optional.
-session "<name>"
+session <name>
There are normally several "session" parts in a spec file. Each
session is executed in its own connection. A session part consists
@@ -86,13 +86,13 @@ session "<name>"
Each step has the syntax
- step "<name>" { <SQL> }
+ step <name> { <SQL> }
where <name> is a name identifying this step, and SQL is a SQL statement
(or statements, separated by semicolons) that is executed in the step.
Step names must be unique across the whole spec file.
-permutation "<step name>" ...
+permutation <step name> ...
A permutation line specifies a list of steps that are run in that order.
Any number of permutation lines can appear. If no permutation lines are
@@ -103,6 +103,13 @@ permutation "<step name>" ...
available steps; it could for instance repeat some steps more than once,
or leave others out.
+Session and step names are SQL identifiers or quoted identifiers.
+A difference from standard SQL is that no case-folding occurs, so
+FOO and "FOO" are the same name while FOO and Foo are different,
+whether you quote them or not. You must use double quotes if you
+want to use an isolation test keyword (such as "permutation") as
+a name.
+
Lines beginning with a # are considered comments.
For each permutation of the session steps (whether these are manually
diff --git a/src/test/isolation/specparse.y b/src/test/isolation/specparse.y
index ae3a999961..efdf333b00 100644
--- a/src/test/isolation/specparse.y
+++ b/src/test/isolation/specparse.y
@@ -39,12 +39,12 @@ TestSpec parseresult; /* result of parsing is left here */
%type <str> opt_setup opt_teardown
%type <str> setup
%type <ptr_list> step_list session_list permutation_list opt_permutation_list
-%type <ptr_list> string_literal_list
+%type <ptr_list> identifier_list
%type <session> session
%type <step> step
%type <permutation> permutation
-%token <str> sqlblock string_literal
+%token <str> sqlblock identifier
%token PERMUTATION SESSION SETUP STEP TEARDOWN TEST
%%
@@ -111,7 +111,7 @@ session_list:
;
session:
- SESSION string_literal opt_setup step_list opt_teardown
+ SESSION identifier opt_setup step_list opt_teardown
{
$$ = pg_malloc(sizeof(Session));
$$->name = $2;
@@ -140,7 +140,7 @@ step_list:
step:
- STEP string_literal sqlblock
+ STEP identifier sqlblock
{
$$ = pg_malloc(sizeof(Step));
$$->name = $2;
@@ -180,7 +180,7 @@ permutation_list:
permutation:
- PERMUTATION string_literal_list
+ PERMUTATION identifier_list
{
$$ = pg_malloc(sizeof(Permutation));
$$->stepnames = (char **) $2.elements;
@@ -188,15 +188,15 @@ permutation:
}
;
-string_literal_list:
- string_literal_list string_literal
+identifier_list:
+ identifier_list identifier
{
$$.elements = pg_realloc($1.elements,
($1.nelements + 1) * sizeof(void *));
$$.elements[$1.nelements] = $2;
$$.nelements = $1.nelements + 1;
}
- | string_literal
+ | identifier
{
$$.nelements = 1;
$$.elements = pg_malloc(sizeof(void *));
diff --git a/src/test/isolation/specs/aborted-keyrevoke.spec b/src/test/isolation/specs/aborted-keyrevoke.spec
index 08945d8b31..4f6f9027cc 100644
--- a/src/test/isolation/specs/aborted-keyrevoke.spec
+++ b/src/test/isolation/specs/aborted-keyrevoke.spec
@@ -17,30 +17,30 @@ teardown
DROP TABLE foo;
}
-session "s1"
+session s1
setup { BEGIN; }
-step "s1s" { SAVEPOINT f; }
-step "s1u" { UPDATE foo SET key = 2; } # obtain KEY REVOKE
-step "s1r" { ROLLBACK TO f; } # lose KEY REVOKE
-step "s1l" { SELECT * FROM foo FOR KEY SHARE; }
-step "s1c" { COMMIT; }
+step s1s { SAVEPOINT f; }
+step s1u { UPDATE foo SET key = 2; } # obtain KEY REVOKE
+step s1r { ROLLBACK TO f; } # lose KEY REVOKE
+step s1l { SELECT * FROM foo FOR KEY SHARE; }
+step s1c { COMMIT; }
-session "s2"
+session s2
setup { BEGIN; }
-step "s2l" { SELECT * FROM foo FOR KEY SHARE; }
-step "s2c" { COMMIT; }
+step s2l { SELECT * FROM foo FOR KEY SHARE; }
+step s2c { COMMIT; }
-permutation "s1s" "s1u" "s1r" "s1l" "s1c" "s2l" "s2c"
-permutation "s1s" "s1u" "s1r" "s1l" "s2l" "s1c" "s2c"
-permutation "s1s" "s1u" "s1r" "s1l" "s2l" "s2c" "s1c"
-permutation "s1s" "s1u" "s1r" "s2l" "s1l" "s1c" "s2c"
-permutation "s1s" "s1u" "s1r" "s2l" "s1l" "s2c" "s1c"
-permutation "s1s" "s1u" "s1r" "s2l" "s2c" "s1l" "s1c"
-permutation "s1s" "s1u" "s2l" "s1r" "s1l" "s1c" "s2c"
-permutation "s1s" "s1u" "s2l" "s1r" "s1l" "s2c" "s1c"
-permutation "s1s" "s1u" "s2l" "s1r" "s2c" "s1l" "s1c"
-permutation "s1s" "s2l" "s1u" "s2c" "s1r" "s1l" "s1c"
-permutation "s1s" "s2l" "s2c" "s1u" "s1r" "s1l" "s1c"
-permutation "s2l" "s1s" "s1u" "s2c" "s1r" "s1l" "s1c"
-permutation "s2l" "s1s" "s2c" "s1u" "s1r" "s1l" "s1c"
-permutation "s2l" "s2c" "s1s" "s1u" "s1r" "s1l" "s1c"
+permutation s1s s1u s1r s1l s1c s2l s2c
+permutation s1s s1u s1r s1l s2l s1c s2c
+permutation s1s s1u s1r s1l s2l s2c s1c
+permutation s1s s1u s1r s2l s1l s1c s2c
+permutation s1s s1u s1r s2l s1l s2c s1c
+permutation s1s s1u s1r s2l s2c s1l s1c
+permutation s1s s1u s2l s1r s1l s1c s2c
+permutation s1s s1u s2l s1r s1l s2c s1c
+permutation s1s s1u s2l s1r s2c s1l s1c
+permutation s1s s2l s1u s2c s1r s1l s1c
+permutation s1s s2l s2c s1u s1r s1l s1c
+permutation s2l s1s s1u s2c s1r s1l s1c
+permutation s2l s1s s2c s1u s1r s1l s1c
+permutation s2l s2c s1s s1u s1r s1l s1c
diff --git a/src/test/isolation/specscanner.l b/src/test/isolation/specscanner.l
index 19883c265b..01f39fe05f 100644
--- a/src/test/isolation/specscanner.l
+++ b/src/test/isolation/specscanner.l
@@ -34,13 +34,19 @@ static void addlitchar(char c);
%x sql
-%x qstr
+%x qident
non_newline [^\n\r]
space [ \t\r\f]
comment ("#"{non_newline}*)
+digit [0-9]
+ident_start [A-Za-z\200-\377_]
+ident_cont [A-Za-z\200-\377_0-9\$]
+
+identifier {ident_start}{ident_cont}*
+
%%
%{
@@ -48,32 +54,42 @@ comment ("#"{non_newline}*)
litbufsize = LITBUF_INIT;
%}
+ /* Keywords (must appear before the {identifier} rule!) */
permutation { return PERMUTATION; }
session { return SESSION; }
setup { return SETUP; }
step { return STEP; }
teardown { return TEARDOWN; }
+ /* Whitespace and comments */
[\n] { yyline++; }
{comment} { /* ignore */ }
{space} { /* ignore */ }
- /* Quoted strings: "foo" */
+ /* Plain identifiers */
+{identifier} {
+ yylval.str = pg_strdup(yytext);
+ return(identifier);
+ }
+
+ /* Quoted identifiers: "foo" */
\" {
litbufpos = 0;
- BEGIN(qstr);
+ BEGIN(qident);
}
-<qstr>\" {
+<qident>\"\" { addlitchar(yytext[0]); }
+<qident>\" {
litbuf[litbufpos] = '\0';
yylval.str = pg_strdup(litbuf);
BEGIN(INITIAL);
- return(string_literal);
+ return(identifier);
}
-<qstr>. { addlitchar(yytext[0]); }
-<qstr>\n { yyerror("unexpected newline in quoted string"); }
-<qstr><<EOF>> { yyerror("unterminated quoted string"); }
+<qident>. { addlitchar(yytext[0]); }
+<qident>\n { yyerror("unexpected newline in quoted identifier"); }
+<qident><<EOF>> { yyerror("unterminated quoted identifier"); }
/* SQL blocks: { UPDATE ... } */
+ /* We trim leading/trailing whitespace, otherwise they're unprocessed */
"{"{space}* {
litbufpos = 0;
@@ -96,6 +112,7 @@ teardown { return TEARDOWN; }
yyerror("unterminated sql block");
}
+ /* Anything else is an error */
. {
fprintf(stderr, "syntax error at line %d: unexpected character \"%s\"\n", yyline, yytext);
exit(1);
