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

From Pavel Stehule
Subject Re: [HACKERS] psql - add special variable to reflect the last query status
Date
Msg-id CAFj8pRCqDtSitJhBuxD20sqmXkLQ6eLOf6keUMT8--ABpFHGxw@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] psql - add special variable to reflect the last querystatus  (Fabien COELHO <coelho@cri.ensmp.fr>)
Responses Re: [HACKERS] psql - add special variable to reflect the last querystatus  (Fabien COELHO <coelho@cri.ensmp.fr>)
List pgsql-hackers


2017-05-22 9:40 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

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 failed
 - ERROR_MESSAGE: ...
 - ROW_COUNT: #rows affected

 SELECT * FROM ;
 \if :ERROR
   \echo oops
   \q
 \endif

I'm not sure that the names are right. Maybe STATUS would be better than
RESULT_STATUS.

I am sending review of this patch:

1. I agree so STATUS is better name, than RESULT status.

Ok, looks simpler.

Currently it returns values with prefix PGRES (like PGRES_FATAL_ERROR, PGRES_TUPLES_OK). Maybe we should to cut this prefix. FATAL_ERROR, TUPLES_OK looks better for custom level. The PGRES prefix has not sense in psql.

Indeed. I skipped "PGRES_".

2. I propose availability to read ERROR_CODE - sometimes it can be more
practical than parsing error possible translated message

Ok.

3. The fields ERROR_MESSAGE and ROW_COUNT are set only when it has sense.
This behave is maybe too strict for psql and the processing needs more
nesting \if command. What do you think about -1 or 0 for ROW_COUNT (for
DDL) and "" for ERROR_MESSAGE when there are not any error?  It will be
consistent with already implemented LASTOID variable (and other state psql
variables). Using default values are not strict clean, but it can reduce
complexity of psql scripts.

My intention was that it could be tested with the "is defined" syntax, which is yet to be agreed upon and implemented, so maybe generating empty string is a better option.

For ROW_COUNT, I think that it should be consistent with what PL/pgSQL does, so it think that 0 should be the default.

4. all regress tests passed
5. there are not any problem with doc building

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 ERROR_MESSAGE are fundamental - and if we add other, then we should to add all. Has not sense to add only some.

Regards

Pavel



--
Fabien.

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS] plpgsql, caching, resowners and jit
Next
From: Nikita Glukhov
Date:
Subject: Re: [HACKERS] PATCH: recursive json_populate_record()