On Wed, Dec 9, 2009 at 9:33 PM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:
> Robert Haas <robertmhaas@gmail.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