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

One thing we could think about if this seems too high is to drop ROW_COUNT. I'm unconvinced that it has a real use-case, and it seems to be taking more than its share of the work in non-error cases, because it turns out that PQcmdTuples() is not an amazingly cheap function. I do think that a

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

Hello Tom, I put back SetResultVariables function which is called twice, for SQL queries and the new descriptions. It worked out of the box with DECLARE which is just another SQL statement, so maybe I did not understood the cursor issue you were signaling... No, I was concerned about

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

Fabien COELHO writes: > See v9 attached. I've pushed this with some editorialization. > I put back SetResultVariables function which is called twice, for SQL > queries and the new descriptions. It worked out of the box with DECLARE > which is just another SQL statement,

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

Well, if we provided a different SQLSTATE for each qualitatively different type of libpq error, that might well be useful enough to justify some risk of application breakage. But replacing a constant string that we've had for ~15 years with a different constraint string isn't doing anything

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

Robert Haas writes: > Well, if we provided a different SQLSTATE for each qualitatively > different type of libpq error, that might well be useful enough to > justify some risk of application breakage. But replacing a constant > string that we've had for ~15 years with a

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

On Tue, Sep 12, 2017 at 3:12 PM, Tom Lane wrote: >> I think this is a bad plan. Right now, libpq sets no SQLSTATE for >> internally generated errors; it is almost certain that there are >> applications testing for an empty SQLSTATE to notice when they're >> getting an error

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

Robert Haas writes: > On Tue, Sep 12, 2017 at 1:23 PM, Fabien COELHO wrote: >> I added two error codes, which is debatable. One is used hardcoded by libpq >> if no diagnostic is found, and the other by psql if libpq returned something >> empty, which

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

2017-09-12 20:43 GMT+02:00 Robert Haas : > On Tue, Sep 12, 2017 at 1:23 PM, Fabien COELHO > wrote: > > I added two error codes, which is debatable. One is used hardcoded by > libpq > > if no diagnostic is found, and the other by psql if libpq returned

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

On Tue, Sep 12, 2017 at 1:23 PM, Fabien COELHO wrote: > I added two error codes, which is debatable. One is used hardcoded by libpq > if no diagnostic is found, and the other by psql if libpq returned something > empty, which might happen if psql is linked with an older

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

Hello Tom, Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that situation where libpq did not report an error? Meh. If we're going to do that I think it might be better to hack libpq itself to do so, ie, force PQresultErrorField(..., PG_DIAG_SQLSTATE) to always return

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

2017-09-11 20:46 GMT+02:00 Fabien COELHO : > > I think you're overly optimistic to believe that every failure will have a SQLSTATE; I don't think that's true for libpq-reported errors, such as connection loss. >>> >> Yep, I thought I was optimistic:-) Can I add

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

I think you're overly optimistic to believe that every failure will have a SQLSTATE; I don't think that's true for libpq-reported errors, such as connection loss. Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that situation where libpq did not report an error? Meh.

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

Fabien COELHO writes: >> I think you're overly optimistic to believe that every failure will >> have a SQLSTATE; I don't think that's true for libpq-reported errors, >> such as connection loss. > Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that >

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

Hello Tom, Hm. Looking closer at this, I see that it doesn't work so well after all to put the variable-setting code in ProcessResult: that fails to cover the ExecQueryUsingCursor code path. Ok, I'll investigate this path. And it also fails to cover DescribeQuery, which arguably should set

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

Fabien COELHO writes: > Small v7 update, sorry for the noise. Hm. Looking closer at this, I see that it doesn't work so well after all to put the variable-setting code in ProcessResult: that fails to cover the ExecQueryUsingCursor code path. And it also fails to cover

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

Hello Pavel, I checked performance - the most fast queries are execution of simple prepared statement prepare x as select 1; -- 100 x execute x; execute x; execute x; execute x; ## patched [pavel@nemesis postgresql]$ time psql -At -1 postgres -f ~/xxx.sql > /dev/null real 0m44,887s user

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

Hi 2017-09-06 11:14 GMT+02:00 Fabien COELHO : > > Here is a version 6. >> > > Small v7 update, sorry for the noise. > > Add testing the initial state of all variables. > > Fix typos in a comment in tests. > > Fix the documentation wrt the current implementation behavior. I

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

Here is a version 6. Small v7 update, sorry for the noise. Add testing the initial state of all variables. Fix typos in a comment in tests. Fix the documentation wrt the current implementation behavior. -- Fabien.diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml

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

Hello Tom, Here is a version 6. A few thoughts about this patch: * I think the ERROR_CODE variable should instead be named SQLSTATE. That is what the SQL standard calls this string, and it's also what just about all our documentation calls it; see PG_DIAG_SQLSTATE in libpq, or the SQLSTATE

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

* It might be better if SQLSTATE and ERROR_MESSAGE were left unchanged by a non-error query. Hmmm. I'm not sure. If so, ERROR_MESSAGE should be LAST_ERROR_MESSAGE maybe? Then what about LAST_ERROR_SQLSTATE to go with it, and let SQLSTATE & ERROR_MESSAGE reflect the last command, and ERROR is

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

Fabien COELHO writes: >> * It might be better if SQLSTATE and ERROR_MESSAGE were left >> unchanged by a non-error query. > Hmmm. I'm not sure. If so, ERROR_MESSAGE should be LAST_ERROR_MESSAGE > maybe? Then what about LAST_ERROR_SQLSTATE to go with it, and let SQLSTATE > &

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

Hello Tom, * I think the ERROR_CODE variable should instead be named SQLSTATE. That is what the SQL standard calls this string, and it's also what just about all our documentation calls it; see PG_DIAG_SQLSTATE in libpq, or the SQLSTATE 'x' construct in pl/pgsql, or the sqlstate attribute

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

A few thoughts about this patch: * I think the ERROR_CODE variable should instead be named SQLSTATE. That is what the SQL standard calls this string, and it's also what just about all our documentation calls it; see PG_DIAG_SQLSTATE in libpq, or the SQLSTATE 'x' construct in pl/pgsql, or the

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

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

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

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 9:25 GMT+02:00 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

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

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 17:30 GMT+02:00 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 >>

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

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] psql - add special variable to reflect the last query status

Hi 2017-06-19 5:55 GMT+02:00 Pavel Stehule : > > > 2017-06-17 7:58 GMT+02:00 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

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

2017-06-17 7:58 GMT+02:00 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

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

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] psql - add special variable to reflect the last query status

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 21:33 GMT+02:00 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?

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

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 9:40 GMT+02:00 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)

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

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] psql - add special variable to reflect the last query status

Hi 2017-04-04 23:01 GMT+02:00 Pavel Stehule : > > > 2017-04-04 22:05 GMT+02:00 Fabien COELHO : > >> >> 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

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

2017-04-04 22:05 GMT+02:00 Fabien COELHO : > > 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: > > -

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

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