List:Commits« Previous MessageNext Message »
From:vasil.dimov Date:June 19 2012 10:23am
Subject:bzr push into mysql-trunk branch (vasil.dimov:4017 to 4019)
View as plain text  
 4019 Vasil Dimov	2012-06-19
      Non-functional change - use bool instead of ibool in dict0stats.cc

    modified:
      storage/innobase/dict/dict0stats.cc
 4018 Vasil Dimov	2012-06-19
      Fix Bug#14172780 CRASH OBSERVED ININNODB.INNODB-WL5522-DEBUG* TEST ON PB2 ON
      MULTIPLE PLATFORMS.
      
      In dict_stats_analyze_index_level(), when we traverse the level we take care
      to copy the rec into prev_rec when we are about to jump on the next page but
      we forgot to do this in the case when the last rec on a page is delete-marked.
      In this case we just "continue;", leaving prev_rec pointing somewhere inside
      the old page which is going to be released by btr_pcur_move_to_next_user_rec()
      when it jumps on the next page.
      
      The fix is to also copy rec into prev_rec before "continue;"ing when a
      delete-marked record is found and it is last on page and prev_rec is not
      already copied (ie we may have this cond twice if a whole page is filled
      with delete-marked records and prev_rec is (copied) from pageN where rec
      jumps into pageN+1, which contains only delete-marked records and then rec
      jumps into pageN+2 - no need to copy prev_rec again in this case).

    modified:
      storage/innobase/dict/dict0stats.cc
 4017 Alexander Barkov	2012-06-19
      Bug#13833019 ASSERTION `T1->RESULT_RANGE' FAILED IN GCALC_OPERATION_R
      Bug#13832749 HANDLE_FATAL_SIGNAL IN GCALC_FUNCTION::COUNT_INTERNAL
      
      1. ST_Buffer() did not work well due to a few bugs in the
        buffer transporter implementation.
      
      a. The sequence of:
      
        func.get_next_operation_pos();
        ...
        func.add_operands_to_op(union_pos, trn.m_nshapes);
      
        did not work in case of collection geometries
        (MultiPoint, MultiLineString, MultiPolygon, GeometryCollection),
        which already create a union of m_nshapes items themself.
        In this case the caller should have generated a union1 operation
        instead of unionN operation, i.e. the above add_operands_to_op()
        call should have passed 1 instead of trn.m_nshapes,
        to get this function execution plan:
      
          union1(unionN(shape1,shape2,...,shapeN))
      
        instead of bad:
      
          unionN(unionN(shape1,shape2,..,shapeN))
      
        The fix moves the code generating the upper-most union command
        *inside* store_shape() for all geometry types.
      
      b. The next problem after moving union generating inside store_shapes()
         was that the Item_func_buffer::Transporter::m_nshapes member was
         erroneously shared by both collection itself and its items, e.g.:
      
        - lines of multilinestring and the multiline itself
        - items of a geometry collection and the collection itself
      
        The fix removes the m_nshapes member from Item_func_buffer::Transporter
        and passes the current shape transformation status as a parameter
        to all tranformation methods line stat_xxx/complete_xxx/add_point, etc.
      
        This simplifies simultanious access to the transformation statistics
        of the collection itself and of the current item.
        A new class Gcalc_shape_status was introduces to store transformation status.
        Before a collection processing code enters an item, a new transformation
        status variable is created on stack and passed to the item processing
        routine.
      
      c. ST_Buffer did not include the very last edge of a polygon with 4+ points
      due to the bug in the point comparison code in
      Item_func_buffer::Transporter::complete:
      
      - if (x2 != x00 && y2 != y00)
      + if (x2 != x00 || y2 != y00)
      
      
      2. Area() of a geometry collection consisting of polygons mistakenly
        returned NULL, because Gis_geometry_collection::area was not implemented.
        Area() of Points and LineStrings also returned NULL. Fixing to return 0.
      
      
      3. Negative buffer values worked wrong:
      
      - ST_Buffer(GeomFromText('POINT(0 0)',-1));
      
        Returned similar value to 
      
        ST_Buffer(GeomFromText('POINT(0 0)',1));
      
      - ST_Buffer(GeomFromText('LineString(0 0)'),-1)
      
        returned similar value to
      
        ST_Buffer(GeomFromText('LineString(0 0)'),1)
      
      Fixing negative buffers to return empty sets for Points and LineStrings,
      according to the OCG specifications.
      
      4. Zero buffer made server crash:
      
        ST_Buffer(<polygon>, 0)
      
        because of division-by-zero-alike problem.
        Changing zero buffer to return the original geometry itself,
        instead of going through the zero-unsafe buffer generating code.
      
      5. Adding GIS debug output functionality, to print debug information
      into server log and into client side warnings. The latter is very useful
      for mtr purposes. GIS debug output is available only in debug builds
      and does not exist in production builds.
      
      6. Adding ST_GIS_DEBUG() function to turn client-side GIS debug warnings on/off:
      
      SELECT ST_GIS_DEBUG(1); -- turn debug output on
      SELECT ST_GIS_DEBUG(0); -- turn debug output off
      
      Client side debug warnings are off by default.
      
      
      Per file comments:
      
      added:
        @ mysql-test/include/gis_debug.inc
          - Adding a shared file to test various GIS functions
      
        @ mysql-test/r/gis-debug.result
        @ mysql-test/t/gis-debug.test
          - Adding a debug test with client-side GIS debug warnings.
      
      modified:
        @ mysql-test/r/gis-precise.result
        @ mysql-test/t/gis-precise.test
          - Including gis_debug.inc,
            to make sure the results are correct in non-debugs.
      
        @ sql/gcalc_slicescan.h.diff
          - Introducing a new class Gcalc_shape_status and passing
            a parameter of Gcalc_shape_status type into transformation methods.
          - Adding prototypes for collection_add_item() and complete_collection()
            transformation methods. Previously these methods did not exist
            because Item_func_buffer::val_str() tried to process the upper-most
            union itself (incorrectly).
      
          - Adding skip_xxx() methods, to exclude certain geometries during
            transformation. For example, negative buffer does not need Points
            and LineStrings.
      
        @ sql/gcalc_tools.cc.diff
          - Adding debug routines
          - Adding Gcalc_shape_status parameter into the transformation methods
          - Adding the default implementation of start_collection(),
            collection_add_item() and complete_collection() routines.
      
        @ sql/gcalc_tools.h.diff
          - Adding prototype for the debug routine
          - Adding an Gcalc_shape_status parameter to transformation routines
      
        @ sql/item_create.cc.diff
          - Adding a code to create ST_GIS_DEBUG() SQL function in debug builds
      
        @ sql/item_geofunc.cc.diff
      
          - Adding Gcalc_shape_status parameter into the transformation methods
          - Adding a new code to catch zero distance
          - Adding a new code to catch empty buffer set after transformation
          - Removing the top-level union generating code
          - Adding Item_func_gis_debug::val_int() implementation
          - Adding calls for debug_print_function_buffer() near all store_shapes()
            calls.
      
        @ sql/item_geofunc.h.diff
          - Removing the m_nshapes member
          - Adding Gcalc_shape_status parameter to the transformation methods
          - Adding prototype for Item_func_gis_debug,
            the item behind the new ST_GIS_DEBUG() SQL function.
      
        @ sql/spatial.cc.diff
          - Implementing Geometry::collection_store_shapes() and
            Geometry::collection_area() methods, which are shared between
            collection geometry types:
            GeometryCollection, MultiPoint, MultiLineString,MultiPolygon
          - Adding Gcalc_shape_status parameter to the transformation methods
          - Implementing the missing Gis_geometry_collection::area().
      
        @ sql/spatial.h.diff
          - Adding prototypes for collection_store_shapes() and collection_area().
          - Adding prototype for the missing Gis_geometry_collection::area().
          - Adding the default implementation for area(), for the geometry
            types who have empty area.
          - Adding Gcalc_shape_status parameter to the transformation methods
        @ sql/sql_class.cc.diff
          - Adding gis_debug initialization into THD constructor
      
        @ sql/sql_class.h.diff
          - Introducing gis_debug member and related methods.

    modified:
      mysql-test/r/gis-precise.result
      mysql-test/t/gis-precise.test
      sql/gcalc_slicescan.h
      sql/gcalc_tools.cc
      sql/gcalc_tools.h
      sql/item_create.cc
      sql/item_geofunc.cc
      sql/item_geofunc.h
      sql/spatial.cc
      sql/spatial.h
      sql/sql_class.cc
      sql/sql_class.h
=== modified file 'storage/innobase/dict/dict0stats.cc'
--- a/storage/innobase/dict/dict0stats.cc	revid:alexander.barkov@stripped
+++ b/storage/innobase/dict/dict0stats.cc	revid:vasil.dimov@stripped
@@ -35,6 +35,7 @@ Created Jan 06, 2010 Vasil Dimov
 #include "data0type.h" /* dtype_t */
 #include "db0err.h" /* dberr_t */
 #include "dyn0dyn.h" /* dyn_array* */
+#include "page0page.h" /* page_align() */
 #include "pars0pars.h" /* pars_info_create() */
 #include "pars0types.h" /* pars_info_t */
 #include "que0que.h" /* que_eval_sql() */
@@ -138,12 +139,12 @@ from that level */
 Checks whether the persistent statistics storage exists and that all
 tables have the proper structure.
 dict_stats_persistent_storage_check() @{
-@return TRUE if exists and all tables are ok */
+@return true if exists and all tables are ok */
 static
-ibool
+bool
 dict_stats_persistent_storage_check(
 /*================================*/
-	ibool	caller_has_dict_sys_mutex)	/*!< in: TRUE if the caller
+	bool	caller_has_dict_sys_mutex)	/*!< in: true if the caller
 						owns dict_sys->mutex */
 {
 	/* definition for the table TABLE_STATS_NAME */
@@ -233,11 +234,11 @@ dict_stats_persistent_storage_check(
 	if (ret != DB_SUCCESS) {
 		ut_print_timestamp(stderr);
 		fprintf(stderr, " InnoDB: Error: %s\n", errstr);
-		return(FALSE);
+		return(false);
 	}
 	/* else */
 
-	return(TRUE);
+	return(true);
 }
 /* @} */
 
@@ -260,7 +261,7 @@ dict_stats_exec_sql(
 	ut_ad(rw_lock_own(&dict_operation_lock, RW_LOCK_EX));
 	ut_ad(mutex_own(&dict_sys->mutex));
 
-	if (!dict_stats_persistent_storage_check(TRUE)) {
+	if (!dict_stats_persistent_storage_check(true)) {
 		pars_info_free(pinfo);
 		return(DB_STATS_DO_NOT_EXIST);
 	}
@@ -780,9 +781,7 @@ dict_stats_analyze_index_level(
 	const page_t*	page;
 	const rec_t*	rec;
 	const rec_t*	prev_rec;
-#ifdef UNIV_DEBUG
-	int		prev_rec_was_copied;
-#endif
+	bool		prev_rec_is_copied;
 	byte*		prev_rec_buf = NULL;
 	ulint		prev_rec_buf_size = 0;
 	ulint		i;
@@ -846,9 +845,7 @@ dict_stats_analyze_index_level(
 	}
 
 	prev_rec = NULL;
-#ifdef UNIV_DEBUG
-	prev_rec_was_copied = 1;
-#endif
+	prev_rec_is_copied = false;
 
 	/* no records by default */
 	*total_recs = 0;
@@ -866,13 +863,26 @@ dict_stats_analyze_index_level(
 		ulint	matched_bytes = 0;
 		ulint	offsets_rec_onstack[REC_OFFS_NORMAL_SIZE];
 		ulint*	offsets_rec;
+		bool	rec_is_last_on_page;
 
 		rec_offs_init(offsets_rec_onstack);
 
 		rec = btr_pcur_get_rec(&pcur);
 
+		/* If rec and prev_rec are on different pages, then prev_rec
+		must have been copied, because we hold latch only on the page
+		where rec resides. */
+		if (prev_rec != NULL
+		    && page_align(rec) != page_align(prev_rec)) {
+
+			ut_a(prev_rec_is_copied);
+		}
+
+		rec_is_last_on_page =
+			page_rec_is_supremum(page_rec_get_next_const(rec));
+
 		/* increment the pages counter at the end of each page */
-		if (page_rec_is_supremum(page_rec_get_next_const(rec))) {
+		if (rec_is_last_on_page) {
 
 			(*total_pages)++;
 		}
@@ -888,6 +898,28 @@ dict_stats_analyze_index_level(
 			    rec,
 			    page_is_comp(btr_pcur_get_page(&pcur)))) {
 
+			if (rec_is_last_on_page
+			    && !prev_rec_is_copied
+			    && prev_rec != NULL) {
+				/* copy prev_rec */
+				ulint	offsets_prev_rec_onstack[
+					REC_OFFS_NORMAL_SIZE];
+				ulint*	offsets_prev_rec;
+
+				rec_offs_init(offsets_prev_rec_onstack);
+
+				offsets_prev_rec = rec_get_offsets(
+					prev_rec, index, offsets_prev_rec_onstack,
+					n_uniq, &heap);
+
+				prev_rec = rec_copy_prefix_to_buf(
+					prev_rec, index,
+					rec_offs_n_fields(offsets_prev_rec),
+					&prev_rec_buf, &prev_rec_buf_size);
+
+				prev_rec_is_copied = true;
+			}
+
 			continue;
 		}
 
@@ -918,7 +950,7 @@ dict_stats_analyze_index_level(
 						"rec_get_status(): %lu, "
 						"rec: %p, "
 						"prev_rec: %p, "
-						"prev_rec_was_copied: %d, "
+						"prev_rec_is_copied: %d, "
 						"page_align(rec): %p, "
 						"page_align(prev_rec): %p, "
 						"*total_recs: " UINT64PF ", "
@@ -926,7 +958,7 @@ dict_stats_analyze_index_level(
 						status,
 						rec,
 						prev_rec,
-						prev_rec_was_copied,
+						(int) prev_rec_is_copied,
 						page_align(rec),
 						page_align(prev_rec),
 						*total_recs,
@@ -1010,7 +1042,7 @@ dict_stats_analyze_index_level(
 			}
 		}
 
-		if (page_rec_is_supremum(page_rec_get_next_const(rec))) {
+		if (rec_is_last_on_page) {
 			/* end of a page has been reached */
 
 			/* we need to copy the record instead of assigning
@@ -1023,9 +1055,7 @@ dict_stats_analyze_index_level(
 			prev_rec = rec_copy_prefix_to_buf(
 				rec, index, rec_offs_n_fields(offsets_rec),
 				&prev_rec_buf, &prev_rec_buf_size);
-#ifdef UNIV_DEBUG
-			prev_rec_was_copied = 2;
-#endif
+			prev_rec_is_copied = true;
 
 		} else {
 			/* still on the same page, the next call to
@@ -1034,9 +1064,7 @@ dict_stats_analyze_index_level(
 			instead of copying the records like above */
 
 			prev_rec = rec;
-#ifdef UNIV_DEBUG
-			prev_rec_was_copied = 3;
-#endif
+			prev_rec_is_copied = false;
 		}
 	}
 
@@ -1770,7 +1798,7 @@ dict_stats_analyze_index(
 	keys (on n_prefix columns) is L, we continue from L when
 	searching for D distinct keys on n_prefix-1 columns. */
 	level = root_level;
-	level_is_analyzed = FALSE;
+	level_is_analyzed = false;
 
 	for (n_prefix = n_uniq; n_prefix >= 1; n_prefix--) {
 
@@ -1816,7 +1844,7 @@ dict_stats_analyze_index(
 			      < N_DIFF_REQUIRED(index));
 
 			level--;
-			level_is_analyzed = FALSE;
+			level_is_analyzed = false;
 		}
 
 		/* descend into the tree, searching for "good enough" level */
@@ -1850,7 +1878,7 @@ dict_stats_analyze_index(
 				/* step one level back and be satisfied with
 				whatever it contains */
 				level++;
-				level_is_analyzed = TRUE;
+				level_is_analyzed = true;
 
 				break;
 			}
@@ -1863,7 +1891,7 @@ dict_stats_analyze_index(
 						       n_diff_boundaries,
 						       &mtr);
 
-			level_is_analyzed = TRUE;
+			level_is_analyzed = true;
 
 			if (n_diff_on_level[n_prefix] >= N_DIFF_REQUIRED(index)
 			    || level == 1) {
@@ -1874,7 +1902,7 @@ dict_stats_analyze_index(
 			}
 
 			level--;
-			level_is_analyzed = FALSE;
+			level_is_analyzed = false;
 		}
 found_level:
 
@@ -2376,7 +2404,7 @@ dict_stats_fetch_table_stats_step(
 dict_stats_fetch_index_stats_step(). */
 typedef struct index_fetch_struct {
 	dict_table_t*	table;	/*!< table whose indexes are to be modified */
-	ibool		stats_were_modified; /*!< will be set to TRUE if at
+	bool		stats_were_modified; /*!< will be set to true if at
 				least one index stats were modified */
 } index_fetch_t;
 
@@ -2535,12 +2563,12 @@ dict_stats_fetch_index_stats_step(
 	if (stat_name_len == 4 /* strlen("size") */
 	    && strncasecmp("size", stat_name, stat_name_len) == 0) {
 		index->stat_index_size = (ulint) stat_value;
-		arg->stats_were_modified = TRUE;
+		arg->stats_were_modified = true;
 	} else if (stat_name_len == 12 /* strlen("n_leaf_pages") */
 		   && strncasecmp("n_leaf_pages", stat_name, stat_name_len)
 		   == 0) {
 		index->stat_n_leaf_pages = (ulint) stat_value;
-		arg->stats_were_modified = TRUE;
+		arg->stats_were_modified = true;
 	} else if (stat_name_len > PFX_LEN /* e.g. stat_name=="n_diff_pfx01" */
 		   && strncasecmp(PFX, stat_name, PFX_LEN) == 0) {
 
@@ -2614,7 +2642,7 @@ dict_stats_fetch_index_stats_step(
 			index->stat_n_sample_sizes[n_pfx] = 0;
 		}
 
-		arg->stats_were_modified = TRUE;
+		arg->stats_were_modified = true;
 	} else {
 		/* silently ignore rows with unknown stat_name, the
 		user may have developed her own stats */
@@ -2667,7 +2695,7 @@ dict_stats_fetch_from_ps(
 			       table);
 
 	index_fetch_arg.table = table;
-	index_fetch_arg.stats_were_modified = FALSE;
+	index_fetch_arg.stats_were_modified = false;
 	pars_info_bind_function(pinfo,
 			        "fetch_index_stats_step",
 			        dict_stats_fetch_index_stats_step,
@@ -2764,7 +2792,7 @@ dict_stats_update_for_index(
 
 	if (dict_stats_is_persistent_enabled(index->table)) {
 
-		if (dict_stats_persistent_storage_check(FALSE)) {
+		if (dict_stats_persistent_storage_check(false)) {
 			dict_table_stats_lock(index->table, RW_X_LATCH);
 			dict_stats_analyze_index(index);
 			dict_table_stats_unlock(index->table, RW_X_LATCH);
@@ -2848,7 +2876,7 @@ dict_stats_update(
 		before calling the potentially slow function
 		dict_stats_update_persistent(); that is a
 		prerequisite for dict_stats_save() succeeding */
-		if (dict_stats_persistent_storage_check(FALSE)) {
+		if (dict_stats_persistent_storage_check(false)) {
 
 			dberr_t	err;
 
@@ -2895,7 +2923,7 @@ dict_stats_update(
 
 		if (dict_stats_is_persistent_enabled(table)) {
 
-			if (dict_stats_persistent_storage_check(FALSE)) {
+			if (dict_stats_persistent_storage_check(false)) {
 
 				return(dict_stats_save(table));
 			}
@@ -2928,7 +2956,7 @@ dict_stats_update(
 		persistent stats enabled */
 		ut_a(strchr(table->name, '/') != NULL);
 
-		if (!dict_stats_persistent_storage_check(FALSE)) {
+		if (!dict_stats_persistent_storage_check(false)) {
 			/* persistent statistics storage does not exist
 			or is corrupted, calculate the transient stats */
 

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (vasil.dimov:4017 to 4019) vasil.dimov19 Jun