InnoDB review fixes

Fix the formatting, and remove the MONITOR interface.
Remove unnecessary wrapper functions for the callbacks,
and replace void* with ha_innobase*.

mysql-test/suite/innodb/r/innodb_skip_innodb_is_tables.result

@@ -286,9 +286,6 @@ icp_attempts	icp	0	NULL	NULL	NULL	0	NULL	NULL	NULL	NULL	NULL	NULL	NULL	disabled
 icp_no_match	icp	0	NULL	NULL	NULL	0	NULL	NULL	NULL	NULL	NULL	NULL	NULL	disabled	counter	Index push-down condition does not match
 icp_out_of_range	icp	0	NULL	NULL	NULL	0	NULL	NULL	NULL	NULL	NULL	NULL	NULL	disabled	counter	Index push-down condition out of range
 icp_match	icp	0	NULL	NULL	NULL	0	NULL	NULL	NULL	NULL	NULL	NULL	NULL	disabled	counter	Index push-down condition matches
-pk-filter checks	pk-filter	0	NULL	NULL	NULL	0	NULL	NULL	NULL	NULL	NULL	NULL	NULL	disabled	counter	Number of lookups into PK-filters
-pk-filter_positive	pk-filter	0	NULL	NULL	NULL	0	NULL	NULL	NULL	NULL	NULL	NULL	NULL	disabled	counter	PK-filter test is positive
-pk-filter_negative	pk-filter	0	NULL	NULL	NULL	0	NULL	NULL	NULL	NULL	NULL	NULL	NULL	disabled	counter	PK-filter test is negative
 select * from information_schema.innodb_ft_default_stopword;
 value
 a

mysql-test/suite/innodb/r/monitor.result

@@ -251,9 +251,6 @@ icp_attempts	disabled
 icp_no_match	disabled
 icp_out_of_range	disabled
 icp_match	disabled
-pk-filter checks	disabled
-pk-filter_positive	disabled
-pk-filter_negative	disabled
 set global innodb_monitor_enable = all;
 select name from information_schema.innodb_metrics where status!='enabled';
 name

storage/innobase/handler/ha_innodb.cc

@@ -4,7 +4,7 @@ Copyright (c) 2000, 2018, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2008, 2009 Google Inc.
 Copyright (c) 2009, Percona Inc.
 Copyright (c) 2012, Facebook Inc.
-Copyright (c) 2013, 2018, MariaDB Corporation.
+Copyright (c) 2013, 2019, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted by
 Google, Inc. Those modifications are gratefully acknowledged and are described
@@ -5238,25 +5238,21 @@ ha_innobase::index_flags(
 		return(0);
 	}
 
-	ulong extra_flag= 0;
-
-	if (table && key == table->s->primary_key) {
-		extra_flag= HA_CLUSTERED_INDEX;
-	}
-
-	ulong flags = HA_READ_NEXT | HA_READ_PREV | HA_READ_ORDER
-		      | HA_READ_RANGE | HA_KEYREAD_ONLY
-		      | extra_flag
-		      | HA_DO_INDEX_COND_PUSHDOWN
-                      | HA_DO_RANGE_FILTER_PUSHDOWN;
-
 	/* For spatial index, we don't support descending scan
 	and ICP so far. */
 	if (table_share->key_info[key].flags & HA_SPATIAL) {
-		flags = HA_READ_NEXT | HA_READ_ORDER| HA_READ_RANGE
+		return HA_READ_NEXT | HA_READ_ORDER| HA_READ_RANGE
 			| HA_KEYREAD_ONLY | HA_KEY_SCAN_NOT_ROR;
 	}
 
+	ulong flags= key == table_share->primary_key
+		? HA_CLUSTERED_INDEX : 0;
+
+	flags |= HA_READ_NEXT | HA_READ_PREV | HA_READ_ORDER
+		| HA_READ_RANGE | HA_KEYREAD_ONLY
+		| HA_DO_INDEX_COND_PUSHDOWN
+		| HA_DO_RANGE_FILTER_PUSHDOWN;
+
 	return(flags);
 }
 
@@ -7610,8 +7606,8 @@ ha_innobase::build_template(
 	ulint num_v = 0;
 
 	if ((active_index != MAX_KEY
-             && active_index == pushed_idx_cond_keyno) ||
+	     && active_index == pushed_idx_cond_keyno)
-            (pushed_rowid_filter && rowid_filter_is_active)) {
+	    || (pushed_rowid_filter && rowid_filter_is_active)) {
 		/* Push down an index condition or an end_range check. */
 		for (ulint i = 0; i < n_fields; i++) {
 			const Field* field = table->field[i];
@@ -20245,36 +20241,6 @@ ha_innobase::multi_range_read_explain_info(
 	return m_ds_mrr.dsmrr_explain_info(mrr_mode, str, size);
 }
 
-/**
-Index Condition Pushdown interface implementation */
-
-/*************************************************************//**
-InnoDB index push-down condition check
-@return ICP_NO_MATCH, ICP_MATCH, or ICP_OUT_OF_RANGE */
-ICP_RESULT
-innobase_index_cond(
-/*================*/
-	void*	file)	/*!< in/out: pointer to ha_innobase */
-{
-	return handler_index_cond_check(file);
-}
-
-bool
-innobase_pk_filter(
-/*===============*/
-	void* file)     /*!< in/out: pointer to ha_innobase */
-{
-        return handler_rowid_filter_check(file);
-}
-
-bool
-innobase_pk_filter_is_active(
-/*==========================*/
-	void* file)     /*!< in/out: pointer to ha_innobase */
-{
-        return handler_rowid_filter_is_active(file);
-}
-
 /** Parse the table file name into table name and database name.
 @param[in]	tbl_name	InnoDB table name
 @param[out]	dbname		database name buffer (NAME_LEN + 1 bytes)
@@ -20811,18 +20777,16 @@ ha_innobase::idx_cond_push(
 }
 
 
-/** Push primary key filter.
+/** Push a primary key filter.
-@param[in] pk_filter  PK filter against which primary keys
+@param[in]	pk_filter	filter against which primary keys
-                      are to be checked */
+				are to be checked
-
+@retval	false if pushed (always) */
-bool
+bool ha_innobase::rowid_filter_push(Rowid_filter* pk_filter)
-ha_innobase::rowid_filter_push(
-        class Rowid_filter* pk_filter)
 {
 	DBUG_ENTER("ha_innobase::rowid_filter_push");
 	DBUG_ASSERT(pk_filter != NULL);
 	pushed_rowid_filter= pk_filter;
-        DBUG_RETURN(FALSE);
+	DBUG_RETURN(false);
 }
 
 /******************************************************************//**

storage/innobase/handler/ha_innodb.h

@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 2000, 2017, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2013, 2018, MariaDB Corporation.
+Copyright (c) 2013, 2019, 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
@@ -421,11 +421,11 @@ public:
 	Item* idx_cond_push(uint keyno, Item* idx_cond);
 	/* @} */
 
-        /** Attempt to push down a rowid filter
+	/** Push a primary key filter.
-        @param[in] pk_filter Handle of the rowid filter to be pushed.
+	@param[in]	pk_filter	filter against which primary keys
-        #return 0 pk-filter is pushed; NULL if not pushed */
+					are to be checked
-	bool rowid_filter_push(class Rowid_filter *rowid_filter);
+	@retval	false if pushed (always) */
-	/* @} */
+	bool rowid_filter_push(Rowid_filter *rowid_filter);
 
 protected:
 

storage/innobase/include/ha_prototypes.h

@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 2006, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2017, 2018, MariaDB Corporation.
+Copyright (c) 2017, 2019, 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
@@ -544,35 +544,6 @@ normalize_table_name_c_low(
 	const char*	name,		/*!< in: table name string */
 	ibool		set_lower_case); /*!< in: TRUE if we want to set
 					name to lower case */
-/*************************************************************//**
-InnoDB index push-down condition check defined in ha_innodb.cc
-@return ICP_NO_MATCH, ICP_MATCH, or ICP_OUT_OF_RANGE */
-
-#include <my_compare.h>
-
-ICP_RESULT
-innobase_index_cond(
-/*================*/
-	void*	file)	/*!< in/out: pointer to ha_innobase */
-	MY_ATTRIBUTE((warn_unused_result));
-
-/*************************************************************//**
-InnoDB Rowid filter check defined in ha_innodb.cc */
-
-bool
-innobase_pk_filter(
-/*================*/
-	void*	file)	/*!< in/out: pointer to ha_innobase */
-	MY_ATTRIBUTE((warn_unused_result));
-
-/*************************************************************//**
-InnoDB check whether pk-filter is active */
-
-bool
-innobase_pk_filter_is_active(
-/*==========================*/
-	void*	file);	/*!< in/out: pointer to ha_innobase */
-
 /******************************************************************//**
 Gets information on the durability property requested by thread.
 Used when writing either a prepare or commit record to the log

storage/innobase/include/row0mysql.h

@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 2000, 2017, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2017, 2018, MariaDB Corporation.
+Copyright (c) 2017, 2019, 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
@@ -43,6 +43,7 @@ Created 9/17/2000 Heikki Tuuri
 extern ibool row_rollback_on_timeout;
 
 struct row_prebuilt_t;
+class ha_innobase;
 
 /*******************************************************************//**
 Frees the blob heap in prebuilt when no longer needed. */
@@ -788,18 +789,18 @@ struct row_prebuilt_t {
 					store it here so that we can return
 					it to MySQL */
 	/*----------------------*/
-	void*		idx_cond;	/*!< In ICP, pointer to a ha_innobase,
+
-					passed to innobase_index_cond().
+	/** Argument of handler_rowid_filter_check(),
-					NULL if index condition pushdown is
+	or NULL if no PRIMARY KEY filter is pushed */
-					not used. */
+	ha_innobase*	pk_filter;
+
+	/** Argument to handler_index_cond_check(),
+	or NULL if no index condition pushdown (ICP) is used. */
+	ha_innobase*	idx_cond;
 	ulint		idx_cond_n_cols;/*!< Number of fields in idx_cond_cols.
 					0 if and only if idx_cond == NULL. */
 	/*----------------------*/
 
-        void*           pk_filter;      /*!< In PK-filters, pointer to a ha_innobase,
-                                        passed to innobase_pk_filter().
-                                        NULL if no PK-filter is pushed. */
-
 	/*----------------------*/
 	rtr_info_t*	rtr_info;	/*!< R-tree Search Info */
 	/*----------------------*/

storage/innobase/include/srv0mon.h

@@ -443,11 +443,6 @@ enum monitor_id_t {
 	MONITOR_ICP_OUT_OF_RANGE,
 	MONITOR_ICP_MATCH,
 
-	MONITOR_MODULE_PK_FILTER,
-	MONITOR_PK_FILTER_CHECKS,
-	MONITOR_PK_FILTER_POSITIVE,
-	MONITOR_PK_FILTER_NEGATIVE,
-
 	/* Mutex/RW-Lock related counters */
 	MONITOR_MODULE_LATCHES,
 	MONITOR_LATCHES,

storage/innobase/row/row0sel.cc

@@ -3129,8 +3129,9 @@ row_sel_store_mysql_rec(
 			= rec_clust
 			? templ->clust_rec_field_no
 			: templ->rec_field_no;
-		/* We should never deliver column prefixes to MySQL,
+		/* We should never deliver column prefixes to the SQL layer,
-		except for evaluating innobase_index_cond(). */
+		except for evaluating handler_index_cond_check()
+		or handler_rowid_filter_check(). */
 		/* ...actually, we do want to do this in order to
 		support the prefix query optimization.
 
@@ -3758,7 +3759,7 @@ row_sel_enqueue_cache_row_for_mysql(
 	/* For non ICP code path the row should already exist in the
 	next fetch cache slot. */
 
-	if (prebuilt->idx_cond != NULL || prebuilt->pk_filter != NULL ) {
+	if (prebuilt->pk_filter || prebuilt->idx_cond) {
 		byte*	dest = row_sel_fetch_last_buf(prebuilt);
 
 		ut_memcpy(dest, mysql_rec, prebuilt->mysql_row_len);
@@ -3856,13 +3857,12 @@ row_search_idx_cond_check(
 	const rec_t*		rec,		/*!< in: InnoDB record */
 	const ulint*		offsets)	/*!< in: rec_get_offsets() */
 {
-	ICP_RESULT	result;
 	ulint		i;
 
 	ut_ad(rec_offs_validate(rec, prebuilt->index, offsets));
 
 	if (!prebuilt->idx_cond) {
-	        if (!(innobase_pk_filter_is_active(prebuilt->pk_filter))) {
+		if (!handler_rowid_filter_is_active(prebuilt->pk_filter)) {
 			return(ICP_MATCH);
 		}
 	} else {
@@ -3898,25 +3898,17 @@ row_search_idx_cond_check(
 	index, if the case of the column has been updated in
 	the past, or a record has been deleted and a record
 	inserted in a different case. */
-        if (prebuilt->idx_cond) {
+	ICP_RESULT result = prebuilt->idx_cond
-	        result = innobase_index_cond(prebuilt->idx_cond);
+		? handler_index_cond_check(prebuilt->idx_cond)
-        } else {
+		: ICP_MATCH;
-	        result = ICP_MATCH;
+
-        }
 	switch (result) {
 	case ICP_MATCH:
-	        if (innobase_pk_filter_is_active(prebuilt->pk_filter)) {
+		if (handler_rowid_filter_is_active(prebuilt->pk_filter)
-		         bool pkf_result;
+		    && !handler_rowid_filter_check(prebuilt->pk_filter)) {
-                         MONITOR_INC(MONITOR_PK_FILTER_CHECKS);
-                         pkf_result = innobase_pk_filter(prebuilt->pk_filter);
-                         if (pkf_result) {
-                                 MONITOR_INC(MONITOR_PK_FILTER_POSITIVE);
-                         } else {
-                                 MONITOR_INC(MONITOR_PK_FILTER_NEGATIVE);
 			MONITOR_INC(MONITOR_ICP_MATCH);
 			return(ICP_NO_MATCH);
 		}
-                }
 		/* Convert the remaining fields to MySQL format.
 		If this is a secondary index record, we must defer
 		this until we have fetched the clustered index record. */
@@ -4369,7 +4361,7 @@ row_search_mvcc(
 				mtr.commit(). */
 				ut_ad(!rec_get_deleted_flag(rec, comp));
 
-				if (prebuilt->idx_cond || prebuilt->pk_filter) {
+				if (prebuilt->pk_filter || prebuilt->idx_cond) {
 					switch (row_search_idx_cond_check(
 							buf, prebuilt,
 							rec, offsets)) {
@@ -5308,7 +5300,7 @@ requires_clust_rec:
 		result_rec = clust_rec;
 		ut_ad(rec_offs_validate(result_rec, clust_index, offsets));
 
-		if (prebuilt->idx_cond || prebuilt->pk_filter) {
+		if (prebuilt->pk_filter || prebuilt->idx_cond) {
 			/* Convert the record to MySQL format. We were
 			unable to do this in row_search_idx_cond_check(),
 			because the condition is on the secondary index
@@ -5369,8 +5361,7 @@ use_covering_index:
 		/* We only convert from InnoDB row format to MySQL row
 		format when ICP is disabled. */
 
-		if (!(prebuilt->idx_cond || prebuilt->pk_filter)) {
+		if (!prebuilt->pk_filter && !prebuilt->idx_cond) {
-
 			/* We use next_buf to track the allocation of buffers
 			where we store and enqueue the buffers for our
 			pre-fetch optimisation.
@@ -5442,7 +5433,7 @@ use_covering_index:
 			       rec_offs_size(offsets));
 			mach_write_to_4(buf,
 					rec_offs_extra_size(offsets) + 4);
-		} else if (!(prebuilt->idx_cond || prebuilt->pk_filter)) {
+		} else if (!prebuilt->pk_filter && !prebuilt->idx_cond) {
 			/* The record was not yet converted to MySQL format. */
 			if (!row_sel_store_mysql_rec(
 				    buf, prebuilt, result_rec, vrow,
@@ -5684,8 +5675,7 @@ normal_return:
 
 	DEBUG_SYNC_C("row_search_for_mysql_before_return");
 
-	if (prebuilt->idx_cond != 0 || prebuilt->pk_filter != 0) {
+	if (prebuilt->pk_filter || prebuilt->idx_cond) {
-
 		/* When ICP is active we don't write to the MySQL buffer
 		directly, only to buffers that are enqueued in the pre-fetch
 		queue. We need to dequeue the first buffer and copy the contents

storage/innobase/srv/srv0mon.cc

@@ -1400,24 +1400,6 @@ static monitor_info_t	innodb_counter_info[] =
 	 MONITOR_NONE,
 	 MONITOR_DEFAULT_START, MONITOR_ICP_MATCH},
 
-	/* ===== Counters for PK-filters Module ===== */
-	{"module_pk-filter", "pk-filter", "Primary Keys Filtering",
-	 MONITOR_MODULE,
-	 MONITOR_DEFAULT_START, MONITOR_MODULE_PK_FILTER},
-
-	{"pk-filter checks", "pk-filter",
-	 "Number of lookups into PK-filters",
-	 MONITOR_NONE,
-	 MONITOR_DEFAULT_START, MONITOR_PK_FILTER_CHECKS},
-
-	{"pk-filter_positive", "pk-filter", "PK-filter test is positive",
-	 MONITOR_NONE,
-	 MONITOR_DEFAULT_START, MONITOR_PK_FILTER_POSITIVE},
-
-	{"pk-filter_negative", "pk-filter", "PK-filter test is negative",
-	 MONITOR_NONE,
-	 MONITOR_DEFAULT_START, MONITOR_PK_FILTER_NEGATIVE},
-
 	/* ========== Mutex monitoring on/off ========== */
 	{"latch_status", "Latch counters",
 	 "Collect latch counters to display via SHOW ENGING INNODB MUTEX",