MDEV-23161 avg_count_reset may wrongly be NULL in I_S.INNODB_METRICS

This issue was originally reported by Fungo Wang, along with a fix, as MySQL Bug #98990. His suggested fix was applied as part of mysql/mysql-server@a003fc3 and released in MySQL 5.7.31. i_s_metrics_fill(): Add the missing call to Field::set_notnull(), and simplify some code.
MariaDB · Jul 14, 2020 · 8d06199 · 8d06199
1 parent dc58987
commit 8d06199
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 26 deletions.
diff --git a/storage/innobase/handler/i_s.cc b/storage/innobase/handler/i_s.cc
@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 2007, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2014, 2019, MariaDB Corporation.
+Copyright (c) 2014, 2020, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -2557,16 +2557,17 @@ i_s_metrics_fill(
 			time_diff = 0;
 		}
 
-		/* Unless MONITOR__NO_AVERAGE is marked, we will need
+		/* Unless MONITOR_NO_AVERAGE is set, we must
 		to calculate the average value. If this is a monitor set
 		owner marked by MONITOR_SET_OWNER, divide
 		the value by another counter (number of calls) designated
 		by monitor_info->monitor_related_id.
 		Otherwise average the counter value by the time between the
 		time that the counter is enabled and time it is disabled
 		or time it is sampled. */
-		if (!(monitor_info->monitor_type & MONITOR_NO_AVERAGE)
-		    && (monitor_info->monitor_type & MONITOR_SET_OWNER)
+		if ((monitor_info->monitor_type
+		     & (MONITOR_NO_AVERAGE | MONITOR_SET_OWNER))
+		    == MONITOR_SET_OWNER
 		    && monitor_info->monitor_related_id) {
 			mon_type_t	value_start
 				 = MONITOR_VALUE_SINCE_START(
@@ -2582,18 +2583,18 @@ i_s_metrics_fill(
 				fields[METRIC_AVG_VALUE_START]->set_null();
 			}
 
-			if (MONITOR_VALUE(monitor_info->monitor_related_id)) {
-				OK(fields[METRIC_AVG_VALUE_RESET]->store(
-					MONITOR_VALUE(count)
-					/ MONITOR_VALUE(
-					monitor_info->monitor_related_id),
-					FALSE));
+			if (mon_type_t related_value =
+			    MONITOR_VALUE(monitor_info->monitor_related_id)) {
+				OK(fields[METRIC_AVG_VALUE_RESET]
+				   ->store(MONITOR_VALUE(count)
+					   / related_value, false));
+				fields[METRIC_AVG_VALUE_RESET]->set_notnull();
 			} else {
 				fields[METRIC_AVG_VALUE_RESET]->set_null();
 			}
-		} else if (!(monitor_info->monitor_type & MONITOR_NO_AVERAGE)
-			   && !(monitor_info->monitor_type
-				& MONITOR_DISPLAY_CURRENT)) {
+		} else if (!(monitor_info->monitor_type
+			     & (MONITOR_NO_AVERAGE
+				| MONITOR_DISPLAY_CURRENT))) {
 			if (time_diff) {
 				OK(fields[METRIC_AVG_VALUE_START]->store(
 					(double) MONITOR_VALUE_SINCE_START(

diff --git a/storage/xtradb/handler/i_s.cc b/storage/xtradb/handler/i_s.cc
@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 2007, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2014, 2019, MariaDB Corporation.
+Copyright (c) 2014, 2020, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -2553,16 +2553,17 @@ i_s_metrics_fill(
 			time_diff = 0;
 		}
 
-		/* Unless MONITOR__NO_AVERAGE is marked, we will need
+		/* Unless MONITOR_NO_AVERAGE is set, we must
 		to calculate the average value. If this is a monitor set
 		owner marked by MONITOR_SET_OWNER, divide
 		the value by another counter (number of calls) designated
 		by monitor_info->monitor_related_id.
 		Otherwise average the counter value by the time between the
 		time that the counter is enabled and time it is disabled
 		or time it is sampled. */
-		if (!(monitor_info->monitor_type & MONITOR_NO_AVERAGE)
-		    && (monitor_info->monitor_type & MONITOR_SET_OWNER)
+		if ((monitor_info->monitor_type
+		     & (MONITOR_NO_AVERAGE | MONITOR_SET_OWNER))
+		    == MONITOR_SET_OWNER
 		    && monitor_info->monitor_related_id) {
 			mon_type_t	value_start
 				 = MONITOR_VALUE_SINCE_START(
@@ -2578,18 +2579,18 @@ i_s_metrics_fill(
 				fields[METRIC_AVG_VALUE_START]->set_null();
 			}
 
-			if (MONITOR_VALUE(monitor_info->monitor_related_id)) {
-				OK(fields[METRIC_AVG_VALUE_RESET]->store(
-					MONITOR_VALUE(count)
-					/ MONITOR_VALUE(
-					monitor_info->monitor_related_id),
-					FALSE));
+			if (mon_type_t related_value =
+			    MONITOR_VALUE(monitor_info->monitor_related_id)) {
+				OK(fields[METRIC_AVG_VALUE_RESET]
+				   ->store(MONITOR_VALUE(count)
+					   / related_value, false));
+				fields[METRIC_AVG_VALUE_RESET]->set_notnull();
 			} else {
 				fields[METRIC_AVG_VALUE_RESET]->set_null();
 			}
-		} else if (!(monitor_info->monitor_type & MONITOR_NO_AVERAGE)
-			   && !(monitor_info->monitor_type
-				& MONITOR_DISPLAY_CURRENT)) {
+		} else if (!(monitor_info->monitor_type
+			     & (MONITOR_NO_AVERAGE
+				| MONITOR_DISPLAY_CURRENT))) {
 			if (time_diff) {
 				OK(fields[METRIC_AVG_VALUE_START]->store(
 					(double) MONITOR_VALUE_SINCE_START(