Re: psql \watch versus \timing

On Thu, Aug 28, 2014 at 8:46 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Aug 26, 2014 at 4:55 AM, Heikki Linnakangas
> <hlinnakangas@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
>>>> <hlinnakangas@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.

Attached patch removes start_xact from PSQLexec.

Regards,

--
Fujii Masao