Tom Lane <t...@sss.pgh.pa.us> wrote: > ... and now that I have, I find at least four highly questionable > things about it: > > 1. The placement of the hook. Why is it three lines down in > ProcessUtility? It's probably reasonable to have the Assert first, > but 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 of completionTag into standard_ProcessUtility. > 2. The naming and documentation of the added GUC setting for > pg_stat_statements. "track_ddl" seems pretty bizarre to me because > there are many utility statements that no one would call DDL. COPY, > for example, is certainly not DDL. Why not call it "track_utility"? Ok, fixed. > 3. The enable-condition test in pgss_ProcessUtility. Is it really > appropriate to be gating this by isTopLevel? I should think that > the nested_level check in pgss_enabled would be sufficient and > more likely to do what's expected. I removed the isTopLevel check. I was worried about auto-generated utility commands; generated sub commands are called with the same query string as the top query. Don't it confuse statistics? > 4. The special case for CopyStmt. That's just weird, and it adds > a maintenance requirement we don't need. I don't see a really good > argument why COPY (alone among utility statements) deserves to have > a rowcount tracked by pg_stat_statements, but even if you want that > it'd be better to rely on examining the completionTag after the fact. > The fact that the tag is "COPY nnnn" is part of the user-visible API > for COPY and won't change lightly. The division of labor between > ProcessUtility and copy.c is far more volatile, but this patch has > injected itself into that. Ok, fixed. I've thought string-based interface is not desirable, but it should be a stable API. COPY and INSERT/UPDATE/DELETE (used by EXECUTE) are counted by pg_stat_statements, but EXECUTE SELECT is impossible. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
ProcessUtility_hook_20091203.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
