Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-30 Thread Fabien COELHO
Hello, Hmmm. The existing "is_no_vacuum" variable is typed *both* as int (in "main") and as bool (in "init"), called by main (yuk!). I see no reason to choose the bad one for the global:-) Yeah, I think this might be a good timing to re-consider int-for-bool habits in pgbench. If we decided

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-29 Thread Fabien COELHO
Hello, Patch applies and works. I would like to insist a little bit: the name was declared as an int but passed to init and used as a bool there before the patch. Conceptually what is meant is really a bool, and I see no added value bar saving a few strokes to have an int. ISTM that recent

Re: [HACKERS] psql --batch

2017-08-28 Thread Fabien COELHO
I don't doubt about a sense of this configuration - but this specific combination depends on usage - so I don't think so using special option is good idea. Although I agree with you that detailed settings are definitely debatable, I'd say that at least it would be a more reasonable starting

Re: [HACKERS] psql --batch

2017-08-28 Thread Fabien COELHO
This doesn't really address the original issue though, that it's far from obvious how to easily and correctly script psql. That is another interesting argument. I understood that the issue was having to type these options, but now it is also to remember which one are relevant and wanted,

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-28 Thread Fabien COELHO
Hello Masahiko-san, Patch applies cleanly, compiles, works for me. At least: "Required to invoke" -> "Require to invoke". Fixed. I meant the added one about -I, not the pre-existing one about -i. About the code: is_no_vacuum should be a bool? We can change it but I think there is no

Re: [HACKERS] psql --batch

2017-08-28 Thread Fabien COELHO
ISTM that the real pain is the "-v ON_ERRORS_STOP=1" which I occasionally encountered as well, the other one letter ones are not too bad. Maybe it would be enough to have a shortcut for this one, say "-B"? I agree with last sentence. I don't think so -qAtX are expected always, but "-v

Re: [HACKERS] psql --batch

2017-08-28 Thread Fabien COELHO
I find myself regurgitating the incantation psql -qAtX -v ON_ERRORS_STOP=1 quite a bit. It's not ... super friendly. It strikes me that we could possibly benefit from a 'psql --batch' option. Thoughts? The link between -qAtX and "batch" is not that fully obvious, especially the unaligned

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-08-27 Thread Fabien COELHO
Hello, Here is the third version of the patch for pgbench thanks to Fabien Coelho comments. As in the previous one, transactions with serialization and deadlock failures are rolled back and retried until they end successfully or their number of tries reaches maximum. Here is some partial

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-27 Thread Fabien COELHO
Quick precision to my previous review. sh> ./pgbench -i -I t done. There should be "creating tables..." Does not seem to have initialized the tables although it was requested... sh> ./pgbench -i -I d creating tables... Probably "filling tables..." would be more appropriate. -- Fabien.

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-27 Thread Fabien COELHO
Hello Masahiko-san, Attached latest v4 patch. Please review it. Patch applies, compiles. The messages/options do not seem to work properly: sh> ./pgbench -i -I t done. Does not seem to have initialized the tables although it was requested... sh> ./pgbench -i -I d creating tables...

Re: [HACKERS] pgbench: faster version of tpcb-like transaction

2017-08-27 Thread Fabien COELHO
About the patch: I'm generally in favor of providing more options to pgbench, especially if it can give optimization ideas to the performance conscious user. I think that the name should be "tpcb-like-plfunc": the script does not implement tpcb per spec, and such a function could be written

Re: [HACKERS] Variable substitution in psql backtick expansion

2017-08-27 Thread Fabien COELHO
Spending developer time to write code for the hypothetical someone running a psql version 11 linked to a libpq < 7.4, if it can even link, does not look like a very good investment... Anyway, here is required the update. The question is the server's version, not libpq. Ok. Modern psql does

Re: [HACKERS] pgbench: faster version of tpcb-like transaction

2017-08-27 Thread Fabien COELHO
Hello Tom, I dunno, it seems like this proposal involves jacking up the test case and driving a completely different one underneath. There is no reason to consider that you've improved the benchmark results --- you've just substituted a different benchmark, one with no historical basis, and

Re: [HACKERS] pgbench: faster version of tpcb-like transaction

2017-08-26 Thread Fabien COELHO
Hello, If all the data is in memory and you have a system with fast fsyncs (or are running with fsync off, or unlogged tables, or synchronous_commit off), then the big bottleneck in pgbench is the amount of back and forth between the pgbench program and the backend. Sure. The throughput of

Re: [HACKERS] psql - add ability to test whether a variable exists

2017-08-26 Thread Fabien COELHO
Here is a version with the :{?varname} syntax. It looks much better for me. I'll admit that it looks better to me as well:-) -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Variable substitution in psql backtick expansion

2017-08-26 Thread Fabien COELHO
Hello Tom, I think you are taking unreasonable shortcuts here: + SetVariable(pset.vars, "SERVER_VERSION_NAME", PQparameterStatus(pset.db, "server_version")); The existing code in connection_warnings() does this: const char *server_version; /* Try to get full

Re: [HACKERS] psql - add ability to test whether a variable exists

2017-08-26 Thread Fabien COELHO
Any colon prefixed syntax can be made to work because it is enough for the lexer to detect and handle... so :{defined varname} may be an option, although I do not like the space much because it adds some fuzzyness in the lexer which has to process it. It is probably doable, though. I like

Re: [HACKERS] Variable substitution in psql backtick expansion

2017-08-26 Thread Fabien COELHO
Hello Tom, I understand that you would prefer VERSION_NAME to show something like "11devel, server 9.6.4" No, that's not what I said. I'm just complaining that as the patch stands it will set SERVER_NAME to "11.0", where I think it should say "11devel" (as of today). Ok. [...]

Re: [HACKERS] psql - add ability to test whether a variable exists

2017-08-26 Thread Fabien COELHO
Hello Pavel, As a follow-up to the \if patch by Corey Huinker, here is a proposal to allow testing whether a client-side variable exists in psql. The syntax is as ugly as the current :'var' and :"var" things, but ISTM that this is the only simple option to have a working SQL-compatible syntax

[HACKERS] psql - add ability to test whether a variable exists

2017-08-26 Thread Fabien COELHO
Hello, As a follow-up to the \if patch by Corey Huinker, here is a proposal to allow testing whether a client-side variable exists in psql. The syntax is as ugly as the current :'var' and :"var" things, but ISTM that this is the only simple option to have a working SQL-compatible syntax

Re: [HACKERS] Variable substitution in psql backtick expansion

2017-08-26 Thread Fabien COELHO
Hello Tom, ... I'm still not sure that there's any use case for the string versions ("9.6.4" etc). If somebody's doing comparisons, they probably want the numeric version, but somebody might want to print the string version in an error message e.g. \if \echo this thing doesn't work on

Re: [HACKERS] proposal: psql command \graw

2017-08-25 Thread Fabien COELHO
Argh, sorry, I put it in the September commitfest, and it seems that it cannot be changed afterwards. Maybe you can close it and redeclare it in the commitfest you want? It can be moved Indeed it can. Feel free to move it, then. -- Fabien. -- Sent via pgsql-hackers mailing list

Re: [HACKERS] proposal: psql command \graw

2017-08-24 Thread Fabien COELHO
Hello, I'll wait to winter commitfest Argh, sorry, I put it in the September commitfest, and it seems that it cannot be changed afterwards. Maybe you can close it and redeclare it in the commitfest you want? for some other ideas, tests, comments - it is topic for PostgreSQL 11, and then

Re: [HACKERS] proposal: psql command \graw

2017-08-24 Thread Fabien COELHO
"column_header" is somehow redundant with "tuples_only". Use the existing one instead of adding a new one? It is different - a result of tuples_only is just tuples - not column names, not title, footer. I needed new special flag for enhancing tuples_only to print column names I do not

Re: [HACKERS] proposal: psql command \graw

2017-08-23 Thread Fabien COELHO
Hello Pavel, I have added the patch to the next commitfest. Patch applies, compiles, works. I'm okay with the names graw/graw+, and for having such short-hands. Missing break in switch, even if last item and useless, because other items do it... Also should be added at its place in

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-08-23 Thread Fabien COELHO
Hello Alik, I am attaching patch v7. Patch generates multiple warnings with "git apply", apparently because of end-of-line spaces, and fails: pgbench-zipf-07v.patch:52: trailing whitespace. { pgbench-zipf-07v.patch:53: trailing whitespace. "random_zipfian", 3,

Re: [HACKERS] pgbench tap tests & minor fixes

2017-08-23 Thread Fabien COELHO
While reviewing another patch, I figured out at least one potential issue on windows when handling execution status. The attached v11 small updates is more likely to pass on windows. -- Fabien.diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l index dc1367b..8255eff 100644

Re: [HACKERS] proposal: psql command \graw

2017-08-22 Thread Fabien COELHO
Hello Pavel, One my idea is introduction new simple output format and execution command with result in this format. It should work something like \setenv GNUPLOT_OPTION '..' SELECT * FROM data \graw | gnuplot ... I understand that it is kind of a shortcut for: \pset fieldsep ' '

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-22 Thread Fabien COELHO
Why not allow -I as a short option for --custom-initialize? Other options for similar purpose such as --foreign-keys also have only a long option. Since I think --custom-initialize option is in the same context as other options I didn't add short option to it for now. Because the options name

Re: [HACKERS] pgbench tap tests & minor fixes

2017-08-20 Thread Fabien COELHO
Hello Nikolay, I've applied the patch to the current master, and it seems that one test now fails: Indeed, Tom removed the -M option order constraint, so the test which check for that does not apply anymore. Here is a v10 without this test. -- Fabien.diff --git

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-16 Thread Fabien COELHO
Hello Masahiko-san, Yeah, once custom initialization patch get committed we can extend it. Attached updated patch. I've incorporated the all comments from Fabien to it, and changed it to single letter version. Patch applies and works. A few comments and questions about the code and

Re: [HACKERS] pgbench more operators & functions

2017-08-15 Thread Fabien COELHO
Here is a rebase. Argh, sorry, missing attachement... Here it is really. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 03e1212..520daae 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -825,14 +825,31 @@ pgbench

Re: [HACKERS] pgbench more operators & functions

2017-08-15 Thread Fabien COELHO
Hello Peter, On 5/24/17 03:14, Fabien COELHO wrote: I've improved it in attached v11: - add a link to the CASE full documentation - add an example expression with CASE ... This patch needs (at least) a rebase for the upcoming commit fest. Here is a rebase. It still passes my tests

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-14 Thread Fabien COELHO
Hello, I think we can use it like --custom-initialize="create_table, vacuum" which is similar to what we specify a connection option to psql for example. Even if it is allowed, do not advertise it. Or use space as a separator and not comma. ISTM that with psql connections space is the

Re: [HACKERS] pgbench - allow to store select results into variables

2017-08-13 Thread Fabien COELHO
Here is a v11. It is basically a simple rebase after Tom committed the "pgbench -M order" patch. It interfered because the compound command management also needs to delay part of the SQL command initialization. Some patch are luckier than others:-) Here is a v10: - does not talk about

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-08-13 Thread Fabien COELHO
Hello Alik, Now “a” does not have upper bound, that’s why on using iterative algorithm with a >= 1 program will stuck on infinite loop because of following line of code: double b = pow(2.0, s - 1.0); Because after overflow “b” becomes “+Inf”. Yep, overflow can happen. So should upper

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-08 Thread Fabien COELHO
Hello Mahahiko-san, My 0.02€ about the patch & feature, which only reflect my point of view: Please add a number to patches to avoid ambiguities. This was patch was the second sent on the thread. There is no need to have both custom_init & init functions. I'll suggest to remove function

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-08 Thread Fabien COELHO
Attached patch I'll look into it. -- 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] Row Level Security Documentation

2017-08-05 Thread Fabien COELHO
Hello Rod, Patch applies cleanly, make html ok, new table looks good to me. I've turned it "Ready for Committer". Thanks! -- 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] [WIP] Zipfian distribution in pgbench

2017-08-05 Thread Fabien COELHO
Hello Alik, So I would be in favor of expanding the documentation but not restricting the parameter beyond avoiding value 1.0. I have removed restriction and expanded documentation in attaching patch v5. I've done some math investigations, which consisted in spending one hour with

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-08-05 Thread Fabien COELHO
Hello Peter, I think that it would also be nice if there was an option to make functions like random_zipfian() actually return a value that has undergone perfect hashing. When this option is used, any given value that the function returns would actually be taken from a random mapping to

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-03 Thread Fabien COELHO
For the CREATE stuff, the script language is SQL, the command to use it is "psql"... The real and hard part is to fill tables with meaningful pseudo-random test data which do not violate constraints for any non trivial schema involving foreign keys and various unique constraints. The

Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization

2017-08-03 Thread Fabien COELHO
Hello, My motivation of this proposal is same as what Robert has. I understand that ad-hoc option can solve only the part of big problem and it could be cause of mess. However It seems me that the script especially for table initialization will not be flexible than we expected. I mean, even if

Re: [HACKERS] Confusing error message in pgbench

2017-08-03 Thread Fabien COELHO
Indeed. It doesn't look that hard: AFAICS the problem is just that process_sql_command() is making premature decisions about whether to extract parameters from the SQL commands. Proposed patch attached. Great. Patch looks good to me. Too me as well: code looks ok, patch applies, compiles,

Re: [HACKERS] Confusing error message in pgbench

2017-08-02 Thread Fabien COELHO
Hello Tatsuo-san, I found an error message in pgbench is quite confusing. pgbench -S -M extended -c 1 -T 30 test query mode (-M) should be specified before any transaction scripts (-f or -b) Since there's no -f or -b option is specified, users will be confused. Indeed. Actually the error

Re: [HACKERS] pgbench minor doc typo

2017-08-01 Thread Fabien COELHO
Alik Khilazhev is submitting a patch about a zipfian random function for pgbench, and noticed a typo in the documentation about random_exponential. Attached is a fix extracted from his patch submission, which could be applied to head/10/9.6. done Ok, thanks. -- Fabien. -- Sent via

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-20 Thread Fabien COELHO
Hello Alik, About the maths: As already said, I'm not at ease with a random_zipfian function which does not display a (good) zipfian distribution. At the minimum the documentation should be clear about the approximations implied depending on the parameter value. I add one more sentence to

Re: [HACKERS] Adding -E switch to pg_dumpall

2017-07-20 Thread Fabien COELHO
Hello Michaël-san, Attached is a patch to add support for this switch. I am parking that in the next CF. I'm in favor of this feature for consistency with pg_dump, and as the environment variable workaround is not specially elegant and can induce issues of its own. Patch applies and

[HACKERS] pgbench minor doc typo

2017-07-19 Thread Fabien COELHO
Alik Khilazhev is submitting a patch about a zipfian random function for pgbench, and noticed a typo in the documentation about random_exponential. Attached is a fix extracted from his patch submission, which could be applied to head/10/9.6. -- Fabien.diff --git

Re: [HACKERS] merge psql ef/ev sf/sv handling functions

2017-07-19 Thread Fabien COELHO
While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. Thank you for the patch. Is this an item for PG11? Yep. -- Fabien. -- Sent via pgsql-hackers

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-19 Thread Fabien COELHO
Hello Alik, I am attaching patch v3. Among other things I fixed small typo in description of random_exponential function in pgbench.sgml file. Ok. Probably this typo should be committed separatly and independently. A few comments about v3: Patch applies cleanly, compiles, works. About

Re: [HACKERS] merge psql ef/ev sf/sv handling functions

2017-07-18 Thread Fabien COELHO
While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. Thank you for the patch. Is this an item for PG11? Yes, as it is submitted to CF 2017-09. -- Fabien.

Re: [HACKERS] merge psql ef/ev sf/sv handling functions

2017-07-18 Thread Fabien COELHO
Hello Victor, While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. I was looking through your patch. It seems good, the of the functions was very similar.

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-17 Thread Fabien COELHO
Hello, Is this bias expected from the drawing method, say because it is approximated and the approximation is weak at some points, or is there an issue with its implementation, says some shift which gets smoothed down for higher indexes? I have checked paper where such implementation was

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-17 Thread Fabien COELHO
Ok, so you did not get the large bias for i=3. Strange. I got large bias for i=3 and theta > 1 even with a million outcomes, Ok. So this is similar to what I got. Is this bias expected from the drawing method, say because it is approximated and the approximation is weak at some points, or

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-14 Thread Fabien COELHO
Algorithm works with theta less than 1. The only problem here is that theta can not be 1, because of next line of code cell->alpha = 1. / (1 - theta); That’s why I put such restriction. Now I see 2 possible solutions for that: 1) Exclude 1, and allow everything in range (0;+∞). Yep. 2)

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-14 Thread Fabien COELHO
And I'm not sure that we should do all the stuff for savepoints rollbacks because: - as I see it now it only makes sense for the deadlock failures; - if there's a failure what savepoint we should rollback to and start the execution again? ISTM that it is the point of having savepoint in the

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-14 Thread Fabien COELHO
If the answer is no, then implement something in pgbench directly. The structure of variables is different, the container structure of the variables is different, so I think that the answer is no. Ok, fine. My point was just to check before proceeding. -- Fabien. -- Sent via

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-14 Thread Fabien COELHO
Note that there is something for psql (src/bin/psql/variable.c) which may or may not be shared. It should be checked before recoding eventually the same thing. Thank you very much for pointing this file! As I checked this is another structure: here there's a simple list, while in pgbench we

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-14 Thread Fabien COELHO
Hello Marina, Not necessarily? It depends on where the locks triggering the issue are set, if they are all set after the savepoint it could work on a second attempt. Don't you mean the deadlock failures where can really help rollback to Yes, I mean deadlock failures can rollback to a

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-13 Thread Fabien COELHO
I was not convinced by the overall memory management around variables to begin with, and it is even less so with their new copy management. Maybe having a clean "Variables" data structure could help improve the situation. Ok! Note that there is something for psql (src/bin/psql/variable.c)

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-13 Thread Fabien COELHO
Hello, [...] I didn't make rollbacks to savepoints after the failure because they cannot help for serialization failures at all: after rollback to savepoint a new attempt will be always unsuccessful. Not necessarily? It depends on where the locks triggering the issue are set, if they are

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-13 Thread Fabien COELHO
Hello Alik, A few comments about the patch v2. Patch applies and compiles. Documentation says that the closer theta is from 0 the flatter the distribution but the implementation requires at least 1, including strange error messages: zipfian parameter must be greater than 1.00 (not

Re: [HACKERS] Row Level Security Documentation

2017-07-13 Thread Fabien COELHO
Hello Rod, This version of the table attempts to stipulate which section of the process the rule applies to. A few comments about this patch. It applies cleanly, make html is ok. It adds a summary table which shows for each case what happens. Although the information can be guessed/infered

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-12 Thread Fabien COELHO
LastBeginState -> RetryState? I'm not sure why this state is a pointer in CState. Putting the struct would avoid malloc/free cycles. Index "-1" may be used to tell it is not set if necessary. Another detail I forgot about this point: there may be a memory leak on variables copies, ISTM that

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-12 Thread Fabien COELHO
Hello Marina, There's the second version of my patch for pgbench. Now transactions with serialization and deadlock failures are rolled back and retried until they end successfully or their number of attempts reaches maximum. In details: - You can set the maximum number of attempts by the

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-10 Thread Fabien COELHO
Hello Alik, Your description is not very precise. What version of Postgres is used? If there is a decline, compared to which version? Is there a link to these results? Benchmark have been done in master v10. I am attaching image with results: . Ok, thanks. More precision would be

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

2017-07-07 Thread Fabien COELHO
Hello Alik, PostgreSQL shows very bad results in YCSB Workload A (50% SELECT and 50% UPDATE of random row by PK) on benchmarking with big number of clients using Zipfian distribution. MySQL also has decline but it is not significant as it is in PostgreSQL. MongoDB does not have decline at

Re: [HACKERS] CommitFest 2017-09 - How do I know what commit to apply patches to

2017-07-04 Thread Fabien COELHO
Hello Ryan, I was diving into CommitFest 2017-09 to help review some patches, Thanks! but I was not sure which version / git commit / git tag of the PostgreSQL repo I should be checked out to in order to correctly apply / test these patches. Is

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-03 Thread Fabien COELHO
The number of retries and maybe failures should be counted, maybe with some adjustable maximum, as suggested. If we fix the maximum number of attempts the maximum number of failures for one script execution will be bounded above (number_of_transactions_in_script *

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-03 Thread Fabien COELHO
Hello Marina, I agree that improving the error handling ability of pgbench is a good thing, although I'm not sure about the implications... Could you tell a little bit more exactly.. What implications are you worried about? The current error handling is either "close connection" or maybe

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

2017-07-01 Thread Fabien COELHO
Hello Marina, A few comments about the submitted patches. I agree that improving the error handling ability of pgbench is a good thing, although I'm not sure about the implications... About the "retry" discussion: I agree that retry is the relevant option from an application point of view.

Re: [HACKERS] psql - add special variable to reflect the last query status

2017-06-28 Thread Fabien COELHO
Hello Pavel, + if (success) + { + char *ntuples = PQcmdTuples(results); + SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0"); + SetVariable(pset.vars, "ERROR", "FALSE"); + } + else + { + SetVariable(pset.vars, "ROW_COUNT", "0"); + SetVariable(pset.vars, "ERROR", "TRUE"); + } +}

Re: [HACKERS] psql - add special variable to reflect the last query status

2017-06-28 Thread Fabien COELHO
Hello Pavel, I agree that the existing "SetVariableBool" function is a misnommer, it should be "SetVariableOn" given what it does, and it is not what we need. switching default setting from ON to TRUE requires wider discussion - Yep. in this moment I like to have special function

Re: [HACKERS] psql - add special variable to reflect the last query status

2017-06-27 Thread Fabien COELHO
Hello Pavel, We can introduce macro SetVariableBool(vars, varname, bool) instead SetVariable(pset.vars, "ERROR", "FALSE"); I checked source code, and it requires little bit more harder refactoring because now we have SetVariableBool - what is unhappy name, because it initialize variable to

Re: [HACKERS] pgbench tap tests & minor fixes

2017-06-25 Thread Fabien COELHO
may be this it because of "end_offset + 1" in expr_scanner_chomp_substring ? Why there is + 1 there? Thanks for the debug! Here is a v9 which does a rebase after the reindentation and fixes the bad offset. -- Fabien.diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l index

Re: [HACKERS] psql - add special variable to reflect the last query status

2017-06-16 Thread Fabien COELHO
I have not any other comments. The implementation is trivial. I rerun all tests and tests passed. I'll mark this patch as ready for commiters. Oops, I just noticed a stupid confusion on my part which got through, I was setting "ERROR" as "success", inverting the expected boolean value.

Re: [HACKERS] proposal psql \gdesc

2017-06-16 Thread Fabien COELHO
Hello Pavel, new update - rebase, changed message I did yet another rebase of your patch after Tom alphabetically ordered backslash commands. Here is the result. -- Fabien.diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index e6eba21..833460e 100644 ---

Re: [HACKERS] pgbench tap tests & minor fixes

2017-06-15 Thread Fabien COELHO
As for me, I would do expr_scanner_chomp_substring(PsqlScanState, int, int&); that changes end_offset as desired... Why not. And use it instead of end_offset = expr_scanner_offset(sstate) - 1; I removed these? The second issue: you are removing all trailing \n and \r. I think you should

Re: [HACKERS] Disallowing multiple queries per PQexec()

2017-06-14 Thread Fabien COELHO
Hello Surafel, My 0.02€: I attach a patch that incorporate the comments and uses similar routines with the rest of the file rather than using command tag I'm not fully convinced by this feature: using multiple queries is a useful trick to reduce network-related latency by combining several

Re: [HACKERS] pgbench tap tests & minor fixes

2017-06-08 Thread Fabien COELHO
Hello Nikolay, I did some investigation: The code there really suppose that there is always \n at the end of the line, and truncates the line. It is done in /* Get location of the ending newline */ end_offset = expr_scanner_offset(sstate) - 1; just two lines above the code we are discussing.

Re: [HACKERS] proposal psql \gdesc

2017-06-05 Thread Fabien COELHO
new update - rebase, changed message Thanks. New patch applies cleanly, make check still ok. -- 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] proposal psql \gdesc

2017-06-04 Thread Fabien COELHO
Hello Brent, Regarding the error message earlier 'No columns or command has no result', it might be clearer with the slightly longer 'The result has no columns or the command has no result'. I agree that a better phrasing may be possible. I'm hesitating about this one because word

Re: [HACKERS] proposal psql \gdesc

2017-06-04 Thread Fabien COELHO
ok - look on new version, please The patch needs a rebase after Tom's reindentation of tab-complete. -- 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 tap tests & minor fixes

2017-05-30 Thread Fabien COELHO
Hello Nikolay, Year, this is much more clear for me. Now I understand this statistics code. Great. I still have three more questions. A new one: my_command->line = expr_scanner_get_substring(sstate, start_offset, -

Re: [HACKERS] pgbench more operators & functions

2017-05-30 Thread Fabien COELHO
The patch looks ok, Ok. but the main issue is missing regress tests. Yes, I agree. I know so it is another patch. But it should be placed differently 1. first patch - initial infrastructure for pgbench regress tests 2. this patch + related regress tests Yep. I have no control about

Re: [HACKERS] pgbench more operators & functions

2017-05-29 Thread Fabien COELHO
[doc about CASE...] I've improved it in attached v11: - add a link to the CASE full documentation - add an example expression with CASE ... Do you think I should improve the doc further? -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to

Re: [HACKERS] pgbench more operators & functions

2017-05-24 Thread Fabien COELHO
Hello Pavel, I am watching this patch - it looks so there are not problems. Great. I found only issue in documentation - new CASE expression is not documented. Hmmm. Actually there was a rather very discreet one in v10: + SQL CASE generic conditional expressions I've improved it

Re: [HACKERS] psql - add special variable to reflect the last query status

2017-05-22 Thread Fabien COELHO
Hello Pavel, I have not any other comments. The implementation is trivial. [...] Indeed. I'll mark this patch as ready for commiters. Thanks for the review. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] psql - add special variable to reflect the last query status

2017-05-22 Thread Fabien COELHO
Please find attached a v2 which hopefully takes into account all your points above. Open question: should it gather more PQerrorResultField, or the two selected one are enough? If more, which should be included? I don't think so it is necessary. No in this moment. ERROR_CODE and

Re: [HACKERS] psql - add special variable to reflect the last query status

2017-05-22 Thread Fabien COELHO
Hello Pavel, After some discussions about what could be useful since psql scripts now accepts tests, this patch sets a few variables which can be used by psql after a "front door" (i.e. actually typed by the user) query: - RESULT_STATUS: the status of the query - ERROR: whether the query

Re: [HACKERS] Variable substitution in psql backtick expansion

2017-05-21 Thread Fabien COELHO
Thanks for the pointer. I grepped for them without success. Here is a v4. I am sending a review of this patch. This patch has trivial implementation - and there are not any objection to used new variable names. 1. there was not any problem with patching, compiling 2. all regress tests

Re: [HACKERS] proposal psql \gdesc

2017-05-21 Thread Fabien COELHO
Hello Pavel, v6 patch applies cleanly; make check ok; code, comments, doc & tests ok; various interactive tests I did ok. Thanks for this useful little feature! Let's see what committers think about it. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To

Re: [HACKERS] proposal psql \gdesc

2017-05-20 Thread Fabien COELHO
Hello Pavel, - Maybe tests could also exercise unnamed columns, eg: SELECT 1, 2, 3 \gdesc \g done Can't see it. No big deal, but if you put it it did not get through, and there is a warning with git apply on the very last line of the patch which may be linked to that:

Re: [HACKERS] proposal psql \gdesc

2017-05-20 Thread Fabien COELHO
Hello Pavel, I am sending a variant with message instead empty result. Thanks. Patch looks good, applies, make check ok, code is neat. Personally I prefer empty result instead message. Hmm. I think that this version is less likely to raise questions from users, especially compared to

Re: [HACKERS] proposal psql \gdesc

2017-05-09 Thread Fabien COELHO
What about detecting the empty result (eg PQntuples()==0?) and writing "Empty result" instead of the strange looking empty table above? That would just mean skipping the PrintQueryResult call in this case? PQntuples == 0 every time - the query is not executed. I meant to test the query

Re: [HACKERS] proposal psql \gdesc

2017-05-09 Thread Fabien COELHO
Hello Pavel, [...] it is little bit worse. I cannot to distinguish between SELECT\gdesc and TRUNCATE xxx\gdesc . All are valid commands and produce empty result, so result of \gdesc command should be empty result too. postgres=# truncate table xx\gdesc ┌──┬──┐ │ Name │ Type │

Re: [HACKERS] proposal psql \gdesc

2017-05-09 Thread Fabien COELHO
Hello Pavel, Patch applies cleanly and compiles. Idem for v2. "make check" ok. Tests look good. I would suggest some rewording, maybe: "Show the description of the result of the current query buffer without actually executing it, by considering it a prepared statement." done Ok. If

Re: [HACKERS] Re: [Pkg-postgresql-public] Debian "postgresql-common" config check issue with pg10

2017-05-08 Thread Fabien COELHO
Hallo Christoph, I've relaxed the regexps there. It's still not exactly what the PG parser accepts, but I think it's a superset now, so we should be safe. Will it be released with the next pg10dev compilation? The answer is "yes", and it works as expected. Vielen Dank again for the fix!

Re: [HACKERS] pgbench tap tests & minor fixes

2017-05-08 Thread Fabien COELHO
Hello, st->cnt -- number of transactions finished successed or failed, right? Or *skipped*. That is why I changed the declaration comment. one iteration of for(;;) is for one transaction or really less. Right? No, under -R -L late schedules are simply skipped. We can't process two

<    1   2   3   4   5   6   7   8   9   10   >