Обсуждение: 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.

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 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.