On Tue, Aug 26, 2014 at 4:55 AM, Heikki Linnakangas
<hlinnakan...@vmware.com> wrote:
> On 08/25/2014 10:48 PM, Heikki Linnakangas wrote:
>>
>> On 08/25/2014 09:22 PM, Fujii Masao wrote:
>>>
>>> On Tue, Aug 26, 2014 at 1:34 AM, Heikki Linnakangas
>>> <hlinnakan...@vmware.com> wrote:
>>>>
>>>> I agree that refactoring this would be nice in the long-term, and I also
>>>> agree that it's probably OK as it is in the short-term. I don't like the
>>>> name PSQLexecInternal, though. PSQLexec is used for "internal" commands
>>>> anyway. In fact it's backwards, because PSQLexecInternal is used for
>>>> non-internal queries, given by \watch, while PSQLexec is used for
>>>> internal
>>>> commands.
>>>
>>>
>>> Agreed. So what about PSQLexecCommon (inspired by
>>> the relation between LWLockAcquireCommon and LWLockAcquire)?
>>> Or any better name?
>>
>>
>> Actually, perhaps it would be better to just copy-paste PSQLexec, and
>> modify the copy to suite \watch's needs. (PSQLexecWatch?
>> SendWatchQuery?). PSQLexec doesn't do much, and there isn't very much
>> overlap between what \watch wants and what other PSQLexec callers want.
>> \watch wants timing output, others don't. \watch doesn't want
>> transaction handling.
Agreed. Attached is the revised version of the patch. I implemented
PSQLexecWatch() which sends the query, prints the results and outputs
the query execution time (if \timing is enabled).
This patch was marked as ready for committer, but since I revised
the code very much, I marked this as needs review again.
>> Do we want --echo-hidden to print the \watch'd
>> query? Not sure..
Per document, --echo-hidden prints the actual queries generated by
backslash command. But \watch doesn't handle backslash commands.
So I think that PSQLexecWatch doesn't need to handle --echo-hidden.
> BTW, I just noticed that none of the callers of PSQLexec pass
> "start_xact=true". So that part of the function is dead code. We might want
> to remove it, and replace with a comment noting that PSQLexec never starts a
> new transaction block, even in autocommit-off mode. (I know you're hacking
> on this, so I didnn't want to joggle your elbow by doing it right now)
Good catch. So I will remove start_xact code later.
Regards,
--
Fujii Masao
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***************
*** 2687,2693 **** do_watch(PQExpBuffer query_buf, long sleep)
for (;;)
{
! PGresult *res;
time_t timer;
long i;
--- 2687,2693 ----
for (;;)
{
! int res;
time_t timer;
long i;
***************
*** 2701,2759 **** do_watch(PQExpBuffer query_buf, long sleep)
myopt.title = title;
/*
! * Run the query. We use PSQLexec, which is kind of cheating, but
! * SendQuery doesn't let us suppress autocommit behavior.
*/
! res = PSQLexec(query_buf->data, false);
!
! /* PSQLexec handles failure results and returns NULL */
! if (res == NULL)
! break;
/*
! * If SIGINT is sent while the query is processing, PSQLexec will
! * consume the interrupt. The user's intention, though, is to cancel
! * the entire watch process, so detect a sent cancellation request and
! * exit in this case.
*/
! if (cancel_pressed)
! {
! PQclear(res);
break;
! }
!
! switch (PQresultStatus(res))
! {
! case PGRES_TUPLES_OK:
! printQuery(res, &myopt, pset.queryFout, pset.logfile);
! break;
!
! case PGRES_COMMAND_OK:
! fprintf(pset.queryFout, "%s\n%s\n\n", title, PQcmdStatus(res));
! break;
!
! case PGRES_EMPTY_QUERY:
! psql_error(_("\\watch cannot be used with an empty query\n"));
! PQclear(res);
! return false;
!
! case PGRES_COPY_OUT:
! case PGRES_COPY_IN:
! case PGRES_COPY_BOTH:
! psql_error(_("\\watch cannot be used with COPY\n"));
! PQclear(res);
! return false;
!
! default:
! /* other cases should have been handled by PSQLexec */
! psql_error(_("unexpected result status for \\watch\n"));
! PQclear(res);
! return false;
! }
!
! PQclear(res);
!
! fflush(pset.queryFout);
/*
* Set up cancellation of 'watch' via SIGINT. We redo this each time
--- 2701,2720 ----
myopt.title = title;
/*
! * Run the query and print out the results. We use PSQLexecWatch,
! * which is kind of cheating, but SendQuery doesn't let us suppress
! * autocommit behavior.
*/
! res = PSQLexecWatch(query_buf->data, &myopt);
/*
! * PSQLexecWatch handles the case where we can no longer
! * repeat the query, and returns 0 or -1.
*/
! if (res == 0)
break;
! if (res == -1)
! return false;
/*
* Set up cancellation of 'watch' via SIGINT. We redo this each time
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
***************
*** 497,502 **** PSQLexec(const char *query, bool start_xact)
--- 497,598 ----
}
+ /*
+ * PSQLexecWatch
+ *
+ * This function is used for \watch command to send the query to
+ * the server and print out the results.
+ *
+ * Returns 1 if the query executed successfully, 0 if it cannot be repeated,
+ * e.g., because of the interrupt, -1 on error.
+ */
+ int
+ PSQLexecWatch(const char *query, const printQueryOpt *opt)
+ {
+ PGresult *res;
+ double elapsed_msec = 0;
+ instr_time before;
+ instr_time after;
+
+ if (!pset.db)
+ {
+ psql_error("You are currently not connected to a database.\n");
+ return 0;
+ }
+
+ SetCancelConn();
+
+ if (pset.timing)
+ INSTR_TIME_SET_CURRENT(before);
+
+ res = PQexec(pset.db, query);
+
+ ResetCancelConn();
+
+ if (!AcceptResult(res))
+ {
+ PQclear(res);
+ return 0;
+ }
+
+ if (pset.timing)
+ {
+ INSTR_TIME_SET_CURRENT(after);
+ INSTR_TIME_SUBTRACT(after, before);
+ elapsed_msec = INSTR_TIME_GET_MILLISEC(after);
+ }
+
+ /*
+ * If SIGINT is sent while the query is processing, the interrupt
+ * will be consumed. The user's intention, though, is to cancel
+ * the entire watch process, so detect a sent cancellation request and
+ * exit in this case.
+ */
+ if (cancel_pressed)
+ {
+ PQclear(res);
+ return 0;
+ }
+
+ switch (PQresultStatus(res))
+ {
+ case PGRES_TUPLES_OK:
+ printQuery(res, opt, pset.queryFout, pset.logfile);
+ break;
+
+ case PGRES_COMMAND_OK:
+ fprintf(pset.queryFout, "%s\n%s\n\n", opt->title, PQcmdStatus(res));
+ break;
+
+ case PGRES_EMPTY_QUERY:
+ psql_error(_("\\watch cannot be used with an empty query\n"));
+ PQclear(res);
+ return -1;
+
+ case PGRES_COPY_OUT:
+ case PGRES_COPY_IN:
+ case PGRES_COPY_BOTH:
+ psql_error(_("\\watch cannot be used with COPY\n"));
+ PQclear(res);
+ return -1;
+
+ default:
+ psql_error(_("unexpected result status for \\watch\n"));
+ PQclear(res);
+ return -1;
+ }
+
+ PQclear(res);
+
+ fflush(pset.queryFout);
+
+ /* Possible microtiming output */
+ if (pset.timing)
+ printf(_("Time: %.3f ms\n"), elapsed_msec);
+
+ return 1;
+ }
+
/*
* PrintNotifications: check for asynchronous notifications, and print them out
*** a/src/bin/psql/common.h
--- b/src/bin/psql/common.h
***************
*** 12,17 ****
--- 12,19 ----
#include <setjmp.h>
#include "libpq-fe.h"
+ #include "print.h"
+
#define atooid(x) ((Oid) strtoul((x), NULL, 10))
extern bool setQFout(const char *fname);
***************
*** 37,42 **** extern void SetCancelConn(void);
--- 39,45 ----
extern void ResetCancelConn(void);
extern PGresult *PSQLexec(const char *query, bool start_xact);
+ extern int PSQLexecWatch(const char *query, const printQueryOpt *opt);
extern bool SendQuery(const char *query);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
