Overall I like the idea; adds some nice visibility to something that has been very ephemeral in the past.
Not included in this patch and maybe for follow-up work, is this > information a good idea also? can be added to EXPLAIN output and perhaps pg_stat_database. > I could see EXPLAIN being somewhat useful (especially for non-interactive things like auto_explain), so a weak +1 on that. Definitely not useful for pg_stat_database IMHO. Some quick comments on the patch: FILE: contrib/pg_stat_statements/expected/plan_cache.out These tests seem very verbose. Why not just prepare a simple query: prepare foo as select $1 > 0; execute foo(1); ...then tweak things via plan_cache_mode to ensure we test the right things? Also would be nice to constrain the pg_stat_statement SELECTs with some careful WHERE clauses. Would also allow you to remove the ORDER BY and the COLLATE. FILE: contrib/pg_stat_statements/meson.build oldextversions should still have a trailing comma FILE: contrib/pg_stat_statements/pg_stat_statements.c + if (cplan && cplan->status > PLAN_CACHE_STATUS_CUSTOM_PLAN) Not really comfortable with using the enum like this. Better would be explicitly listing the two states that lead to the increment. Not as good but still better: cplan->status >= PLAN_CACHE_STATUS_GENERIC_PLAN_BUILD + PlanCacheStatus status; /* is it a reused generic plan? */ The comment should be updated FILE: contrib/pg_stat_statements/sql/plan_cache.sql Missing a newline at the end FILE: doc/src/sgml/pgstatstatements.sgml + Total number of non-utility statements executed using a generic plan I'm not sure we need to specify non-utility here. Cheers, Greg -- Crunchy Data - https://www.crunchydata.com Enterprise Postgres Software Products & Tech Support
