Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Fabien COELHO writes:
>> Probably the easiest fix is to add a rule that explicitly matches this
>> situation:
>> {nonspace}+{continuation} { ... throw back 2 chars and return the rest ... }
> Well, as the continuation characters must be ignored, so there is no need
> to throw them back, just adding the special case is enough?
True, for the moment. If we ever put an action into the continuation rule
(e.g. to count line numbers in the script) it might be better the other
way, but for now this is fine.
> Note anyway that it is not necessarily what people may intend when using a
> continuation:
Yeah, but as you say this varies from one language to another. I'm fine
with treating the continuation marker like whitespace.
Pushed with cosmetic adjustments (mostly, adding comments).
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
You have rules for
{nonspace}+ [...]
{continuation}
Remember that flex always takes the rule that produces the longest match
starting at the current point. {space}+ and {newline} don't conflict with
continuations, but {nonspace}+ does:
Indeed, I totally overlooked "{nonspace}+".
[...] I think this is surprising and inconsistent.
Sure.
Probably the easiest fix is to add a rule that explicitly matches this
situation:
{nonspace}+{continuation} { ... throw back 2 chars and return the rest ... }
Well, as the continuation characters must be ignored, so there is no need
to throw them back, just adding the special case is enough?
Attached a patch which adds the rule and just sends the found word, plus a
test script which also exercises this particular case.
Note anyway that it is not necessarily what people may intend when using a
continuation:
foo\
bla
might mean "foobla" rather than "foo" then "bla". For instance with bash:
sh>ec\
> ho 1
1
But the same trick in python gives a syntax error:
py> print\
... (1)
1 # ok...
py> pri\
... nt(1)
File "", line 2
nt(1)
^
SyntaxError: invalid syntax
I think it fine if pgbench behaves as python.
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 3fb29f8..eb0e8ac 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -810,6 +810,7 @@ pgbench options dbname
Script file meta commands begin with a backslash (\) and
extend to the end of the line.
+ They can spread over several lines with backslash-return continuations.
Arguments to a meta command are separated by white space.
These meta commands are supported:
@@ -838,7 +839,8 @@ pgbench options dbname
Examples:
\set ntellers 10 * :scale
-\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) % \
+ (10 * :scale) + 1
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 9a3be3d..ec58ae3 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -65,6 +65,7 @@ alnum [a-zA-Z0-9_]
space [ \t\r\f\v]
nonspace [^ \t\r\f\v\n]
newline [\n]
+continuation \\{newline}
/* Exclusive states */
%x EXPR
@@ -90,6 +91,11 @@ newline [\n]
/* INITIAL state */
+{nonspace}+{continuation} {
+ /* Found "word\\\n", just emit word and return it */
+ psqlscan_emit(cur_state, yytext, yyleng-2);
+ return 1;
+}
{nonspace}+ {
/* Found a word, emit and return it */
psqlscan_emit(cur_state, yytext, yyleng);
@@ -104,6 +110,8 @@ newline [\n]
return 0;
}
+{continuation} { /* ignore */ }
+
/* EXPR state */
{
@@ -138,6 +146,8 @@ newline [\n]
return FUNCTION;
}
+{continuation} { /* ignore */ }
+
{newline} {
/* report end of command */
last_was_newline = true;
cont.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
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Rafia Sabih writes:
> Okay, I am marking it as 'Ready for committer' with the following note
> to committer,
> I am getting whitespace errors in v3 of patch, which I corrected in
> v4, however, Fabien is of the opinion that v3 is clean and is showing
> whitespace errors because of downloader, etc. issues in my setup.
FWIW, what I see is that v3 saves out with Windows-style newlines (\r\n)
while v4 saves out with Unix newlines. It wouldn't really matter, because
"patch" converts Windows newlines (at least mine does), but Unix newlines
are our project style.
As for the substance of the patch ... the fix for continuations in
the INITIAL state is not this simple. You have rules for
{nonspace}+
{space}+
{newline}
{continuation}
Remember that flex always takes the rule that produces the longest match
starting at the current point. {space}+ and {newline} don't conflict with
continuations, but {nonspace}+ does: suppose that the next four characters
are "a" "b" "\" newline. flex will decide that the token is "ab\", rather
than taking the token as being "ab" and letting the "\" wait till next
time when it can be parsed as {continuation}.
In other words, it works to write backslash-newline immediately after a
token in the EXPR state (mainly because backslash isn't a legal part of
any token EXPR state currently allows), but not to write it immediately
after a token in INITIAL state. I think this is surprising and
inconsistent.
Probably the easiest fix is to add a rule that explicitly matches this
situation:
{nonspace}+{continuation} { ... throw back 2 chars and return the rest ... }
The "throw back" part is easily done with "yyless(yyleng - 2)"; compare
some rules in the core lexer. I have not actually tested this,
but I think it will work, because when it applies it would always be
the longest possible match.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
On Fri, Jan 13, 2017 at 9:15 PM, Fabien COELHO wrote: > >>> Could it be related to transformations operated when the file is >>> downloaded >>> and saved, because it is a text file? >> >> >> I think this is delaying the patch unnecessarily, I have attached a >> version, please see if you can apply it successfully, we can proceed >> with that safely then... > > > This is the same file I sent: > > sh> sha1sum pgbench-continuation-4.patch pgbench-continuation-3.patch > 97fe805a89707565210699694467f9256ca02dab pgbench-continuation-4.patch > 97fe805a89707565210699694467f9256ca02dab pgbench-continuation-3.patch > > The difference is that your version is mime-typed with a generic > > Application/OCTET-STREAM > > While the one I sent was mime-typed as > > text/x-diff > > as defined by the system in /etc/mime.types on my xenial laptop. > > My guess is that with the later your mail client changes the file when > saving it, because it sees "text". > Okay, I am marking it as 'Ready for committer' with the following note to committer, I am getting whitespace errors in v3 of patch, which I corrected in v4, however, Fabien is of the opinion that v3 is clean and is showing whitespace errors because of downloader, etc. issues in my setup. -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Could it be related to transformations operated when the file is downloaded and saved, because it is a text file? I think this is delaying the patch unnecessarily, I have attached a version, please see if you can apply it successfully, we can proceed with that safely then... This is the same file I sent: sh> sha1sum pgbench-continuation-4.patch pgbench-continuation-3.patch 97fe805a89707565210699694467f9256ca02dab pgbench-continuation-4.patch 97fe805a89707565210699694467f9256ca02dab pgbench-continuation-3.patch The difference is that your version is mime-typed with a generic Application/OCTET-STREAM While the one I sent was mime-typed as text/x-diff as defined by the system in /etc/mime.types on my xenial laptop. My guess is that with the later your mail client changes the file when saving it, because it sees "text". -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
On Fri, Jan 13, 2017 at 4:57 PM, Fabien COELHO wrote:
>
> Hello Rafia,
>
>>> sh> git apply ~/pgbench-continuation-3.patch
>>> # ok
>>
>>
>> It still gives me whitespace errors with git apply,
>
>
> Strange.
>
Yes, quite strange.
>> /Users/edb/Downloads/pgbench-continuation-3.patch:31: trailing whitespace.
>> continuation \\{newline}
>
>
>> Looks like an editor issue, I used awk '{ sub("\r$", ""); print }'
>> patch1 > patch2 to clean it and it was applying then.
>
>
> Doing that does not change the file for me.
>
> I see no \r in the patch file according to "od", I just see "nl" (0x0a).
>
> sha1sum: 97fe805a89707565210699694467f9256ca02dab
> pgbench-continuation-3.patch
>
> Could it be related to transformations operated when the file is downloaded
> and saved, because it is a text file?
>
I think this is delaying the patch unnecessarily, I have attached a
version, please see if you can apply it successfully, we can proceed
with that safely then...
--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
pgbench-continuation-4.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello Rafia,
sh> git apply ~/pgbench-continuation-3.patch
# ok
It still gives me whitespace errors with git apply,
Strange.
/Users/edb/Downloads/pgbench-continuation-3.patch:31: trailing whitespace.
continuation \\{newline}
Looks like an editor issue, I used awk '{ sub("\r$", ""); print }'
patch1 > patch2 to clean it and it was applying then.
Doing that does not change the file for me.
I see no \r in the patch file according to "od", I just see "nl" (0x0a).
sha1sum: 97fe805a89707565210699694467f9256ca02dab pgbench-continuation-3.patch
Could it be related to transformations operated when the file is
downloaded and saved, because it is a text file?
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
On Wed, Jan 11, 2017 at 5:39 PM, Fabien COELHO wrote:
>
> Hello,
>
>>> The attached patch adds backslash-return (well newline really)
>>> continuations
>>> to all pgbench backslash-commands.
>>>
>>> The attached test uses continuations on all such commands (sleep set
>>> setshell and shell).
>>>
>>> I think that adding continuations to psql should be a distinct patch.
>
>
>> The patch does not apply on the latest head, I guess this requires
>> rebasing since yours is posted in December.
>
>
> Strange. Here is a new version and a test for all known backslash-commands
> in pgbench.
>
> sh> git br test master
> sh> git apply ~/pgbench-continuation-3.patch
> # ok
> sh> git diff
> # ok
> sh> cd src/bin/pgbench
> sh> make
> sh> ./pgbench -t 1 -f SQL/cont.sql
> starting vacuum...end.
> debug(script=0,command=1): int 0
> debug(script=0,command=2): int 1
> debug(script=0,command=4): int 2
> 3
> debug(script=0,command=8): int 4
> transaction type: SQL/cont.sql
>
>> Again, it is giving trailing whitespace errors (as I reported for the
>> earlier version), plus it does not apply with git apply,
>
>
> It does above with the attached version.
It still gives me whitespace errors with git apply,
/Users/edb/Downloads/pgbench-continuation-3.patch:31: trailing whitespace.
continuation \\{newline}
/Users/edb/Downloads/pgbench-continuation-3.patch:39: trailing whitespace.
{continuation} { /* ignore */ }
/Users/edb/Downloads/pgbench-continuation-3.patch:40: trailing whitespace.
/Users/edb/Downloads/pgbench-continuation-3.patch:48: trailing whitespace.
{continuation} { /* ignore */ }
/Users/edb/Downloads/pgbench-continuation-3.patch:49: trailing whitespace.
error: patch failed: doc/src/sgml/ref/pgbench.sgml:810
error: doc/src/sgml/ref/pgbench.sgml: patch does not apply
error: patch failed: src/bin/pgbench/exprscan.l:65
error: src/bin/pgbench/exprscan.l: patch does not apply
Looks like an editor issue, I used awk '{ sub("\r$", ""); print }'
patch1 > patch2 to clean it and it was applying then.
>> hopefully that would be fixed once rebased. Other than that, I observed
>> that if after backslash space is there, then the command fails.
>
>
> Yes, this is expected.
>
>> I think it should be something like if after backslash some spaces are
>> there, followed by end-of-line then it should ignore these spaces and read
>> next line, atleast with this new meaning of backslash.
>
>
> Hmmm. This is not the behavior of backslash continuation in bash or python,
> I do not think that this is desirable to have a different behavior.
>
Okay, seems sensible.
>> Otherwise, it should be mentioned in the docs that backslash should not be
>> followed by space.
>
>
> I'm not sure. Doc says that continuation is "backslash-return", it cannot be
> more explicit. If it must say what it is not, where should it stop?
>
> --
> Fabien.
Please post the clean version and I'll mark it as ready for committer then.
--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello,
The attached patch adds backslash-return (well newline really) continuations
to all pgbench backslash-commands.
The attached test uses continuations on all such commands (sleep set
setshell and shell).
I think that adding continuations to psql should be a distinct patch.
The patch does not apply on the latest head, I guess this requires
rebasing since yours is posted in December.
Strange. Here is a new version and a test for all known backslash-commands
in pgbench.
sh> git br test master
sh> git apply ~/pgbench-continuation-3.patch
# ok
sh> git diff
# ok
sh> cd src/bin/pgbench
sh> make
sh> ./pgbench -t 1 -f SQL/cont.sql
starting vacuum...end.
debug(script=0,command=1): int 0
debug(script=0,command=2): int 1
debug(script=0,command=4): int 2
3
debug(script=0,command=8): int 4
transaction type: SQL/cont.sql
Again, it is giving trailing whitespace errors (as I reported for the
earlier version), plus it does not apply with git apply,
It does above with the attached version.
hopefully that would be fixed once rebased. Other than that, I observed
that if after backslash space is there, then the command fails.
Yes, this is expected.
I think it should be something like if after backslash some spaces are
there, followed by end-of-line then it should ignore these spaces and
read next line, atleast with this new meaning of backslash.
Hmmm. This is not the behavior of backslash continuation in bash or
python, I do not think that this is desirable to have a different
behavior.
Otherwise, it should be mentioned in the docs that backslash should not
be followed by space.
I'm not sure. Doc says that continuation is "backslash-return", it cannot
be more explicit. If it must say what it is not, where should it stop?
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 3fb29f8..eb0e8ac 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -810,6 +810,7 @@ pgbench options dbname
Script file meta commands begin with a backslash (\) and
extend to the end of the line.
+ They can spread over several lines with backslash-return continuations.
Arguments to a meta command are separated by white space.
These meta commands are supported:
@@ -838,7 +839,8 @@ pgbench options dbname
Examples:
\set ntellers 10 * :scale
-\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) % \
+ (10 * :scale) + 1
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 9a3be3d..33cc1ea 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -65,6 +65,7 @@ alnum [a-zA-Z0-9_]
space [ \t\r\f\v]
nonspace [^ \t\r\f\v\n]
newline [\n]
+continuation \\{newline}
/* Exclusive states */
%x EXPR
@@ -104,6 +105,8 @@ newline [\n]
return 0;
}
+{continuation} { /* ignore */ }
+
/* EXPR state */
{
@@ -138,6 +141,8 @@ newline [\n]
return FUNCTION;
}
+{continuation} { /* ignore */ }
+
{newline} {
/* report end of command */
last_was_newline = true;
cont.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
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
On Sat, Dec 3, 2016 at 1:52 PM, Fabien COELHO wrote: > >> FWIW, I looked a bit further and concluded that probably psqlscan.l >> doesn't need to be modified; so likely you could do this across all of >> pgbench's commands without touching psql. That might be an acceptable >> compromise for now, though I still think that as soon as we have this >> for pgbench, users will start wanting it in psql. > > > The attached patch adds backslash-return (well newline really) continuations > to all pgbench backslash-commands. > > The attached test uses continuations on all such commands (sleep set > setshell and shell). > > I think that adding continuations to psql should be a distinct patch. > > -- > Fabien. Hello Fabien, The patch does not apply on the latest head, I guess this requires rebasing since yours is posted in December. Again, it is giving trailing whitespace errors (as I reported for the earlier version), plus it does not apply with git apply, hopefully that would be fixed once rebased. Other than that, I observed that if after backslash space is there, then the command fails. I think it should be something like if after backslash some spaces are there, followed by end-of-line then it should ignore these spaces and read next line, atleast with this new meaning of backslash. Otherwise, it should be mentioned in the docs that backslash should not be followed by space. -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
FWIW, I looked a bit further and concluded that probably psqlscan.l
doesn't need to be modified; so likely you could do this across all of
pgbench's commands without touching psql. That might be an acceptable
compromise for now, though I still think that as soon as we have this
for pgbench, users will start wanting it in psql.
The attached patch adds backslash-return (well newline really)
continuations to all pgbench backslash-commands.
The attached test uses continuations on all such commands (sleep set
setshell and shell).
I think that adding continuations to psql should be a distinct patch.
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 3a65729..b155db5 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -814,6 +814,7 @@ pgbench options dbname
Script file meta commands begin with a backslash (\) and
extend to the end of the line.
+ They can spread over several lines with backslash-return continuations.
Arguments to a meta command are separated by white space.
These meta commands are supported:
@@ -842,7 +843,8 @@ pgbench options dbname
Examples:
\set ntellers 10 * :scale
-\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) % \
+ (10 * :scale) + 1
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 20891a3..3c428de 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -65,6 +65,7 @@ alnum [a-zA-Z0-9_]
space [ \t\r\f\v]
nonspace [^ \t\r\f\v\n]
newline [\n]
+continuation \\{newline}
/* Exclusive states */
%x EXPR
@@ -104,6 +105,8 @@ newline [\n]
return 0;
}
+{continuation} { /* ignore */ }
+
/* EXPR state */
{
@@ -138,6 +141,8 @@ newline [\n]
return FUNCTION;
}
+{continuation} { /* ignore */ }
+
{newline} {
/* report end of command */
last_was_newline = true;
cont.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
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
On Fri, Dec 2, 2016 at 6:13 AM, Tom Lane wrote: > Fabien COELHO writes: > >> In psql, if backslash followed by [CR]LF is interpreted as a > >> continuation symbol, commands like these seem problematic > >> on Windows since backslash is the directory separator: > >> > >> \cd \ > >> \cd c:\somepath\ > >> > >> Shell invocations also come to mind: > >> \! dir \ > > > Thanks for pointing out these particular cases. I was afraid of such > > potential issues, hence my questions... > > Those look like nasty counterexamples, but I think they are not, because > they don't work today. What you get is > > regression=# \cd \ > Invalid command \. Try \? for help. > > AFAICT you would need to write it > > regression=# \cd '\\' > \cd: could not change directory to "\": No such file or directory > > (That's on Unix of course, on Windows I'd expect it to succeed.) > > The reason for this is that psql already has a policy that an unquoted > backslash begins a new backslash command on the same line. Since > there is no command named backslash-return, this is available syntax > that can be repurposed in the direction we want. > > I believe that we need to do basically the same thing in pgbench, and > I'm fine with that because I think we should have an overall policy of > synchronizing the psql and pgbench metacommand syntaxes as best we can. > This will at some point necessitate invention of a quoting rule for > pgbench metacommand arguments, so that you can write a backslash as part > of an argument when you need to. We might not need to do that today > (as I'm not sure there are any use-cases for one), but maybe it'd be best > to just bite the bullet and put it in. > > The review from the committer is arrived at the end of commitfest, so moved the patch into next CF with "waiting on author" status. Regards, Hari Babu Fujitsu Australia
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Fabien COELHO writes: >> In psql, if backslash followed by [CR]LF is interpreted as a >> continuation symbol, commands like these seem problematic >> on Windows since backslash is the directory separator: >> >> \cd \ >> \cd c:\somepath\ >> >> Shell invocations also come to mind: >> \! dir \ > Thanks for pointing out these particular cases. I was afraid of such > potential issues, hence my questions... Those look like nasty counterexamples, but I think they are not, because they don't work today. What you get is regression=# \cd \ Invalid command \. Try \? for help. AFAICT you would need to write it regression=# \cd '\\' \cd: could not change directory to "\": No such file or directory (That's on Unix of course, on Windows I'd expect it to succeed.) The reason for this is that psql already has a policy that an unquoted backslash begins a new backslash command on the same line. Since there is no command named backslash-return, this is available syntax that can be repurposed in the direction we want. I believe that we need to do basically the same thing in pgbench, and I'm fine with that because I think we should have an overall policy of synchronizing the psql and pgbench metacommand syntaxes as best we can. This will at some point necessitate invention of a quoting rule for pgbench metacommand arguments, so that you can write a backslash as part of an argument when you need to. We might not need to do that today (as I'm not sure there are any use-cases for one), but maybe it'd be best to just bite the bullet and put it in. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello Daniel, - if not, are possible corner case backward incompatibilities introduced by such feature ok? In psql, if backslash followed by [CR]LF is interpreted as a continuation symbol, commands like these seem problematic on Windows since backslash is the directory separator: \cd \ \cd c:\somepath\ Shell invocations also come to mind: \! dir \ Thanks for pointing out these particular cases. I was afraid of such potential issues, hence my questions... Would requiring a space after the \ to accept these be ok, even if it is somehow a regression? By the way, how does changing directory to a directory with a space in the name works? I.e. does "\" already means something for some commands? Given these examples, I'm not sure I see a way out to having continuations on all backslash commands without really breaking some stuff... Or maybe having continuations for some commands only, but this is exactly what is rejected by Tom about my patch... -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Fabien COELHO wrote: > - if not, are possible corner case backward incompatibilities introduced >by such feature ok? In psql, if backslash followed by [CR]LF is interpreted as a continuation symbol, commands like these seem problematic on Windows since backslash is the directory separator: \cd \ \cd c:\somepath\ Shell invocations also come to mind: \! dir \ Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello Tom, [...] I definitely agree that having homogeneous continuations on every backslash commands is better, but this has a cost in code lines and possible backward compatibilities. This is the kind of thing better addressed in the initial design rather than long afterwards... FWIW, I looked a bit further and concluded that probably psqlscan.l doesn't need to be modified; so likely you could do this across all of pgbench's commands without touching psql. Hmmm, I'm a little bit lost, you just asked for that: In short, I want to [...] ask for a version that applies globally to all backslash commands in psql and pgbench. I'm pointing out that this requirements implies that all such commands share some minimal common lexical structure, higher that the current "pass every characters up to the next new line". - what would be the common acceptable requirements? at least \ = continuation - maybe \\ is just \ if need be? or not?? - what about simple/double quoted strings (eg in \copy ...)? or can we say that the continuation preprocessing does not look into that, and is pretty rough, and that is fine? - if not, are possible corner case backward incompatibilities introduced by such feature ok? That might be an acceptable compromise for now, though I still think that as soon as we have this for pgbench, users will start wanting it in psql. ISTM that you put the case for having them everywhere or nowhere, so I'm trying to measure the effort implied by the former before deciding what I'll do. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Fabien COELHO writes: >> In short, I want to mark this RWF for today and ask for a version that >> applies globally to all backslash commands in psql and pgbench. > I'm not sure such a simple feature deserves so much energy. It's not a "simple feature". As you have it, it's a non-orthogonal wart. It's like telling users that the return key only works in \set commands and elsewhere you have to type shift-return to end a line. I'm fine with setting up a global policy that backslash-newline works to continue backslash commands across lines, but I'm not fine with saying that it only works in \set. That's just weird, and the fact that you can do it with a trivial patch doesn't make it less weird. You're proposing to put a new cognitive load on users because you can't be bothered to write a patch that's not half-baked. We don't do things like that around here. FWIW, I looked a bit further and concluded that probably psqlscan.l doesn't need to be modified; so likely you could do this across all of pgbench's commands without touching psql. That might be an acceptable compromise for now, though I still think that as soon as we have this for pgbench, users will start wanting it in psql. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello Tom, In short, I want to mark this RWF for today and ask for a version that applies globally to all backslash commands in psql and pgbench. Hmmm. The modus operandi of backslash command scanning is to switch to possibly another scanner just after scanning the backslash command, so I did the simple thing which is to handle this in the expression scanner. A simple feature achieved with a simple code. Factoring out the behavior means handling continuations from the master scanner, probably by using some other intermediate buffer which is then rescanned... This implies that all backslash commands share some kind of minimal lexical convention that \ followed by a (\r|\n|\r\n) is a continuation... But then what if the backslash command accept quoted strings, should a continuation still be a continuation inside quotes? If not, how do I know? Are all currently existing backslash command compatible with a common set of lexical convention? Or do some commands should accept backslash continuations and others not, and have to be treated differently and knowingly by the lexer? There is also the issue of locating error messages if the continuation is in another buffer, probably someone will complain. Basically, many issues arise, all of them somehow solvable, but I'm afraid with many more lines than my essentially one line patch:-) I'm not sure such a simple feature deserves so much energy. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Rafia Sabih writes: > On Wed, Nov 9, 2016 at 3:28 PM, Fabien COELHO wrote: +1. My vote is for backslash continuations. >> I'm fine with that! > Looks good to me also. I concur that we don't want implicit continuation; that creates too many special cases and will certainly fail to generalize to other backslash commands. My gripe with pgbench-set-continuation-1.patch is actually the latter: I do not like the idea that this works only for \set and not other backslash commands. I think we should have uniform behavior across all backslash commands, which probably means implementing this in psqlscan.l not exprscan.l, which probably means that it would apply in psql as well as pgbench. But I argue that's a Good Thing. I certainly don't see that psql needs this less ... try a \copy command with a complicated SELECT source sometime. In short, I want to mark this RWF for today and ask for a version that applies globally to all backslash commands in psql and pgbench. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
On Wed, Nov 9, 2016 at 3:28 PM, Fabien COELHO wrote: > > +1. My vote is for backslash continuations. >> > > I'm fine with that! > > -- > Fabien. > Looks good to me also. -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
+1. My vote is for backslash continuations. I'm fine with that! -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
On Tue, Nov 8, 2016 at 5:33 AM, Fabien COELHO wrote: > I do not think that having both inferred continuations and explicit > backslash continuations is desirable, it should be one or the other. +1. My vote is for backslash continuations. Inferred continuations require you to end your expressions in places where they can't legally stop, so you can't do \set x 2 +3 will not do the same thing as \set x 2+ 3 I don't want to get into a situation where every future bit of pgbench syntax we want to introduce has to worry about what the interaction with inferred continuations might be. Backslash continuations are kind of ugly, but it's a simple rule and virtually everybody who is likely to be writing pgbench scripts will understand it immediately. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello Rafia, Please keep copying to the list. Though I find the first version of this patch acceptable in terms that it would be helpful on enhancing the readability of expressions as you mentioned. However, the second version is not clear as I mentioned before also, there has to be detailed documentation regarding this, still as an end-user one needs to be too careful to decide which statements to split lines based on the patch, for example following \set bid CASE WHEN random(0, 99) < 85 THEN :tbid ELSE :abid + (:abid >= :tbid) END should be written as \set bid CASE WHEN random(0, 99) < 85 THEN :tbid ELSE :abid + (:abid >= :tbid) END I do not understand the "should". It is a matter of style, and both cases would work with the second version of the patch. (and a few more variants of splitting lines just after an operator or open parenthesis) which does not look like much enhancement in readability to me. As I said, I will not fight over this one. I like inferred continuations in scala, but I guess it would be surprising for such an old school script like pgbench, and it would be hard to do that consistently for pgsql because the backslash command syntax may depends on the command itself (eg does it have to respect strings and parenthesis, or some other structures...). So this patch needs to add the support to read the next line even when "\set a" and other such similar expressions are encountered. The scala convention is that if the line is not finished the user must either use explicit parenthesis are use an unambiguous operator which expects an operand, so that the lexer knows it has to go on. I do not think that having both inferred continuations and explicit backslash continuations is desirable, it should be one or the other. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
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.
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.
--
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 options dbname
%) with their usual precedence and associativity,
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.
Examples:
\set ntellers 10 * :scale
-\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) %
+ (10 * :scale) + 1
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 */
+
+{
+
+{newline} { /* ignore */ }
+
+}
+
/* EXPR state */
{
-"+"{ return '+'; }
-"-"{ return '-'; }
-"*"{ return '*'; }
-"/"{ return '/'; }
-"%"{ return '%'; }
-"("{ return '('; }
-")"{ return ')'; }
-","{ return ','; }
+{newline} {
+ /* report end of command */
+ last_was_newline = true;
+ return 0;
+}
+}
+
+ /* EXPR & COEX states common rules */
+
+{
+
+"+"{ 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;
}
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
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
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. In general, this new approach is likely to create confusions in such cases. 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. On Tue, Nov 1, 2016 at 4:23 PM, Fabien COELHO wrote: > > Attached patch does what is described in the title, hopefully. >> Continuations in other pgbench backslash-commands should be dealt with >> elsewhere... >> >> Also attached is a small test script. >> > > Here is another approach, with "infered" continuations: no backslash is > needed, the parsing is pursued if the last token of the line cannot end an > expression (eg an operator) or if there is an unclosed parenthesis. > > I think that backslashes are less surprising for the classically minded > user, but this one is more fun:-) Also, this version changes a little more > the scanner because on each token the next state (continued or not) must be > decided. > > -- > Fabien. > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- Regards, Rafia Sabih EnterpriseDB: http://www.enterprisedb.com/
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Attached patch does what is described in the title, hopefully. Continuations
in other pgbench backslash-commands should be dealt with elsewhere...
Also attached is a small test script.
Here is another approach, with "infered" continuations: no backslash is
needed, the parsing is pursued if the last token of the line cannot end an
expression (eg an operator) or if there is an unclosed parenthesis.
I think that backslashes are less surprising for the classically minded
user, but this one is more fun:-) Also, this version changes a little more
the scanner because on each token the next state (continued or not) must
be decided.
--
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 options dbname
%) with their usual precedence and associativity,
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.
Examples:
\set ntellers 10 * :scale
-\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) %
+ (10 * :scale) + 1
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 20891a3..1b0d4d0 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 */
+
+{
+
+{newline} { /* ignore */ }
+
+}
+
/* EXPR state */
{
-"+"{ return '+'; }
-"-"{ return '-'; }
-"*"{ return '*'; }
-"/"{ return '/'; }
-"%"{ return '%'; }
-"("{ return '('; }
-")"{ return ')'; }
-","{ return ','; }
+{newline} {
+ /* report end of command */
+ last_was_newline = true;
+ return 0;
+}
+}
+
+ /* EXPR & COEX states common rules */
+
+{
+
+"+"{ 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 */ }
.{
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
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello Rafia,
Seems like an interesting addition to pgbench interface, but not sure where
it's required, it'll be good if you may provide some cases where it's
utility can be witnessed. Something like where you absolutely need
continuations in expression.
It is never "absolutely needed", you can always put the expression on one
longer line.
As an long line example, supposing some other currently submitted patches
are committed to pgbench, for implementing tpc-b according to the
specification one may write the following:
\set bid CASE WHEN random(0, 99) < 85 THEN :tbid ELSE :abid + (:abid >=
:tbid) END
or
\set bid \
CASE WHEN random(0, 99) < 85 \
THEN :tbid \
ELSE :abid + (:abid >= :tbid) \
END
I find the second version more readable, but it is a really matter of
taste, obviously.
While applying it is giving some trailing whitespace errors, please
correct them.
sh> git checkout -b tmp
sh> git apply ~/pgbench-set-continuation-1.patch
sh> git commit -a
sh>
I do not get any errors, and I have not found any trailing spaces in the
patch. Could you give me the precise error message you got?
As an additional comment you may consider reformatting
following snippet
{continuation} { /* ignore */ }
as
{continuation} {
/* ignore */
}
Hmmm... Other lex rules to ignore spaces use the one line syntax, I would
prefer to keep it the same as those.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hie Fabien,
Seems like an interesting addition to pgbench interface, but not sure where
it's required, it'll be good if you may provide some cases where it's
utility can be witnessed. Something like where you absolutely need
continuations in expression.
While applying it is giving some trailing whitespace errors, please correct
them.
As an additional comment you may consider reformatting following snippet
> {continuation} { /* ignore */ }
>
> as
> {continuation} {
> /* ignore */
> }
Thanks.
On Mon, Oct 3, 2016 at 6:16 PM, Christoph Berg wrote:
> Re: Fabien COELHO 2016-10-03
> > > I "\set" a bunch of lengthy SQL commands in there, e.g.
> >
> > I agree that this looks like a desirable feature, however I would tend to
> > see that as material for another independent patch.
>
> Sure, my question was by no means intending to stop your pgbench patch
> from going forward by adding extra requirements.
>
> > Hmmm. I'm not sure how this is parsed. If this is considered a string
> '...',
> > then maybe \set should wait for the end of the string instead of the end
> of
> > the line, i.e. no continuation would be needed...
> >
> > \set config '
> > SELECT name, ...
> >CASE ... END
> > FROM pg_settings
> > WHERE ...;'
>
> I guess that would be the sane solution here, yes. Not adding any \
> chars at the end of the line would also mean cut-and-paste of the RHS
> content would work.
>
> Thanks for the feedback!
>
> Christoph
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Re: Fabien COELHO 2016-10-03 > > I "\set" a bunch of lengthy SQL commands in there, e.g. > > I agree that this looks like a desirable feature, however I would tend to > see that as material for another independent patch. Sure, my question was by no means intending to stop your pgbench patch from going forward by adding extra requirements. > Hmmm. I'm not sure how this is parsed. If this is considered a string '...', > then maybe \set should wait for the end of the string instead of the end of > the line, i.e. no continuation would be needed... > > \set config ' > SELECT name, ... >CASE ... END > FROM pg_settings > WHERE ...;' I guess that would be the sane solution here, yes. Not adding any \ chars at the end of the line would also mean cut-and-paste of the RHS content would work. Thanks for the feedback! Christoph -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Hello Christoph, Attached patch does what is described in the title, hopefully. Continuations in other pgbench backslash-commands should be dealt with elsewhere... Would (a similar version of) that patch also apply to .psqlrc? Pgbench has its own lexer & parser for \set expressions, so the continuation is handled there. I "\set" a bunch of lengthy SQL commands in there, e.g. I agree that this looks like a desirable feature, however I would tend to see that as material for another independent patch. I think that .pgsqrc is really just a "psql" script so the handling would be somewhere there... I'll have a look. \set config 'SELECT name, current_setting(name), CASE source WHEN $$configuration file$$ THEN sourcefile||$$:$$||sourceline ELSE source END FROM pg_settings WHERE source <> $$default$$;' Hmmm. I'm not sure how this is parsed. If this is considered a string '...', then maybe \set should wait for the end of the string instead of the end of the line, i.e. no continuation would be needed... \set config ' SELECT name, ... CASE ... END FROM pg_settings WHERE ...;' Being able to split that over several lines would greatly improve maintainability. (Though I do realize this would also require a notion for splitting/continuing strings.) Yep. I'm not sure of the actual feature which is needed. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench - allow backslash continuations in \set expressions
Re: Fabien COELHO 2016-10-03 > > Attached patch does what is described in the title, hopefully. Continuations > in other pgbench backslash-commands should be dealt with elsewhere... Would (a similar version of) that patch also apply to .psqlrc? I "\set" a bunch of lengthy SQL commands in there, e.g. \set config 'SELECT name, current_setting(name), CASE source WHEN $$configuration file$$ THEN sourcefile||$$:$$||sourceline ELSE source END FROM pg_settings WHERE source <> $$default$$;' Being able to split that over several lines would greatly improve maintainability. (Though I do realize this would also require a notion for splitting/continuing strings.) Christoph signature.asc Description: PGP signature
[HACKERS] pgbench - allow backslash continuations in \set expressions
Attached patch does what is described in the title, hopefully.
Continuations in other pgbench backslash-commands should be dealt with
elsewhere...
Also attached is a small test script.
While looking at the code, I noticed that newline is \n. Maybe it should
be (\r|\n|\r\n) to allow for MacOS & Windows. I have not changed that for
now.
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 285608d..6f068cd 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -818,6 +818,8 @@ pgbench options dbname
Sets variable varname to a value calculated
from expression.
+ The expression may span several lines with backslash-newline
+ continuations.
The expression may contain integer constants such as 5432,
double constants such as 3.14159,
references to variables :variablename,
@@ -832,7 +834,8 @@ pgbench options dbname
Examples:
\set ntellers 10 * :scale
-\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) % \
+ (10 * :scale) + 1
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 20891a3..97e7a79 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -65,6 +65,8 @@ alnum [a-zA-Z0-9_]
space [ \t\r\f\v]
nonspace [^ \t\r\f\v\n]
newline [\n]
+/* continuations in pgbench expressions */
+continuation \\{newline}
/* Exclusive states */
%x EXPR
@@ -138,6 +140,8 @@ newline [\n]
return FUNCTION;
}
+{continuation} { /* ignore */ }
+
{newline} {
/* report end of command */
last_was_newline = true;
cont.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
