Re: psql \watch versus \timing

On Tue, Aug 26, 2014 at 4:55 AM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)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
>>> <hlinnakangas(at)vmware(dot)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