diff --git a/edb/buildmeta.py b/edb/buildmeta.py index d381e336b4b..7763df23efb 100644 --- a/edb/buildmeta.py +++ b/edb/buildmeta.py @@ -60,7 +60,7 @@ # The merge conflict there is a nice reminder that you probably need # to write a patch in edb/pgsql/patches.py, and then you should preserve # the old value. -EDGEDB_CATALOG_VERSION = 2024_11_15_00_00 +EDGEDB_CATALOG_VERSION = 2024_11_22_00_00 EDGEDB_MAJOR_VERSION = 6 diff --git a/edb/lib/cfg.edgeql b/edb/lib/cfg.edgeql index 56df07eacbb..6a586e666d6 100644 --- a/edb/lib/cfg.edgeql +++ b/edb/lib/cfg.edgeql @@ -45,6 +45,7 @@ CREATE SCALAR TYPE cfg::ConnectionTransport EXTENDING enum< TCP, TCP_PG, HTTP, SIMPLE_HTTP, HTTP_METRICS, HTTP_HEALTH>; CREATE SCALAR TYPE cfg::QueryCacheMode EXTENDING enum< InMemory, RegInline, PgFunc, Default>; +CREATE SCALAR TYPE cfg::QueryStatsOption EXTENDING enum; CREATE ABSTRACT TYPE cfg::ConfigObject EXTENDING std::BaseObject; @@ -387,6 +388,12 @@ ALTER TYPE cfg::AbstractConfig { CREATE CONSTRAINT std::min_value(1); SET default := 100; }; + + CREATE PROPERTY track_query_stats -> cfg::QueryStatsOption { + CREATE ANNOTATION cfg::backend_setting := '"edb_stat_statements.track"'; + CREATE ANNOTATION std::description := + 'Select what queries are tracked in sys::QueryStats'; + }; }; diff --git a/edb_stat_statements/edb_stat_statements.c b/edb_stat_statements/edb_stat_statements.c index 6b8b322d33d..80aedca8430 100644 --- a/edb_stat_statements/edb_stat_statements.c +++ b/edb_stat_statements/edb_stat_statements.c @@ -336,31 +336,35 @@ static HTAB *pgss_hash = NULL; typedef enum { PGSS_TRACK_NONE, /* track no statements */ - PGSS_TRACK_TOP, /* only top level statements */ - PGSS_TRACK_ALL, /* all statements, including nested ones */ + PGSS_TRACK_ALL, /* all recognized top-level statements */ + PGSS_TRACK_DEV, /* all top-level statements, including unrecognized ones */ + PGSS_TRACK_NESTED, /* all statements, including unrecognized and nested ones */ } PGSSTrackLevel; static const struct config_enum_entry track_options[] = { - {"none", PGSS_TRACK_NONE, false}, - {"top", PGSS_TRACK_TOP, false}, - {"all", PGSS_TRACK_ALL, false}, + {"None", PGSS_TRACK_NONE, false}, + {"All", PGSS_TRACK_ALL, false}, + {"Dev", PGSS_TRACK_DEV, false}, + {"Nested", PGSS_TRACK_NESTED, false}, {NULL, 0, false} }; static int pgss_max = 5000; /* max # statements to track */ -static int pgss_track = PGSS_TRACK_TOP; /* tracking level */ +static int pgss_track = PGSS_TRACK_ALL; /* tracking level */ static bool pgss_track_utility = true; /* whether to track utility commands */ static bool pgss_track_planning = false; /* whether to track planning * duration */ static bool pgss_save = true; /* whether to save stats across shutdown */ -static bool edbss_track_unrecognized = false; /* whether to track unrecognized statements as-is */ #define pgss_enabled(level) \ (!IsParallelWorker() && \ - (pgss_track == PGSS_TRACK_ALL || \ - (pgss_track == PGSS_TRACK_TOP && (level) == 0))) + (pgss_track == PGSS_TRACK_NESTED || \ + (pgss_track != PGSS_TRACK_NONE && (level) == 0))) + +#define edbss_track_unrecognized() \ + (pgss_track == PGSS_TRACK_DEV || pgss_track == PGSS_TRACK_NESTED) #define record_gc_qtexts() \ do { \ @@ -492,7 +496,7 @@ _PG_init(void) "Selects which statements are tracked by edb_stat_statements.", NULL, &pgss_track, - PGSS_TRACK_TOP, + PGSS_TRACK_ALL, track_options, PGC_SUSET, 0, @@ -533,17 +537,6 @@ _PG_init(void) NULL, NULL); - DefineCustomBoolVariable("edb_stat_statements.track_unrecognized", - "Selects whether unrecognized SQL statements are tracked as-is.", - NULL, - &edbss_track_unrecognized, - false, - PGC_SIGHUP, - 0, - NULL, - NULL, - NULL); - MarkGUCPrefixReserved("edb_stat_statements"); /* @@ -1188,7 +1181,7 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query, JumbleState *jstate) 0, 0); edbss_free_stmt_info(info); - } else if (!edbss_track_unrecognized) { + } else if (!edbss_track_unrecognized()) { query->queryId = UINT64CONST(0); } else if (jstate && jstate->clocations_count > 0) /* @@ -1715,7 +1708,7 @@ pgss_store(const char *query, uint64 queryId, id = &info->id.uuid; stmt_type = info->stmt_type; extras = info->extras; - } else if (!edbss_track_unrecognized) { + } else if (!edbss_track_unrecognized()) { /* skip unrecognized statements unless we're told not to */ goto done; } else { diff --git a/edb_stat_statements/expected/level_tracking.out.17 b/edb_stat_statements/expected/level_tracking.out.17 index c5de894cb6d..adb99ed80f8 100644 --- a/edb_stat_statements/expected/level_tracking.out.17 +++ b/edb_stat_statements/expected/level_tracking.out.17 @@ -10,7 +10,7 @@ SELECT edb_stat_statements_reset() IS NOT NULL AS t; -- DO block - top-level tracking. CREATE TABLE stats_track_tab (x int); -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; DELETE FROM stats_track_tab; DO $$ BEGIN @@ -36,7 +36,7 @@ SELECT edb_stat_statements_reset() IS NOT NULL AS t; (1 row) -- DO block - all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; DELETE FROM stats_track_tab; DO $$ BEGIN @@ -64,7 +64,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements | | END; $$ f | 1 | SELECT $1::TEXT t | 1 | SELECT edb_stat_statements_reset() IS NOT NULL AS t - t | 1 | SET edb_stat_statements.track = 'all' + t | 1 | SET edb_stat_statements.track = 'nested' (7 rows) -- Procedure with multiple utility statements. @@ -77,7 +77,7 @@ AS $$ $$; SET edb_stat_statements.track_utility = TRUE; -- all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- @@ -96,7 +96,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (4 rows) -- top-level tracking. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- @@ -113,7 +113,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (2 rows) -- DO block - top-level tracking without utility. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SET edb_stat_statements.track_utility = FALSE; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t @@ -140,7 +140,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (2 rows) -- DO block - all-level tracking without utility. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- @@ -168,7 +168,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (4 rows) -- PL/pgSQL function - top-level tracking. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SET edb_stat_statements.track_utility = FALSE; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t @@ -244,7 +244,7 @@ SELECT toplevel, calls, rows, query FROM edb_stat_statements ORDER BY query COLL (5 rows) -- PL/pgSQL function - all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- diff --git a/edb_stat_statements/expected/level_tracking.out.18 b/edb_stat_statements/expected/level_tracking.out.18 index 876f0c1aa1e..7e9aec155a1 100644 --- a/edb_stat_statements/expected/level_tracking.out.18 +++ b/edb_stat_statements/expected/level_tracking.out.18 @@ -10,7 +10,7 @@ SELECT edb_stat_statements_reset() IS NOT NULL AS t; -- DO block - top-level tracking. CREATE TABLE stats_track_tab (x int); -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; DELETE FROM stats_track_tab; DO $$ BEGIN @@ -36,7 +36,7 @@ SELECT edb_stat_statements_reset() IS NOT NULL AS t; (1 row) -- DO block - all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; DELETE FROM stats_track_tab; DO $$ BEGIN @@ -77,7 +77,7 @@ AS $$ $$; SET edb_stat_statements.track_utility = TRUE; -- all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- @@ -96,7 +96,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (4 rows) -- top-level tracking. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- @@ -113,7 +113,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (2 rows) -- DO block - top-level tracking without utility. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SET edb_stat_statements.track_utility = FALSE; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t @@ -140,7 +140,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (2 rows) -- DO block - all-level tracking without utility. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- @@ -168,7 +168,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements (4 rows) -- PL/pgSQL function - top-level tracking. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SET edb_stat_statements.track_utility = FALSE; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t @@ -244,7 +244,7 @@ SELECT toplevel, calls, rows, query FROM edb_stat_statements ORDER BY query COLL (5 rows) -- PL/pgSQL function - all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; t --- diff --git a/edb_stat_statements/sql/level_tracking.sql b/edb_stat_statements/sql/level_tracking.sql index 9cb852e8e20..123693059e0 100644 --- a/edb_stat_statements/sql/level_tracking.sql +++ b/edb_stat_statements/sql/level_tracking.sql @@ -7,7 +7,7 @@ SELECT edb_stat_statements_reset() IS NOT NULL AS t; -- DO block - top-level tracking. CREATE TABLE stats_track_tab (x int); -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; DELETE FROM stats_track_tab; DO $$ BEGIN @@ -19,7 +19,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements SELECT edb_stat_statements_reset() IS NOT NULL AS t; -- DO block - all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; DELETE FROM stats_track_tab; DO $$ BEGIN @@ -43,20 +43,20 @@ AS $$ $$; SET edb_stat_statements.track_utility = TRUE; -- all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; CALL proc_with_utility_stmt(); SELECT toplevel, calls, query FROM edb_stat_statements ORDER BY query COLLATE "C", toplevel; -- top-level tracking. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; CALL proc_with_utility_stmt(); SELECT toplevel, calls, query FROM edb_stat_statements ORDER BY query COLLATE "C", toplevel; -- DO block - top-level tracking without utility. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SET edb_stat_statements.track_utility = FALSE; SELECT edb_stat_statements_reset() IS NOT NULL AS t; DELETE FROM stats_track_tab; @@ -73,7 +73,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements ORDER BY query COLLATE "C", toplevel; -- DO block - all-level tracking without utility. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; DELETE FROM stats_track_tab; DO $$ @@ -89,7 +89,7 @@ SELECT toplevel, calls, query FROM edb_stat_statements ORDER BY query COLLATE "C", toplevel; -- PL/pgSQL function - top-level tracking. -SET edb_stat_statements.track = 'top'; +SET edb_stat_statements.track = 'dev'; SET edb_stat_statements.track_utility = FALSE; SELECT edb_stat_statements_reset() IS NOT NULL AS t; CREATE FUNCTION PLUS_TWO(i INTEGER) RETURNS INTEGER AS $$ @@ -122,7 +122,7 @@ SELECT PLUS_THREE(10); SELECT toplevel, calls, rows, query FROM edb_stat_statements ORDER BY query COLLATE "C"; -- PL/pgSQL function - all-level tracking. -SET edb_stat_statements.track = 'all'; +SET edb_stat_statements.track = 'nested'; SELECT edb_stat_statements_reset() IS NOT NULL AS t; -- we drop and recreate the functions to avoid any caching funnies diff --git a/tests/test_edgeql_sys.py b/tests/test_edgeql_sys.py index 74bcc5e5071..35c9a2b60b9 100644 --- a/tests/test_edgeql_sys.py +++ b/tests/test_edgeql_sys.py @@ -31,6 +31,9 @@ class TestQueryStatsMixin: async def _query_for_stats(self): raise NotImplementedError + async def _configure_track(self, option: str): + raise NotImplementedError + async def _bad_query_for_stats(self): raise NotImplementedError @@ -46,20 +49,33 @@ async def _test_sys_query_stats(self): ) select sum(stats.calls) ''' + + # Take the initial tracking number of executions calls = await self.con.query_single(stats_query, self.stats_type) + # Execute the query one more time await self._query_for_stats() self.assertEqual( await self.con.query_single(stats_query, self.stats_type), calls + 1, ) + # Bad queries are not tracked await self._bad_query_for_stats() self.assertEqual( await self.con.query_single(stats_query, self.stats_type), calls + 1, ) + # Turn off cfg::Config.track_query_stats, verify tracking is stopped + await self._configure_track('None') + await self._query_for_stats() + self.assertEqual( + await self.con.query_single(stats_query, self.stats_type), + calls + 1, + ) + + # sys::reset_query_stats() branch filter works correctly self.assertIsNone( await self.con.query_single( "select sys::reset_query_stats(branch_name := 'non_exdb')" @@ -70,6 +86,7 @@ async def _test_sys_query_stats(self): calls + 1, ) + # sys::reset_query_stats() works correctly self.assertIsNotNone( await self.con.query('select sys::reset_query_stats()') ) @@ -130,6 +147,12 @@ async def _query_for_stats(self): [], ) + async def _configure_track(self, option: str): + await self.con.query(f''' + configure session set track_query_stats := + {common.quote_literal(option)}; + ''') + async def _bad_query_for_stats(self): async with self.assertRaisesRegexTx( edgedb.InvalidReferenceError, 'does not exist' @@ -152,6 +175,13 @@ async def _query_for_stats(self): [[self.stats_magic_word]], ) + async def _configure_track(self, option: str): + # XXX: we should probably translate the config name in the compiler, + # so that we can use the frontend name (track_query_stats) here instead + await self.scon.execute(f''' + set "edb_stat_statements.track" TO {option}; + ''') + async def _bad_query_for_stats(self): import asyncpg diff --git a/tests/test_server_ops.py b/tests/test_server_ops.py index d8676304dde..fbee3f88c2a 100644 --- a/tests/test_server_ops.py +++ b/tests/test_server_ops.py @@ -1718,7 +1718,7 @@ async def test_edb_stat_statements(self): self.assertTrue(await cluster.ensure_initialized()) await cluster.start(server_settings={ 'edb_stat_statements.track_planning': 'false', - 'edb_stat_statements.track_unrecognized': 'true', + 'edb_stat_statements.track': 'dev', 'max_prepared_transactions': '5', }) try: