Add input variable validation to the diagnostics() routine

Author: MarkLeith

mysql-test/suite/sysschema/r/pr_diagnostics.result

@@ -11,3 +11,11 @@ SET GLOBAL show_compatibility_56 = @tmp_show_compatibility_56,
 SET @sys.debug                        = 'OFF',
 @sys.diagnostics.allow_i_s_tables = 'OFF',
 @sys.diagnostics.include_raw      = 'OFF';
+CALL sys.diagnostics(0, 0, 'full');
+summary
+Disabled 1 thread
+ERROR 45000: in_max_runtime must be greater than 0
+CALL sys.diagnostics(2, 0, 'full');
+ERROR 45000: in_interval must be greater than 0
+CALL sys.diagnostics(1, 2, 'full');
+ERROR 45000: in_max_runtime must be greater than or equal to in_interval

mysql-test/suite/sysschema/t/pr_diagnostics.test

@@ -35,3 +35,11 @@ SET @sys.debug                        = 'OFF',
     @sys.diagnostics.allow_i_s_tables = 'OFF',
     @sys.diagnostics.include_raw      = 'OFF';
 --enable_result_log
+
+# Check input variable validation
+-- error S45000
+CALL sys.diagnostics(0, 0, 'full');
+-- error S45000
+CALL sys.diagnostics(2, 0, 'full');
+-- error S45000
+CALL sys.diagnostics(1, 2, 'full');

procedures/diagnostics.sql

@@ -57,10 +57,10 @@ CREATE DEFINER='root'@'localhost' PROCEDURE diagnostics (
 
              in_max_runtime (INT UNSIGNED):
                The maximum time to keep collecting data.
-               Use NULL to get the default which is 60 seconds.
+               Use NULL to get the default which is 60 seconds, otherwise enter a value greater than 0.
              in_interval (INT UNSIGNED):
                How long to sleep between data collections.
-               Use NULL to get the default which is 30 seconds.
+               Use NULL to get the default which is 30 seconds, otherwise enter a value greater than 0.
              in_auto_config (ENUM(''current'', ''medium'', ''full''))
                Automatically enable Performance Schema instruments and consumers.
                NOTE: The more that are enabled, the more impact on the performance.
@@ -149,35 +149,26 @@ BEGIN
          ORDER BY table_name; 
     DECLARE CONTINUE HANDLER FOR NOT FOUND SET v_done = TRUE;
 
-    -- Some metrics variables doesn't make sense in delta and rate calculations even if they are numeric
-    -- as they really are more like settings or "current" status.
-    SET v_no_delta_names = CONCAT('s%{COUNT}.Variable_name NOT IN (',
-        '''innodb_buffer_pool_pages_total'', ',
-        '''innodb_page_size'', ',
-        '''last_query_cost'', ',
-        '''last_query_partial_plans'', ',
-        '''qcache_total_blocks'', ',
-        '''slave_last_heartbeat'', ',
-        '''ssl_ctx_verify_depth'', ',
-        '''ssl_ctx_verify_mode'', ',
-        '''ssl_session_cache_size'', ',
-        '''ssl_verify_depth'', ',
-        '''ssl_verify_mode'', ',
-        '''ssl_version'', ',
-        '''buffer_flush_lsn_avg_rate'', ',
-        '''buffer_flush_pct_for_dirty'', ',
-        '''buffer_flush_pct_for_lsn'', ',
-        '''buffer_pool_pages_total'', ',
-        '''lock_row_lock_time_avg'', ',
-        '''lock_row_lock_time_max'', ',
-        '''innodb_page_size''',
-    ')');
-
     -- Do not track the current thread - no reason to clutter the output
     SELECT INSTRUMENTED INTO v_this_thread_enabled FROM performance_schema.threads WHERE PROCESSLIST_ID = CONNECTION_ID();
     IF (v_this_thread_enabled = 'YES') THEN
         CALL sys.ps_setup_disable_thread(CONNECTION_ID());
     END IF;
+
+    -- Check options are sane
+    IF (in_max_runtime < in_interval) THEN
+        SIGNAL SQLSTATE '45000'
+           SET MESSAGE_TEXT = 'in_max_runtime must be greater than or equal to in_interval';
+    END IF;
+    IF (in_max_runtime = 0) THEN
+        SIGNAL SQLSTATE '45000'
+           SET MESSAGE_TEXT = 'in_max_runtime must be greater than 0';
+    END IF;
+    IF (in_interval = 0) THEN
+        SIGNAL SQLSTATE '45000'
+           SET MESSAGE_TEXT = 'in_interval must be greater than 0';
+    END IF;
+
     -- Set configuration options
     IF (@sys.diagnostics.allow_i_s_tables IS NULL) THEN
         SET @sys.diagnostics.allow_i_s_tables = sys.sys_get_config('diagnostics.allow_i_s_tables', 'OFF');
@@ -198,6 +189,30 @@ BEGIN
         SET sql_log_bin = 0;
     END IF;
 
+    -- Some metrics variables doesn't make sense in delta and rate calculations even if they are numeric
+    -- as they really are more like settings or "current" status.
+    SET v_no_delta_names = CONCAT('s%{COUNT}.Variable_name NOT IN (',
+        '''innodb_buffer_pool_pages_total'', ',
+        '''innodb_page_size'', ',
+        '''last_query_cost'', ',
+        '''last_query_partial_plans'', ',
+        '''qcache_total_blocks'', ',
+        '''slave_last_heartbeat'', ',
+        '''ssl_ctx_verify_depth'', ',
+        '''ssl_ctx_verify_mode'', ',
+        '''ssl_session_cache_size'', ',
+        '''ssl_verify_depth'', ',
+        '''ssl_verify_mode'', ',
+        '''ssl_version'', ',
+        '''buffer_flush_lsn_avg_rate'', ',
+        '''buffer_flush_pct_for_dirty'', ',
+        '''buffer_flush_pct_for_lsn'', ',
+        '''buffer_pool_pages_total'', ',
+        '''lock_row_lock_time_avg'', ',
+        '''lock_row_lock_time_max'', ',
+        '''innodb_page_size''',
+    ')');
+
     IF (in_auto_config <> 'current') THEN
         IF (@sys.debug = 'ON') THEN
             SELECT CONCAT('Updating Performance Schema configuration to ', in_auto_config) AS 'Debug';