Re: ProcessUtility_hook

On Wed, Dec 9, 2009 at 9:33 PM, Takahiro Itagaki
<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> Why does this patch #ifdef out the _PG_fini code in pg_stat_statements?
>
> That's because _PG_fini is never called in current releases.
> We could remove it completely, but I'd like to leave it for future
> releases where _PG_fini callback is re-enabled.
> Or, removing #ifdef (enabling _PG_fini function) is also harmless.

I guess my vote is for leaving it alone, but I might not know what I'm
talking about. :-)

>> Where you check for INSERT, UPDATE, and DELETE return codes in
>> pgss_ProcessUtility, I think this deserves a comment explaining that
>> these could occur as a result of EXECUTE.  It wasn't obvious to me,
>> anyway.
>
> Like this?
> /*
>  * Parse command tag to retrieve the number of affected rows.
>  * COPY command returns COPY tag. EXECUTE command might return INSERT,
>  * UPDATE, or DELETE tags, but we cannot retrieve the number of rows
>  * for SELECT. We assume other commands always return 0 row.
>  */

I'm confused by the "we cannot retrieve the number of rows for SELECT"
part. Can you clarify that?

>> It seems to me that the current hook placement does not address this complaint
>> >> I don't see why the hook function should have the ability to
>> >> editorialize on the behavior of everything about ProcessUtility
>> >> *except* the read-only-xact check.
>
> I moved the initialization code of completionTag as the comment,
> but check_xact_readonly() should be called before the hook.
> Am I missing something?

Beats me. I interpreted Tom's remark to be saying the hook call
should come first, but I'm not sure which way is actually better.

...Robert