| Lists: | pgsql-hackers |
|---|
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | pgbench - add \if support |
| Date: | 2017-11-25 21:33:53 |
| Message-ID: | alpine.DEB.2.20.1711252200190.28523@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
This patch adds \if support to pgbench, similar to psql's version added
in March.
This patch brings a consistent set of features especially when combined
with two other patches already in the (slow) CF process:
- https://commitfest.postgresql.org/10/596/ .. /15/985/
adds support for booleans expressions (comparisons, logical
operators, ...). This enhanced expression engine would be useful
to allow client-side expression in psql.
- https://commitfest.postgresql.org/10/669/ .. /15/669/
adds support for \gset, so that pgbench can interact with a database
and extract something into a variable, instead of discarding it.
This patch adds a \if construct so that an expression on variables,
possibly with data coming from the database, can change the behavior of a
script.
For instance, the following script, which uses features from the three
patches, would implement TPC-B per spec (not "tpcb-like", but really as
specified).
\set tbid random(1, :scale)
\set tid 10 * (:tbid - 1) + random(1, 10)
-- client in same branch as teller at 85%
\if :scale = 1 OR random(0, 99) < 85
-- same branch
\set bid :tbid
\else
-- different branch
\set bid 1 + (:tbid + random(1, :scale - 1)) % :scale
\endif
\set aid :bid * 100000 + random(1, 100000)
\set delta random(-999999, 999999)
BEGIN;
UPDATE pgbench_accounts
SET abalance = abalance + :delta WHERE aid = :aid
RETURNING abalance AS balance \gset
UPDATE pgbench_tellers
SET tbalance = tbalance + :delta WHERE tid = :tid;
UPDATE pgbench_branches
SET bbalance = bbalance + :delta WHERE bid = :bid;
INSERT INTO pgbench_history (tid, bid, aid, delta, mtime)
VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP);
END;
The patch moves the conditional stack infrastructure from psql to
fe_utils, so that it is available to both psql & pgbench.
A partial evaluation is performed to detect structural errors (eg missing
endif, else after else...) when the script is parsed, so that such errors
cannot occur when a script is running.
A new automaton state is added to quickly step over false branches.
TAP tests ensure reasonable coverage of the feature.
--
Fabien
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-1.patch | text/x-diff | 39.3 KB |
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2017-12-14 12:15:16 |
| Message-ID: | alpine.DEB.2.20.1712141314150.13653@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Mostly a rebase after zipfian function commit.
> This patch adds \if support to pgbench, similar to psql's version added in
> March.
>
> This patch brings a consistent set of features especially when combined with
> two other patches already in the (slow) CF process:
>
> - https://commitfest.postgresql.org/10/596/ .. /15/985/
> adds support for booleans expressions (comparisons, logical
> operators, ...). This enhanced expression engine would be useful
> to allow client-side expression in psql.
>
> - https://commitfest.postgresql.org/10/669/ .. /15/669/
> adds support for \gset, so that pgbench can interact with a database
> and extract something into a variable, instead of discarding it.
>
> This patch adds a \if construct so that an expression on variables, possibly
> with data coming from the database, can change the behavior of a script.
>
> For instance, the following script, which uses features from the three
> patches, would implement TPC-B per spec (not "tpcb-like", but really as
> specified).
>
> \set tbid random(1, :scale)
> \set tid 10 * (:tbid - 1) + random(1, 10)
> -- client in same branch as teller at 85%
> \if :scale = 1 OR random(0, 99) < 85
> -- same branch
> \set bid :tbid
> \else
> -- different branch
> \set bid 1 + (:tbid + random(1, :scale - 1)) % :scale
> \endif
> \set aid :bid * 100000 + random(1, 100000)
> \set delta random(-999999, 999999)
> BEGIN;
> UPDATE pgbench_accounts
> SET abalance = abalance + :delta WHERE aid = :aid
> RETURNING abalance AS balance \gset
> UPDATE pgbench_tellers
> SET tbalance = tbalance + :delta WHERE tid = :tid;
> UPDATE pgbench_branches
> SET bbalance = bbalance + :delta WHERE bid = :bid;
> INSERT INTO pgbench_history (tid, bid, aid, delta, mtime)
> VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP);
> END;
>
> The patch moves the conditional stack infrastructure from psql to fe_utils,
> so that it is available to both psql & pgbench.
>
> A partial evaluation is performed to detect structural errors (eg missing
> endif, else after else...) when the script is parsed, so that such errors
> cannot occur when a script is running.
>
> A new automaton state is added to quickly step over false branches.
>
> TAP tests ensure reasonable coverage of the feature.
>
>
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-2.patch | text/x-diff | 39.3 KB |
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2017-12-29 21:53:51 |
| Message-ID: | alpine.DEB.2.20.1712292251410.30681@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Another rebase after the pow function commit.
> Mostly a rebase after zipfian function commit.
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-3.patch | text/x-diff | 39.2 KB |
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-04 06:32:05 |
| Message-ID: | alpine.DEB.2.20.1801040731080.20034@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Another rebase to try to please the patch tester.
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-4.patch | text/x-diff | 39.2 KB |
| From: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-04 12:35:25 |
| Message-ID: | ba5505bc-eb20-12ee-a7f8-f1564c7ac35a@2ndquadrant.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On 01/04/2018 07:32 AM, Fabien COELHO wrote:
>
> Another rebase to try to please the patch tester.
Thank you. I plan on reviewing this over the weekend.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
| From: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
|---|---|
| To: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
| Cc: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-08 17:39:49 |
| Message-ID: | CA+q6zcUCW=shzAZot_Tdmt3uOXA08cR_MfeT0V5ouh5BhOmXvw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> On 4 January 2018 at 07:32, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>
> Another rebase to try to please the patch tester.
Thanks for working on this. I had just a quick look at it, but I hope I'll have
time to post a proper review. In the meantime I'm wondering what am I doing
wrong here (I see a similar example in your first message)?
```
-- test.sql
\if random(0, 99) < 85
\set test 1
\else
\set test 2
\endif
select :test;
```
```
$ pgbench -s 10 -f test.sql
test.sql:1: unexpected character (<) in command "if"
\if random(0, 99) < 85
^ error found here
```
I'm using `pgbench-if-4.patch`, and everything is fine for simple
conditions like `\if 1`.
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-08 18:36:41 |
| Message-ID: | alpine.DEB.2.20.1801081927360.22243@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hello Dmitry,
> Thanks for working on this. I had just a quick look at it, but I hope
> I'll have time to post a proper review. In the meantime I'm wondering
> what am I doing wrong here (I see a similar example in your first
> message)?
>
> ```
> -- test.sql
> \if random(0, 99) < 85
> \set test 1
> \else
> \set test 2
> \endif
> select :test;
> ```
>
> ```
> $ pgbench -s 10 -f test.sql
> test.sql:1: unexpected character (<) in command "if"
> \if random(0, 99) < 85
> ^ error found here
Sure.
What you are trying to do is the result of combining the pgbench-if patch
and the pgbench-more-ops-and-funcs patch.
There is also with the ability to put the result of a SELECT into a
variable, which would then enable doing some if about data coming
from the database.
https://commitfest.postgresql.org/16/985/
https://commitfest.postgresql.org/16/669/
https://commitfest.postgresql.org/16/1385/
These are distinct entries in the CF, because they do quite distinct
things, and interact weakly one with the other.
However, it really makes full sense when they are all available together,
so I put an example which combines all three. "\if 1" is not that
interesting in itself, obviously.
Everytime I sent a (relatively) big patch in the past I was asked to cut
it in bites, which is tiresome when everything is intermixed, so now I'm
doing the bytes first.
--
Fabien.
| From: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-09 11:16:04 |
| Message-ID: | CA+q6zcW_LUGzR2wsVqpXmWiXnLnZNLV4Ld4761P1FQdqv9HVfw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> On 8 January 2018 at 19:36, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> What you are trying to do is the result of combining the pgbench-if patch
> and the pgbench-more-ops-and-funcs patch.
Oh, I see. I missed the first message about this patch, sorry. But for some
reason I can't apply both patches (pgbench-more-ops-funcs-23.patch and
pgbench-if-4.patch) in any order:
Hunk #24 FAILED at 2824.
Hunk #25 succeeded at 5178 (offset 251 lines).
1 out of 25 hunks FAILED -- saving rejects to file
src/bin/pgbench/pgbench.c.rej
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/bin/pgbench/pgbench.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/bin/pgbench/t/001_pgbench_with_server.pl
Hunk #2 FAILED at 226.
Hunk #3 FAILED at 287.
Hunk #4 succeeded at 448 (offset 41 lines).
Hunk #5 succeeded at 505 (offset 41 lines).
Hunk #6 succeeded at 516 (offset 41 lines).
2 out of 6 hunks FAILED -- saving rejects to file
src/bin/pgbench/t/001_pgbench_with_server.pl.rej
Is there any other dependency I should apply?
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-09 13:46:24 |
| Message-ID: | alpine.DEB.2.20.1801091403530.6718@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hello Dmitry,
>> What you are trying to do is the result of combining the pgbench-if patch
>> and the pgbench-more-ops-and-funcs patch.
>
> Oh, I see. I missed the first message about this patch, sorry. But for some
> reason I can't apply both patches (pgbench-more-ops-funcs-23.patch and
> pgbench-if-4.patch) in any order:
> Hunk #24 FAILED at 2824. [...]
Indeed, they interfere.
> Is there any other dependency I should apply?
No, the patches really conflict in minor ways. They are expected to be
reviewed independently. If/when one feature is committed, I('ll) update
the remaining patches so that they still work.
If I produce dependent patches ISTM that it makes a more complicated
review, so the patches are less likely to be reviewed and maybe finally
committed.
I just wanted to point out that the the features are more interestings
when combined than on their own.
--
Fabien.
| From: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-09 14:02:15 |
| Message-ID: | 5f12699a-8ec0-8047-14fd-1d49c85f5a44@2ndquadrant.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On 11/25/2017 10:33 PM, Fabien COELHO wrote:
>
> This patch adds \if support to pgbench, similar to psql's version added
> in March.
>
> This patch brings a consistent set of features especially when combined
> with two other patches already in the (slow) CF process:
>
> - https://commitfest.postgresql.org/10/596/ .. /15/985/
> adds support for booleans expressions (comparisons, logical
> operators, ...). This enhanced expression engine would be useful
> to allow client-side expression in psql.
>
> - https://commitfest.postgresql.org/10/669/ .. /15/669/
> adds support for \gset, so that pgbench can interact with a database
> and extract something into a variable, instead of discarding it.
>
> This patch adds a \if construct so that an expression on variables,
> possibly with data coming from the database, can change the behavior of
> a script.
I have given this patch a pretty good shake and I'm happy with it. I
did not test it with the other two patches, only on its own.
> A partial evaluation is performed to detect structural errors (eg
> missing endif, else after else...) when the script is parsed, so that
> such errors cannot occur when a script is running.
Very good.
> A new automaton state is added to quickly step over false branches.
This one took me a little while to understand while reading the patch,
but mostly because of how diff doesn't handle moving things around.
> TAP tests ensure reasonable coverage of the feature.
And the documentation seems sufficient, as well.
It's a shame this feature uses \elif instead of \elsif to be closer to
plpgsql, but I suppose this ship already sailed when psql chose \elif,
and I think it is correct that this patch follows psql.
Marking as ready for committer.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
| Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-09 14:36:20 |
| Message-ID: | alpine.DEB.2.20.1801091532430.6718@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hello Vik,
>> This patch adds a \if construct so that an expression on variables,
>> possibly with data coming from the database, can change the behavior of
>> a script.
>
> I have given this patch a pretty good shake and I'm happy with it. I
> did not test it with the other two patches, only on its own.
As noted by Dmitry, they intefere slightly one with the other so it
would require some conflict resolution.
[...]
> Marking as ready for committer.
Ok, thanks.
--
Fabien.
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
| Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-10 08:19:08 |
| Message-ID: | alpine.DEB.2.20.1801100916340.2283@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
>> A new automaton state is added to quickly step over false branches.
>
> This one took me a little while to understand while reading the patch,
> but mostly because of how diff doesn't handle moving things around.
"git diff -w --patience" may help.
> Marking as ready for committer.
Here is a rebase. I made some tests use actual expressions instead of just
0 and 1. No other changes.
--
Fabien.
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
| Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-10 08:21:50 |
| Message-ID: | alpine.DEB.2.20.1801100920470.2283@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> Here is a rebase. I made some tests use actual expressions instead of just 0
> and 1. No other changes.
Sigh. Better with the attachment. Sorry for the noise.
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-5.patch | text/x-diff | 39.1 KB |
| From: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
| Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-12 16:14:22 |
| Message-ID: | 952cbd30-c703-e322-ba65-2cd951ccbe7a@sigaev.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi!
Hm, isn't already commited when/case/then/else syntax do the same? If not, could
it be added to existing synax?
Fabien COELHO wrote:
>
>> Here is a rebase. I made some tests use actual expressions instead of just 0
>> and 1. No other changes.
>
> Sigh. Better with the attachment. Sorry for the noise.
>
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-12 18:29:38 |
| Message-ID: | alpine.DEB.2.20.1801121920140.15795@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> Hm, isn't already commited when/case/then/else syntax do the same?
No, not strictly. The "CASE WHEN" is an if *within* an expression:
\set i CASE WHEN condition THEN val1 ELSE val2 END
The \if is at the script level, like psql already available version, which
can change what SQL is sent.
\if condition
SOME SQL
\else
OTHER SQL
\endif
You could achieve the CASE semantics with some \if:
\if condition
\set i val1
\else
\set i val2
\endif
But the reverse is not possible.
--
Fabien.
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
| Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-15 17:28:09 |
| Message-ID: | alpine.DEB.2.20.1801151827260.11126@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
>> Here is a rebase. I made some tests use actual expressions instead of just
>> 0 and 1. No other changes.
>
> Sigh. Better with the attachment. Sorry for the noise.
Here is a very minor rebase.
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-6.patch | text/x-diff | 38.8 KB |
| From: | Craig Ringer <craig(at)2ndquadrant(dot)com> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-21 22:31:27 |
| Message-ID: | CAMsr+YGHYUsSfzxyJ8UQOB3qKn86zdhfiNzMKWRRVxeiW+ovxA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On 16 January 2018 at 06:28, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>
> Here is a rebase. I made some tests use actual expressions instead of just
>>> 0 and 1. No other changes.
>>>
>>
>> Sigh. Better with the attachment. Sorry for the noise.
>>
>
> Here is a very minor rebase.
>
I'm a smidge worried about this. It seems like psql is growing a scripting
language. Do we want to go our own way with a kind of organically grown
scripting system? Or should we be looking at embedding client-side
scripting in a more structured, formal way instead? Embed a lua interpreter
or something?
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Craig Ringer <craig(at)2ndquadrant(dot)com> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-22 05:20:23 |
| Message-ID: | alpine.DEB.2.20.1801220603330.6320@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Helo Craig,
> I'm a smidge worried about this. It seems like psql is growing a
> scripting language.
The patch is about aligning pgbench with psql, which already has \if.
> Do we want to go our own way with a kind of organically grown
> scripting system? Or should we be looking at embedding client-side
> scripting in a more structured, formal way instead? Embed a lua interpreter
> or something?
My 0.02€ is that the point is to deal with useful/needed simple client
capabilities while integrating gracefully with bare server-side executed
SQL.
As for useful client-side capabilities, for both psql & pgbench ISTM that
it is more in line with a limited cpp-like thing: include, expressions,
variables, conditions... maybe minimal error handling. No loop.
As for a language interpreter, it would raise the question of which
language (lua, tcl, python, perl, VB, sh, R, ...) and the graceful (upward
compatible) integration of any such language: eg how do have pieces of
bare SQL and any other existing language would require some scanning
conventions that do not exist.
psql & pgbench already have ":x" variables. psql has the ability to set
variable from SQL (\gset), and pgbench could do limited expressions to set
these variables with (\set), which have been extended to be more complete
, and there was use cases which motivate an (\if).
ISTM enough to align both tools for reasonnably simple use cases that
could arise when running a basic SQL script of bench. If you have
something really complicated, then full fledge programming is the answer,
which cannot be bare-SQL compatible.
So the answer is that it is okay to aim at "limited" scripting because it
covers useful use cases.
--
Fabien.
| From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
|---|---|
| To: | Craig Ringer <craig(at)2ndquadrant(dot)com> |
| Cc: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-22 06:43:27 |
| Message-ID: | CAFj8pRB=e4QCzotmnp4sCdhE8F0Y1e6yXUho_6eioPTP_PCz8A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
2018-01-21 23:31 GMT+01:00 Craig Ringer <craig(at)2ndquadrant(dot)com>:
> On 16 January 2018 at 06:28, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>
>>
>> Here is a rebase. I made some tests use actual expressions instead of
>>>> just 0 and 1. No other changes.
>>>>
>>>
>>> Sigh. Better with the attachment. Sorry for the noise.
>>>
>>
>> Here is a very minor rebase.
>>
>
> I'm a smidge worried about this. It seems like psql is growing a scripting
> language. Do we want to go our own way with a kind of organically grown
> scripting system? Or should we be looking at embedding client-side
> scripting in a more structured, formal way instead? Embed a lua interpreter
> or something?
>
few scripting features doesn't mean scripting language. \if in psql is nice
feature that reduce duplicate code, unreadable code, and helps with
deployment and test scripts. pgbench and psql should to have similar
environment - and I am thinking so \if should be there.
Using Lua is not bad idea - in psql too - I though about it much, but in
this case is better to start from zero.
Regards
Pavel
>
> --
> Craig Ringer http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
| Cc: | Craig Ringer <craig(at)2ndquadrant(dot)com>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-22 09:45:19 |
| Message-ID: | alpine.DEB.2.20.1801221006110.22852@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> few scripting features doesn't mean scripting language. \if in psql is nice
> feature that reduce duplicate code, unreadable code, and helps with
> deployment and test scripts. pgbench and psql should to have similar
> environment - and I am thinking so \if should be there.
>
> Using Lua is not bad idea - in psql too - I though about it much, but in
> this case is better to start from zero.
Yep. Having another versatile (interactive) client would not be a bad
thing. I'm still wondering how to conciliate any scripting language with
"bare SQL". The backslash-oriented syntax already used for psql & pgbench
seems the only available option. Otherwise ISTM that it is back to a
standard library oriented client access with import, connect, exec...
whatever set of function already provided by standard libraries (psycopg
for python, ...).
--
Fabien.
| From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | Craig Ringer <craig(at)2ndquadrant(dot)com>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-01-22 09:57:46 |
| Message-ID: | CAFj8pRD0ZOWRKBBetFxOgLXZx5_L8jU2YmWdiuNwqSn-dKur8A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
2018-01-22 10:45 GMT+01:00 Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>:
>
> few scripting features doesn't mean scripting language. \if in psql is nice
>> feature that reduce duplicate code, unreadable code, and helps with
>> deployment and test scripts. pgbench and psql should to have similar
>> environment - and I am thinking so \if should be there.
>>
>> Using Lua is not bad idea - in psql too - I though about it much, but in
>> this case is better to start from zero.
>>
>
> Yep. Having another versatile (interactive) client would not be a bad
> thing. I'm still wondering how to conciliate any scripting language with
> "bare SQL". The backslash-oriented syntax already used for psql & pgbench
> seems the only available option. Otherwise ISTM that it is back to a
> standard library oriented client access with import, connect, exec...
> whatever set of function already provided by standard libraries (psycopg
> for python, ...).
>
The implementation of some parts in C is frustrating - mainly tab complete.
There is not possibility to create own backslash command - or enhance
buildin commands. Is not possible to customize output.
So some hypothetical client can be implemented like some core C module -
for fast processing of tabular data and all other can be implemented in
Lua. I can imagine so this client can support some input forms, for bar
menu, for some simple reports. It can be more like FoxPro client than
command line only client. In few years we can use ncurses everywhere, and
then there are possibility to write rich TUI client.
Regards
Pavel
> --
> Fabien.
>
| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-01 08:26:45 |
| Message-ID: | 20180301082645.4oe2lsj2gmmo65mf@alap3.anarazel.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi,
On 2018-01-15 18:28:09 +0100, Fabien COELHO wrote:
> Here is a very minor rebase.
Your recent patches all seem to have windows line-endings. It'd be good
to fix that for the future...
- Andres
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-01 10:21:20 |
| Message-ID: | alpine.DEB.2.20.1803011107190.7903@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hello Andres,
>> Here is a very minor rebase.
>
> Your recent patches all seem to have windows line-endings.
There does not seem to be any on my side:
sh> hexdump pgbench-if-6.patch | grep '0[ad]' | head -3
0000040 6562 636e 2e68 6773 6c6d 690a 646e 7865
0000060 2031 3031 3630 3434 2d0a 2d2d 6120 642f
0000080 6770 6562 636e 2e68 6773 6c6d 2b0a 2b2b
There are a few LF (0a) but no CR (0d) visible.
I'd guess that you are running on windows, that the mime-type of the
attachement is "text/x-diff", which is what my ubuntu box lists as
appropriate for "*.patch", and that because of "text/" your mail client
would have decided to switch "\n" to "\r\n" on its own?
> It'd be good to fix that for the future...
What should I do about transformations on the receiving side?
--
Fabien.
| From: | "Daniel Verite" <daniel(at)manitou-mail(dot)org> |
|---|---|
| To: | "Fabien COELHO" <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | "Andres Freund" <andres(at)anarazel(dot)de>,"Vik Fearing" <vik(dot)fearing(at)2ndquadrant(dot)com>,"PostgreSQL Developers" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-01 16:57:22 |
| Message-ID: | 35b0d650-f6d4-4337-8fe0-46d201d75a82@manitou-mail.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Fabien COELHO wrote:
> There does not seem to be any on my side:
>
> sh> hexdump pgbench-if-6.patch | grep '0[ad]' | head -3
> 0000040 6562 636e 2e68 6773 6c6d 690a 646e 7865
> 0000060 2031 3031 3630 3434 2d0a 2d2d 6120 642f
> 0000080 6770 6562 636e 2e68 6773 6c6d 2b0a 2b2b
FYI when looking at the corresponding raw mailfile as we
subscribers receive it, here's what I got:
(for Message-ID: <alpine(dot)DEB(dot)2(dot)20(dot)1801151827260(dot)11126(at)lancre>)
==== start of raw mailfile excerpt ====
Here is a very minor rebase.
--
Fabien.
--8323329-978247287-1516037290=:11126
Content-Type: text/x-diff; name=pgbench-if-6.patch
Content-Transfer-Encoding: BASE64
Content-ID: <alpine(dot)DEB(dot)2(dot)20(dot)1801151828090(dot)11126(at)lancre>
Content-Description:
Content-Disposition: attachment; filename=pgbench-if-6.patch
ZGlmZiAtLWdpdCBhL2RvYy9zcmMvc2dtbC9yZWYvcGdiZW5jaC5zZ21sIGIv
ZG9jL3NyYy9zZ21sL3JlZi9wZ2JlbmNoLnNnbWwNCmluZGV4IDNkZDQ5MmMu
LmMyMDNjNDEgMTAwNjQ0DQotLS0gYS9kb2Mvc3JjL3NnbWwvcmVmL3BnYmVu
Y2guc2dtbA0KKysrIGIvZG9jL3NyYy9zZ21sL3JlZi9wZ2JlbmNoLnNnbWwN
CkBAIC04OTUsNiArODk1LDIxIEBAIHBnYmVuY2ggPG9wdGlvbmFsPiA8cmVw
bGFjZWFibGU+b3B0aW9uczwvcmVwbGFjZWFibGU+IDwvb3B0aW9uYWw+IDxy
==== cut here ====
Now this base64 part piped through `base64 -d` and then `hexdump -C`
gives:
00000000 64 69 66 66 20 2d 2d 67 69 74 20 61 2f 64 6f 63 |diff --git
a/doc|
00000010 2f 73 72 63 2f 73 67 6d 6c 2f 72 65 66 2f 70 67
|/src/sgml/ref/pg|
00000020 62 65 6e 63 68 2e 73 67 6d 6c 20 62 2f 64 6f 63 |bench.sgml
b/doc|
00000030 2f 73 72 63 2f 73 67 6d 6c 2f 72 65 66 2f 70 67
|/src/sgml/ref/pg|
00000040 62 65 6e 63 68 2e 73 67 6d 6c 0d 0a 69 6e 64 65
|bench.sgml..inde|
00000050 78 20 33 64 64 34 39 32 63 2e 2e 63 32 30 33 63 |x
3dd492c..c203c|
00000060 34 31 20 31 30 30 36 34 34 0d 0a 2d 2d 2d 20 61 |41 100644..---
a|
00000070 2f 64 6f 63 2f 73 72 63 2f 73 67 6d 6c 2f 72 65
|/doc/src/sgml/re|
00000080 66 2f 70 67 62 65 6e 63 68 2e 73 67 6d 6c 0d 0a
|f/pgbench.sgml..|
00000090 2b 2b 2b 20 62 2f 64 6f 63 2f 73 72 63 2f 73 67 |+++
b/doc/src/sg|
000000a0 6d 6c 2f 72 65 66 2f 70 67 62 65 6e 63 68 2e 73
|ml/ref/pgbench.s|
000000b0 67 6d 6c 0d 0a 40 40 20 2d 38 39 35 2c 36 20 2b |gml..@@ -895,6
+|
Line endings are already 0d 0a at this point, which normally means
that they were like that in the mail as you submitted it.
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Daniel Verite <daniel(at)manitou-mail(dot)org> |
| Cc: | Andres Freund <andres(at)anarazel(dot)de>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-01 17:47:04 |
| Message-ID: | alpine.DEB.2.20.1803011839490.7903@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> 00000040 62 65 6e 63 68 2e 73 67 6d 6c 0d 0a 69 6e 64 65
Hmmm. "0d 0a" looks bad.
I've meddled into "/etc/mime.types" to change "text/x-diff" to
"text/plain" used by some other mailers. Here is the v6 version again,
size is 38424 and md5sum is 63d79c0d5a93294f002edc640a3f525b.
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-6.patch | text/plain | 38.8 KB |
| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | Daniel Verite <daniel(at)manitou-mail(dot)org>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-01 18:06:22 |
| Message-ID: | 20180301180622.aeccicklh2d3owi6@alap3.anarazel.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi Fabien, Daniel,
On 2018-03-01 18:47:04 +0100, Fabien COELHO wrote:
>
> > 00000040 62 65 6e 63 68 2e 73 67 6d 6c 0d 0a 69 6e 64 65
Thanks for the analysis.
> Hmmm. "0d 0a" looks bad.
It does. TBQH the aesthetics of seing ^M's all over is what made me
complain ;)
> I've meddled into "/etc/mime.types" to change "text/x-diff" to "text/plain"
> used by some other mailers. Here is the v6 version again, size is 38424 and
> md5sum is 63d79c0d5a93294f002edc640a3f525b.
This does look better. Not quite sure what the problem is, but ...
Greetings,
Andres Freund
| From: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-21 14:23:58 |
| Message-ID: | 4c1dd23c-a846-4954-8635-cc61b9c51de6@sigaev.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> No, not strictly. The "CASE WHEN" is an if *within* an expression:Okay, I see.
Patch seems usefull and commitable except comments in conditional.[ch]. I'd like
to top/header comment in each file more detailed and descriptive. As for now it
mentions only psql usage without explaining how it is basic or common.
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/
| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-21 18:10:23 |
| Message-ID: | alpine.DEB.2.20.1803211841500.1860@lancre |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hello Teodor,
> Patch seems usefull and commitable except comments in conditional.[ch]. I'd
> like to top/header comment in each file more detailed and descriptive. As for
> now it mentions only psql usage without explaining how it is basic or common.
Indeed, it was not updated.
I've fixed the file names and added a simple description at the beginning
of the header file, and a one liner in the code file.
Do you think that more is needed?
The patch also needed a rebase after the hash function addition.
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-if-7.patch | text/plain | 39.4 KB |
| From: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
|---|---|
| To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
| Cc: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgbench - add \if support |
| Date: | 2018-03-22 14:44:21 |
| Message-ID: | 2241beca-1491-6e0d-b457-f593b24a255d@sigaev.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Thank you, pushed
Fabien COELHO wrote:
>
> Hello Teodor,
>
>> Patch seems usefull and commitable except comments in conditional.[ch]. I'd
>> like to top/header comment in each file more detailed and descriptive. As for
>> now it mentions only psql usage without explaining how it is basic or common.
>
> Indeed, it was not updated.
>
> I've fixed the file names and added a simple description at the beginning of the
> header file, and a one liner in the code file.
>
> Do you think that more is needed?
>
> The patch also needed a rebase after the hash function addition.
>
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/