Thread: PATCH: pgbench allow '=' in \set
Hello devs, Since an expression syntax has been added to pgbench, I find that the readability of expressions is not great. An extra '=' improves the situation for me: \set id = 1 + abs((:id * 1021) % (100000 * :scale)) seems slightly better than: \set id 1 + abs((:id * 1021) % (100000 * :scale)) But that is debatable! The attached patch just ignores a leading '=' in a pgbench expression. -- Fabien.
2015-05-07 20:18 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:
Hello devs,
Since an expression syntax has been added to pgbench, I find that the readability of expressions is not great. An extra '=' improves the situation for me:
\set id = 1 + abs((:id * 1021) % (100000 * :scale))
seems slightly better than:
\set id 1 + abs((:id * 1021) % (100000 * :scale))
But that is debatable!
The attached patch just ignores a leading '=' in a pgbench expression.
It is question :( - it break a consistency with psql
Regards
Pavel
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2015-05-07 20:18 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:
>> The attached patch just ignores a leading '=' in a pgbench expression.
> It is question :( - it break a consistency with psql
I've got doubts about this too: it introduces fundamental inconsistency
in pgbench itself, which may come back to bite us later. Who's to say
that '=' will never be a meaningful operator in pgbench expressions?
regards, tom lane
Hello, >> \set id = 1 + abs((:id * 1021) % (100000 * :scale)) >> >> seems slightly better than: >> >> \set id 1 + abs((:id * 1021) % (100000 * :scale)) > > It is question :( - it break a consistency with psql It actually "breaks" nothing as it is purely cosmectic:-) More seriously, I'm not sure that having a apparent syntatic homogeneity between psql and pgbench should be a requirement, but that is a point worth raising. The syntax are not really the same anyway: for instance "\set" and "\set NAME" means something for psql but not for pgbench. Moreover the "\set [NAME [VALUE]]" syntax of psql does not allow an expression, so it stays quite readable as it is, a situation not comparable to pgbench with expressions. So I would tend to fully ignore the similitude between the two as a guideline, as it is only a simulitude and not a real compatibility issue. -- Fabien.
Hello,\set id = 1 + abs((:id * 1021) % (100000 * :scale))
seems slightly better than:
\set id 1 + abs((:id * 1021) % (100000 * :scale))
It is question :( - it break a consistency with psql
It actually "breaks" nothing as it is purely cosmectic:-)
Would "colon-equal" be more acceptable - like in pl/pgsql?Even if "=" becomes a valid operator I would have to think it is a binary operator and so its position at the start of an expression would still be unambiguous as to whether it is cosmetic or functional.
More seriously, I'm not sure that having a apparent syntatic homogeneity between psql and pgbench should be a requirement, but that is a point worth raising.
The syntax are not really the same anyway: for instance "\set" and "\set NAME" means something for psql but not for pgbench.
Moreover the "\set [NAME [VALUE]]" syntax of psql does not allow an expression, so it stays quite readable as it is, a situation not comparable to pgbench with expressions.
This seems logical though having never used pgbench or compared them in this manner...
The idea of an actual symbol separating the variable and the expression seems worthwhile to add even in the face of "inconsistency" - which itself should possibly be improved by yet additional changes.
David J.
Hello Tom, > I've got doubts about this too: it introduces fundamental inconsistency > in pgbench itself, which may come back to bite us later. Who's to say > that '=' will never be a meaningful operator in pgbench expressions? Hmmm... it would not be an issue as long as '=' is not a unary operator? If you add "expr '=' expr" in the syntax, there should be no conflict with the result target "'=' expr" anyway? I've just tried that and there is no issue raised by bison. So ISTM that it should not bite later on that ground. -- Fabien.
> Would "colon-equal" be more acceptable - like in pl/pgsql? Hmmm... I would tend to think that is even clearer as a separator than a mere "=". Too much Pascal in my youth? :-) Although ":" means beginning of variable name in pgbench, I would not think that it is an issue because = is not a valid variable name. > Even if "=" becomes a valid operator I would have to think it is a binary > operator and so its position at the start of an expression would still be > unambiguous as to whether it is cosmetic or functional. That is also my conclusion. -- Fabien.
On Thu, May 7, 2015 at 2:18 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: > Since an expression syntax has been added to pgbench, I find that the > readability of expressions is not great. An extra '=' improves the situation > for me: > > \set id = 1 + abs((:id * 1021) % (100000 * :scale)) > > seems slightly better than: > > \set id 1 + abs((:id * 1021) % (100000 * :scale)) > > But that is debatable! I would like to debate that. :-) Generally, I don't like optional noise words. A good example of how this can bite you is the whole problem with LOCK TABLE foo. You can also write that as LOCK foo. What if you want to lock something that's not a table? If the word "table" were required, then you could support LOCK VIEW foo or LOCK MATERIALIZED VIEW foo or even LOCK FUNCTION foo(). But because the word TABLE is optional, that syntax creates parser ambiguities. We have not been able to agree on a reasonable solution to this problem, and it has blocked implementation of this important and useful feature for years. While I can't foresee what trouble specifically your proposal might cause, I have become wary of such things. I also don't like to restate what has already been said. \set at the beginning of the line tells you that you will be setting a variable. Adding = or := only restates the same thing. I agree it superficially looks a little nicer, but I'm not sure it's really going to add clarity, because it's basically just redundant. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> I also don't like to restate what has already been said. \set at the > beginning of the line tells you that you will be setting a variable. > Adding = or := only restates the same thing. I agree it superficially > looks a little nicer, but I'm not sure it's really going to add > clarity, because it's basically just redundant. Ok. I've marked this one as REJECTED. Otherwise, I was considering this kind of things: n := expr If we have functions, that could include: n := random(1, 100) with more work (handling of double constants): n := exprandom(1, 100, 3.5) and maybe: n := SELECT ... or even: n, m, p, q := SELECT ... Also, having ";" as a end of commands could also help by allowing multiline commands, but that would break compatibility. Maybe allowing continuations (\\\n) would be an acceptable compromise. -- Fabien.
On Wed, May 13, 2015 at 3:54 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: > Ok. I've marked this one as REJECTED. > > Otherwise, I was considering this kind of things: > > n := expr > n, m, p, q := SELECT ... > > Also, having ";" as a end of commands could also help by allowing multiline > commands, but that would break compatibility. Maybe allowing continuations > (\\\n) would be an acceptable compromise. It's been my assumption that we wanted to keep the existing pgbench syntax mostly intact, and extend it. We could, of course, invent a completely new syntax, but it might be hard to get everybody to agree on what the new thing should look like. I loathe violently the convention of using a backslash at the end of a line, because it's too easy to write backslash-space-newline or backslash-tab-newline when you meant to write backslash-newline. But maybe we should do it anyway. We certainly need some solution to that problem, because the status quo is monumentally annoying, and that might be the least bad solution available. Another option, breaking backward compatibility, would be to decide that backslash commands have to be terminated by a semicolon token. Then we wouldn't need a continuation character, because we'd just continue across lines until we hit the terminator. Of course, that doesn't solve the problem for people who want to include multi-line SQL queries. If we wanted to make that work, the best option might be to duplicate the backend lexer into pgbench just as we already do with psql. Then it could identify properly-terminated SQL queries automatically. That, too, would be a backward compatibility break, since the terminating semicolon would become required there as well. I somewhat lean toward this second option, because I think it will be a lot more convenient in the long run. We'll probably get some complains about breaking people's pgbench scripts, but I'd personally be prepared to accept that as the price of progress. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hello Robert, <Sorry resent, wrong from> >> Also, having ";" as a end of commands could also help by allowing multiline >> commands, but that would break compatibility. Maybe allowing continuations >> (\\\n) would be an acceptable compromise. > I loathe violently the convention of using a backslash at the end of a line, > because it's too easy to write backslash-space-newline or > backslash-tab-newline when you meant to write backslash-newline. But maybe we > should do it anyway. We certainly need some solution to that problem, > because the status quo is monumentally annoying, and that might be the least > bad solution available. I survive with that in bash/make/python... > Another option, breaking backward compatibility, would be to decide > that backslash commands have to be terminated by a semicolon token. I do not like it much, as it is inconsistent/incompatible with "psql". > [...] multi-line SQL queries. If we wanted to make that work, the best > option might be to duplicate the backend lexer into pgbench just as we > already do with psql. [...] > > I somewhat lean toward this second option, because I think it will be > a lot more convenient in the long run. We'll probably get some > complains about breaking people's pgbench scripts, but I'd personally > be prepared to accept that as the price of progress. For an actual lexer: currently there is no real lexer for SQL commands in pgbench, the line is just taken as is, so that would mean adding another one, although probably a simplified one would do. To conclude, I'm rather for continuations, despite their ugliness, because (1) it is much easier (just a very small change in read_line_from_file) and (2) it is backward compatible, so no complaints handle. -- Fabien.
On Thu, May 14, 2015 at 3:20 AM, Fabien COELHO <fabien.coelho@mines-paristech.fr> wrote: >> I loathe violently the convention of using a backslash at the end of a >> line, because it's too easy to write backslash-space-newline or >> backslash-tab-newline when you meant to write backslash-newline. But maybe >> we should do it anyway. We certainly need some solution to that problem, >> because the status quo is monumentally annoying, and that might be the least >> bad solution available. > > I survive with that in bash/make/python... Yeah. >> Another option, breaking backward compatibility, would be to decide >> that backslash commands have to be terminated by a semicolon token. > > I do not like it much, as it is inconsistent/incompatible with "psql". True, but anything will be, as far as backslash commands are concerned. psql doesn't support continuation lines in backslash commands at all. >> [...] multi-line SQL queries. If we wanted to make that work, the best >> option might be to duplicate the backend lexer into pgbench just as we >> already do with psql. [...] >> >> I somewhat lean toward this second option, because I think it will be >> a lot more convenient in the long run. We'll probably get some >> complains about breaking people's pgbench scripts, but I'd personally >> be prepared to accept that as the price of progress. > > For an actual lexer: currently there is no real lexer for SQL commands in > pgbench, the line is just taken as is, so that would mean adding another > one, although probably a simplified one would do. I think what we'd do is extend the expression lexer to cover everything in the file. > To conclude, I'm rather for continuations, despite their ugliness, because > (1) it is much easier (just a very small change in read_line_from_file) and > (2) it is backward compatible, so no complaints handle. Those are certainly points to consider. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Fabien COELHO <coelho@cri.ensmp.fr> writes:
>> Another option, breaking backward compatibility, would be to decide
>> that backslash commands have to be terminated by a semicolon token.
> I do not like it much, as it is inconsistent/incompatible with "psql".
>> [...] multi-line SQL queries. If we wanted to make that work, the best
>> option might be to duplicate the backend lexer into pgbench just as we
>> already do with psql. [...]
>>
>> I somewhat lean toward this second option, because I think it will be
>> a lot more convenient in the long run. We'll probably get some
>> complains about breaking people's pgbench scripts, but I'd personally
>> be prepared to accept that as the price of progress.
> For an actual lexer: currently there is no real lexer for SQL commands in
> pgbench, the line is just taken as is, so that would mean adding another one,
> although probably a simplified one would do.
Probably not; we'd have to borrow psql's, hook line and sinker. Even if
you could come up with something creative that only failed occasionally,
it would be better from a maintenance perspective if it were as much
like the existing lexers as possible.
(In this context it's worth pointing out that we already have trouble
with keeping psql's and ecpg's lexers in step with the core code.
Adding yet another one, not quite like any of the other ones, doesn't
seem appetizing. If flex were better at factorizing .l files maybe
this wouldn't be quite so painful, but it ain't :-()
I tend to agree with the bottom line that that's just more complication
than is justified. I sympathize with Robert's dislike for backslash
continuations; but doing it the other way would be a huge amount of
up-front work and a nontrivial amount of continuing maintenance, for which
we would not get thanks from users but rather bitching about how we broke
their scripts. Seems like a lose-lose situation.
regards, tom lane
Hello Robert, >> Also, having ";" as a end of commands could also help by allowing multiline >> commands, but that would break compatibility. Maybe allowing continuations >> (\\\n) would be an acceptable compromise. > I loathe violently the convention of using a backslash at the end of a > line, because it's too easy to write backslash-space-newline or > backslash-tab-newline when you meant to write backslash-newline. But > maybe we should do it anyway. We certainly need some solution to that > problem, because the status quo is monumentally annoying, and that might > be the least bad solution available. I survive with that in bash/make/python... > Another option, breaking backward compatibility, would be to decide > that backslash commands have to be terminated by a semicolon token. I do not like it much, as it is inconsistent/incompatible with "psql". > [...] multi-line SQL queries. If we wanted to make that work, the best > option might be to duplicate the backend lexer into pgbench just as we > already do with psql. [...] > > I somewhat lean toward this second option, because I think it will be > a lot more convenient in the long run. We'll probably get some > complains about breaking people's pgbench scripts, but I'd personally > be prepared to accept that as the price of progress. For an actual lexer: currently there is no real lexer for SQL commands in pgbench, the line is just taken as is, so that would mean adding another one, although probably a simplified one would do. To conclude, I'm rather for continuations, despite their ugliness, because (1) it is much easier (just a very small change in read_line_from_file) and (2) it is backward compatible, so no complaints handle. -- Fabien.
> [...] I tend to agree with the bottom line that that's just more > complication than is justified. I sympathize with Robert's dislike for > backslash continuations; but doing it the other way would be a huge > amount of up-front work and a nontrivial amount of continuing > maintenance, for which we would not get thanks from users but rather > bitching about how we broke their scripts. Seems like a lose-lose > situation. I agree. It is a small matter that does not justify a large patch, a greater maintenance burden and breaking compatibility. I've posted a small patch for backslash-continuations as a new thread. -- Fabien.