On Wed, Jan 11, 2017 at 7:47 PM, Dilip Kumar <dilipbal...@gmail.com> wrote:
> +void
> +pgstat_count_sqlstmt(const char *commandTag)
> +{
> + PgStat_SqlstmtEntry *htabent;
> + bool found;
> +
> + if (!pgstat_track_sql)
> + return
>
> Callers of pgstat_count_sqlstmt are always ensuring that
> pgstat_track_sql is true, seems it's redundant here.I have done testing of the feature, it's mostly working as per the expectation. I have few comments/questions. -------- If you execute the query with EXECUTE then commandTag will be EXECUTE that way we will not show the actual query type, I mean all the statements will get the common tag "EXECUTE". You can try this. PREPARE fooplan (int) AS INSERT INTO t VALUES($1); EXECUTE fooplan(1); ------ + /* Destroy the SQL statement counter stats HashTable */ + hash_destroy(pgstat_sql); + + /* Create SQL statement counter Stats HashTable */ I think in the patch we have used mixed of "Stats HashTable" and "stats HashTable", I think better to be consistent throughout the patch. Check other similar instances. ---------------- @@ -1102,6 +1102,12 @@ exec_simple_query(const char *query_string) PortalDrop(portal, false); + /* + * Count SQL statement for pg_stat_sql view + */ + if (pgstat_track_sql) + pgstat_count_sqlstmt(commandTag); We are only counting the successful SQL statement, Is that intentional? ------ I have noticed that pgstat_count_sqlstmt is called from exec_simple_query and exec_execute_message. Don't we want to track the statement executed from functions? ------- -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
