Re: [HACKERS] New SQL counter statistics view (pg_stat_sql)
От | vinayak |
---|---|
Тема | Re: [HACKERS] New SQL counter statistics view (pg_stat_sql) |
Дата | |
Msg-id | d1f556f8-3920-cfaf-5b37-24d1259b9f9e@lab.ntt.co.jp обсуждение исходный текст |
Ответ на | Re: [HACKERS] New SQL counter statistics view (pg_stat_sql) (Haribabu Kommi <kommi.haribabu@gmail.com>) |
Список | pgsql-hackers |
On 2017/01/18 14:15, Haribabu Kommi wrote:
I have reviewed the patch. All the test cases works fine.On Sat, Jan 14, 2017 at 2:58 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:The view is currently designed to count user/application initiated SQL statements,On Wed, Jan 11, 2017 at 7:47 PM, Dilip Kumar <dilipbalaut@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.Thanks for the review and test.The use case for this view is to find out the number of different SQL statementsthat are getting executed successfully on an instance by the application/user.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);
------Yes, that's correct. Currently the count is incremented based on the base command,because of this reason, the EXECUTE is increased, instead of the actual operation.+ /* 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.
----------------Corrected.
@@ -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?Yes, I thought of counting the successful SQL operations that changed thedatabase over a period of time. I am not sure whether there will be manyfailure operations that can occur on an instance to be counted.------
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?
-------but not the internal SQL queries that are getting executed from functions and etc.>>+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.Removed the check in pgstat_count_sqlstmt statement and add it exec_execute_messagefunction where the check is missed.Updated patch attached.
Regards,
Vinayak Pokale
NTT Open Source Software Center
В списке pgsql-hackers по дате отправления: