Optimized GIS functions

heap/hp_delete.c:
  Added comments
mysql-test/r/gis.result:
  Updated results after name changes (all results line are unchanged)
mysql-test/r/show_check.result:
  Update test results after fix in hp_delete.cc
mysql-test/t/gis.test:
  Changed table names to longer, hopefully non conflicting ones.
  Added missing drop table
mysys/hash.c:
  Inendation cleanup
mysys/tree.c:
  Updated comments
  Decrease tree->allocated on delete (for status)
sql/field.cc:
  Added safety checking for GIS objects
sql/gstream.cc:
  Added copyright message
  Made a lot of speed/space optimizations
  Changed class names to be MySQL compliant
sql/gstream.h:
  Made a lot of speed/space optimizations
  Changed class names to be MySQL compliant
sql/item_create.cc:
  Indentation fixup
sql/item_geofunc.cc:
  Use new gis interface functions and new gis class names.
  Simple optimizations
  Indentation fixups
  Fixed a lot of unlikely but possible errors.
sql/item_geofunc.h:
  Moved SRID_SIZE to spatial.h
sql/spatial.cc:
  Added copyright message
  Made a lot of speed/space optimizations
  Changed class names to be MySQL compliant
sql/spatial.h:
  Made a lot of speed/space optimizations
  Changed class names to be MySQL compliant
  Indentation fixes
  Use bool instead of int as result type for functions that only return 0 or 1
sql/sql_string.cc:
  Simple optimizations
sql/sql_string.h:
  Simple cleanups
sql/structs.h:
  Added LEX_STRING_WITH_INIT (needed by spatial.cc)

heap/hp_delete.c

@@ -60,9 +60,11 @@ err:
   DBUG_RETURN(my_errno);
 }
 
+
 /*
-Remove one key from rb-tree
+  Remove one key from rb-tree
 */
+
 int hp_rb_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo,
 		   const byte *record, byte *recpos, int flag)
 {
@@ -82,11 +84,25 @@ int hp_rb_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo,
   return res;
 }
 
-	/* Remove one key from hash-table */
-	/* Flag is set if we want's to correct info->current_ptr */
+
+/*
+  Remove one key from hash-table
+
+  SYNPOSIS
+    hp_delete_key()
+    info		Hash handler
+    keyinfo		key definition of key that we want to delete
+    record		row data to be deleted
+    recpos		Pointer to heap record in memory
+    flag		Is set if we want's to correct info->current_ptr
+
+  RETURN
+    0	ok
+    #	error number
+*/
 
 int hp_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo,
-		   const byte *record, byte *recpos, int flag)
+		  const byte *record, byte *recpos, int flag)
 {
   ulong blength,pos2,pos_hashnr,lastpos_hashnr;
   HASH_INFO *lastpos,*gpos,*pos,*pos3,*empty,*last_ptr;

mysql-test/r/show_check.result

@@ -359,9 +359,9 @@ delete from t2 where b=3;
 delete from t3 where a=3;
 show table status;
 Name	Type	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
-t1	HEAP	Fixed	4	5	39904	249415	105	5	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
+t1	HEAP	Fixed	4	5	39904	249415	89	5	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
 t2	HEAP	Fixed	4	5	39904	249415	39904	5	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
-t3	HEAP	Fixed	4	9	33072	248103	22153	9	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
+t3	HEAP	Fixed	4	9	33072	248103	22137	9	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
 delete from t1;
 delete from t2;
 delete from t3;
@@ -383,7 +383,7 @@ delete from t2 where b=5;
 delete from t3 where a=5;
 show table status;
 Name	Type	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment
-t1	HEAP	Fixed	0	5	39904	249415	21	5	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
+t1	HEAP	Fixed	0	5	39904	249415	5	5	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
 t2	HEAP	Fixed	0	5	39904	249415	39904	5	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
-t3	HEAP	Fixed	0	9	33072	248103	22069	9	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
+t3	HEAP	Fixed	0	9	33072	248103	22053	9	NULL	NULL	NULL	NULL	latin1_swedish_ci	NULL		
 drop table t1, t2, t3;

mysys/hash.c

@@ -215,9 +215,9 @@ static int hashcmp(HASH *hash,HASH_LINK *pos,const byte *key,uint length)
 {
   uint rec_keylength;
   byte *rec_key= (byte*) hash_key(hash,pos->data,&rec_keylength,1);
-  return (length && length != rec_keylength) ||
-     my_strnncoll(hash->charset, (uchar*) rec_key, rec_keylength,
-		  (uchar*) key, length);
+  return ((length && length != rec_keylength) ||
+	  my_strnncoll(hash->charset, (uchar*) rec_key, rec_keylength,
+		       (uchar*) key, length));
 }
 
 

mysys/tree.c

@@ -170,8 +170,8 @@ void delete_tree(TREE* tree)
 
 void reset_tree(TREE* tree)
 {
+  /* do not free mem_root, just mark blocks as free */
   free_tree(tree, MYF(MY_MARK_BLOCKS_FREE));
-  /* do not my_free() mem_root if applicable, just mark blocks as free */
 }
 
 
@@ -188,10 +188,14 @@ static void delete_tree_element(TREE *tree, TREE_ELEMENT *element)
   }
 }
 
-	/* Code for insert, search and delete of elements */
-	/*   parent[0] = & parent[-1][0]->left ||
-	     parent[0] = & parent[-1][0]->right */
 
+/*
+  insert, search and delete of elements
+
+  The following should be true:
+    parent[0] = & parent[-1][0]->left ||
+    parent[0] = & parent[-1][0]->right
+*/
 
 TREE_ELEMENT *tree_insert(TREE *tree, void *key, uint key_size, 
                           void* custom_arg)
@@ -232,8 +236,7 @@ TREE_ELEMENT *tree_insert(TREE *tree, void *key, uint key_size,
     if (tree->with_delete)
       element=(TREE_ELEMENT *) my_malloc(alloc_size, MYF(MY_WME));
     else
-      element=(TREE_ELEMENT *)
-	alloc_root(&tree->mem_root,alloc_size);
+      element=(TREE_ELEMENT *) alloc_root(&tree->mem_root,alloc_size);
     if (!element)
       return(NULL);
     **parent=element;
@@ -251,9 +254,9 @@ TREE_ELEMENT *tree_insert(TREE *tree, void *key, uint key_size,
     }
     else
       memcpy((byte*) element+tree->offset_to_key,key,(size_t) key_size);
-    element->count=1;				/* May give warning in purify */
+    element->count=1;			/* May give warning in purify */
     tree->elements_in_tree++;
-    rb_insert(tree,parent,element);		/* rebalance tree */
+    rb_insert(tree,parent,element);	/* rebalance tree */
   }
   else
   {
@@ -320,6 +323,8 @@ int tree_delete(TREE *tree, void *key, void *custom_arg)
     rb_delete_fixup(tree,parent);
   if (tree->free)
     (*tree->free)(ELEMENT_KEY(tree,element), free_free, tree->custom_arg);
+  /* This doesn't include key_size, but better than nothing */
+  tree->allocated-= sizeof(TREE_ELEMENT)+tree->size_of_element;
   my_free((gptr) element,MYF(0));
   tree->elements_in_tree--;
   return 0;

sql/field.cc

@@ -4715,18 +4715,26 @@ void Field_blob::get_key_image(char *buff,uint length,
 #ifdef HAVE_SPATIAL
   if (type == itMBR)
   {
-    if (!blob_length)
-      return;
-    get_ptr(&blob);
-
+    const char *dummy;
     MBR mbr;
     Geometry gobj;
+
+    if (blob_length < SRID_SIZE)
+    {
+      bzero(buff, SIZEOF_STORED_DOUBLE*4);
+      return;
+    }
+    get_ptr(&blob);
     gobj.create_from_wkb(blob + SRID_SIZE, blob_length - SRID_SIZE);
-    gobj.get_mbr(&mbr);
-    float8store(buff,    mbr.xmin);
-    float8store(buff+8,  mbr.xmax);
-    float8store(buff+16, mbr.ymin);
-    float8store(buff+24, mbr.ymax);
+    if (gobj.get_mbr(&mbr, &dummy))
+      bzero(buff, SIZEOF_STORED_DOUBLE*4);
+    else
+    {
+      float8store(buff,    mbr.xmin);
+      float8store(buff+8,  mbr.xmax);
+      float8store(buff+16, mbr.ymin);
+      float8store(buff+24, mbr.ymax);
+    }
     return;
   }
 #endif /*HAVE_SPATIAL*/
@@ -4939,6 +4947,7 @@ uint Field_blob::max_packed_col_length(uint max_length)
   return (max_length > 255 ? 2 : 1)+max_length;
 }
 
+
 #ifdef HAVE_SPATIAL
 
 void Field_geom::get_key_image(char *buff, uint length, CHARSET_INFO *cs,
@@ -4947,17 +4956,26 @@ void Field_geom::get_key_image(char *buff, uint length, CHARSET_INFO *cs,
   length-= HA_KEY_BLOB_LENGTH;
   ulong blob_length= get_length(ptr);
   char *blob;
-  get_ptr(&blob);
-
+  const char *dummy;
   MBR mbr;
+
+  if (blob_length < SRID_SIZE)
+  {
+    bzero(buff, SIZEOF_STORED_DOUBLE*4);
+    return;
+  }
+  get_ptr(&blob);
   Geometry gobj;
   gobj.create_from_wkb(blob + SRID_SIZE, blob_length - SRID_SIZE);
-  gobj.get_mbr(&mbr);
-  float8store(buff, mbr.xmin);
-  float8store(buff + 8, mbr.xmax);
-  float8store(buff + 16, mbr.ymin);
-  float8store(buff + 24, mbr.ymax);
-  return;
+  if (gobj.get_mbr(&mbr, &dummy))
+    bzero(buff, SIZEOF_STORED_DOUBLE*4);
+  else
+  {
+    float8store(buff, mbr.xmin);
+    float8store(buff + 8, mbr.xmax);
+    float8store(buff + 16, mbr.ymin);
+    float8store(buff + 24, mbr.ymax);
+  }
 }
 
 
@@ -5001,16 +5019,16 @@ void Field_geom::sql_type(String &res) const
 int Field_geom::store(const char *from, uint length, CHARSET_INFO *cs)
 {
   if (!length)
-  {
     bzero(ptr, Field_blob::pack_length());
-  }
   else
   {
-    // Should check given WKB
-    if (length < 4 + 1 + 4 + 8 + 8)		// SRID + WKB_HEADER + X + Y
-      return 1;
-    uint32 wkb_type= uint4korr(from + 5);
-    if (wkb_type < 1 || wkb_type > 7)
+    // Check given WKB
+    uint32 wkb_type;
+    if (length < SRID_SIZE + WKB_HEADER_SIZE + SIZEOF_STORED_DOUBLE*2)
+      goto err;
+    wkb_type= uint4korr(from + WKB_HEADER_SIZE);
+    if (wkb_type < (uint32) Geometry::wkbPoint ||
+	wkb_type > (uint32) Geometry::wkb_end)
       return 1;
     Field_blob::store_length(length);
     if (table->copy_blobs || length <= MAX_FIELD_WIDTH)
@@ -5021,6 +5039,10 @@ int Field_geom::store(const char *from, uint length, CHARSET_INFO *cs)
     bmove(ptr + packlength, (char*) &from, sizeof(char*));
   }
   return 0;
+
+err:
+  bzero(ptr, Field_blob::pack_length());  
+  return 1;
 }
 
 #endif /*HAVE_SPATIAL*/

sql/gstream.cc

+/* Copyright (C) 2004 MySQL AB
+
+   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 Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+
+/*
+  Functions to read and parse geometrical data.
+  NOTE: These functions assumes that the string is end \0 terminated!
+*/
+
 #include "mysql_priv.h"
 
-int GTextReadStream::get_next_toc_type() const
+enum Gis_read_stream::enum_tok_types Gis_read_stream::get_next_toc_type()
 {
-  const char *cur = m_cur;
-  while ((*cur)&&(strchr(" \t\r\n",*cur)))
-  {
-    cur++;
-  }
-  if (!(*cur))
-  {
+  skip_space();
+  if (!*m_cur)
     return eostream;
-  }
-
-  if (((*cur>='a') && (*cur<='z')) || ((*cur>='A') && (*cur<='Z')) ||
-      (*cur=='_'))
-  {
+  if (my_isvar_start(&my_charset_bin, *m_cur))
     return word;
-  }
-
-  if (((*cur>='0') && (*cur<='9')) || (*cur=='-') || (*cur=='+') ||
-      (*cur=='.'))
-  {
+  if ((*m_cur >= '0' && *m_cur <= '9') || *m_cur == '-' || *m_cur == '+')
     return numeric;
-  }
-
-  if (*cur == '(')
-  {
+  if (*m_cur == '(')
     return l_bra;
-  }
-  
-  if (*cur == ')')
-  {
+  if (*m_cur == ')')
     return r_bra;
-  }
-
-  if (*cur == ',')
-  {
+  if (*m_cur == ',')
     return comma;
-  }
-
   return unknown;
 }
 
-const char *GTextReadStream::get_next_word(int *word_len)
-{
-  const char *cur = m_cur;
-  while ((*cur)&&(strchr(" \t\r\n",*cur)))
-  {
-    cur++;
-  }
-  m_last_text_position = cur;
-
-  if (!(*cur))
-  {
-    return 0;
-  }
 
-  const char *wd_start = cur;
-
-  if (((*cur<'a') || (*cur>'z')) && ((*cur<'A') || (*cur>'Z')) && (*cur!='_'))
-  {
-    return NULL;
-  }
+bool Gis_read_stream::get_next_word(LEX_STRING *res)
+{
+  skip_space();
+  res->str= (char*) m_cur;
+  /* The following will also test for \0 */
+  if (!my_isvar_start(&my_charset_bin, *m_cur))
+    return 1;
 
-  ++cur;
+  /*
+    We can't combine the following increment with my_isvar() because
+    my_isvar() is a macro that would cause side effects
+  */
+  m_cur++;
+  while (my_isvar(&my_charset_bin, *m_cur))
+    m_cur++;
 
-  while (((*cur>='a') && (*cur<='z')) || ((*cur>='A') && (*cur<='Z')) ||
-	 (*cur=='_') || ((*cur>='0') && (*cur<='9')))
-  {
-    ++cur;
-  }
+  res->length= (uint32) (m_cur - res->str);
+  return 0;
+}
 
-  *word_len = cur - wd_start;
 
-  m_cur = cur;
+/*
+  Read a floating point number
 
-  return wd_start;
-}
+  NOTE: Number must start with a digit or sign. It can't start with a decimal
+  point
+*/
 
-int GTextReadStream::get_next_number(double *d)
+bool Gis_read_stream::get_next_number(double *d)
 {
-  const char *cur = m_cur;
-  while ((*cur)&&(strchr(" \t\r\n",*cur)))
-  {
-    cur++;
-  }
-
-  m_last_text_position = cur;
-  if (!(*cur))
-  {
-    set_error_msg("Numeric constant expected");
-    return 1;
-  }
+  char *endptr;
 
-  if (((*cur<'0') || (*cur>'9')) && (*cur!='-') && (*cur!='+') && (*cur!='.'))
+  skip_space();
+  /* The following will also test for end \0 */
+  if ((*m_cur < '0' || *m_cur > '9') && *m_cur != '-' && *m_cur != '+')
   {
     set_error_msg("Numeric constant expected");
     return 1;
   }
 
-  char *endptr;
-
-  *d = my_strtod(cur, &endptr);
-
+  *d = my_strtod(m_cur, &endptr);
   if (endptr)
-  {
     m_cur = endptr;
-  }
-
   return 0;
 }
 
-char GTextReadStream::get_next_symbol()
+
+bool Gis_read_stream::check_next_symbol(char symbol)
 {
-  const char *cur = m_cur;
-  while ((*cur)&&(strchr(" \t\r\n",*cur)))
+  skip_space();
+  if (*m_cur != symbol)
   {
-    cur++;
-  }
-  if (!(*cur))
-  {
-    return 0;
+    char buff[32];
+    strmov(buff, "'?' expected");
+    buff[2]= symbol;
+    set_error_msg(buff);
+    return 1;
   }
+  m_cur++;
+  return 0;
+}
 
-  m_cur = cur + 1;
-  m_last_text_position = cur;
 
-  return *cur;
-}
+/*
+  Remember error message.
+*/
 
-void GTextReadStream::set_error_msg(const char *msg)
+void Gis_read_stream::set_error_msg(const char *msg)
 {
-  size_t len = strlen(msg);
-  m_err_msg = (char *)my_realloc(m_err_msg, len + 1, MYF(MY_ALLOW_ZERO_PTR));
+  size_t len= strlen(msg);			// ok in this context
+  m_err_msg= (char *) my_realloc(m_err_msg, len + 1, MYF(MY_ALLOW_ZERO_PTR));
   memcpy(m_err_msg, msg, len + 1);
 }
-
-

sql/gstream.h

@@ -15,10 +15,10 @@
    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
 
 
-class GTextReadStream
+class Gis_read_stream
 {
 public:
-  enum TokTypes
+  enum enum_tok_types
   {
     unknown,
     eostream,
@@ -29,41 +29,47 @@ public:
     comma
   };
 
-  GTextReadStream(const char *buffer, int size)
-    :m_cur(buffer), m_limit(buffer + size), m_last_text_position(buffer),
-    m_err_msg(NULL)
+  Gis_read_stream(const char *buffer, int size)
+    :m_cur(buffer), m_limit(buffer + size), m_err_msg(NULL)
   {}
-  GTextReadStream(): m_cur(NULL), m_limit(NULL), m_err_msg(NULL)
+  Gis_read_stream(): m_cur(NullS), m_limit(NullS), m_err_msg(NullS)
   {}
-
-  ~GTextReadStream()
+  ~Gis_read_stream()
   {
     my_free(m_err_msg, MYF(MY_ALLOW_ZERO_PTR));
   }
 
-  int get_next_toc_type() const;
-  const char *get_next_word(int *word_len);
-  int get_next_number(double *d);
-  char get_next_symbol();
+  enum enum_tok_types get_next_toc_type();
+  bool get_next_word(LEX_STRING *);
+  bool get_next_number(double *);
+  bool check_next_symbol(char);
 
-  const char *get_last_text_position() const
+  inline void skip_space()
   {
-    return m_last_text_position;
+    while (my_isspace(&my_charset_latin1, *m_cur))
+      m_cur++;
+  }
+  /* Skip next character, if match. Return 1 if no match */
+  inline bool skip_char(char skip)
+  {
+    skip_space();
+    if (*m_cur != skip)
+      return 1;					/* Didn't find char */
+    m_cur++;
+    return 0;
   }
-
   void set_error_msg(const char *msg);
 
   // caller should free this pointer
   char *get_error_msg()
   {
     char *err_msg = m_err_msg;
-    m_err_msg = NULL;
+    m_err_msg= NullS;
     return err_msg;
   }
 
 protected:
   const char *m_cur;
   const char *m_limit;
-  const char *m_last_text_position;
   char *m_err_msg;
 };

sql/item_create.cc

@@ -455,6 +455,7 @@ Item *create_func_cast(Item *a, Cast_target cast_type, int len,
 {
   Item *res;
   LINT_INIT(res);
+
   switch (cast_type) {
   case ITEM_CAST_BINARY: 	res= new Item_func_binary(a); break;
   case ITEM_CAST_SIGNED_INT:	res= new Item_func_signed(a); break;

sql/item_geofunc.h

@@ -23,8 +23,6 @@
 #pragma interface			/* gcc class implementation */
 #endif
 
-#define SRID_SIZE sizeof(uint32)
-
 class Item_func_geometry_from_text: public Item_str_func
 {
 public:

sql/sql_string.cc

@@ -126,8 +126,8 @@ bool String::set(double num,uint decimals, CHARSET_INFO *cs)
   str_charset=cs;
   if (decimals >= NOT_FIXED_DEC)
   {
-    sprintf(buff,"%.14g",num);			// Enough for a DATETIME
-    return copy(buff, (uint32) strlen(buff), &my_charset_latin1, cs);
+    uint32 len= my_sprintf(buff,(buff, "%.14g",num));// Enough for a DATETIME
+    return copy(buff, len, &my_charset_latin1, cs);
   }
 #ifdef HAVE_FCONVERT
   int decpt,sign;
@@ -671,9 +671,8 @@ int String::reserve(uint32 space_needed, uint32 grow_by)
   return FALSE;
 }
 
-void String::qs_append(const char *str)
+void String::qs_append(const char *str, uint32 len)
 {
-  int len = strlen(str);
   memcpy(Ptr + str_length, str, len + 1);
   str_length += len;
 }
@@ -681,8 +680,7 @@ void String::qs_append(const char *str)
 void String::qs_append(double d)
 {
   char *buff = Ptr + str_length;
-  sprintf(buff,"%.14g", d);
-  str_length += strlen(buff);
+  str_length+= my_sprintf(buff, (buff, "%.14g", d));
 }
 
 void String::qs_append(double *d)
@@ -692,12 +690,6 @@ void String::qs_append(double *d)
   qs_append(ld);
 }
 
-void String::qs_append(const char &c)
-{
-  Ptr[str_length] = c;
-  str_length += sizeof(c);
-}
-
 
 /*
   Compare strings according to collation, without end space.

sql/sql_string.h

@@ -237,7 +237,7 @@ public:
     q_*** methods writes values of parameters itself
     qs_*** methods writes string representation of value
   */
-  void q_append(const char &c)
+  void q_append(const char c)
   {
     Ptr[str_length++] = c;
   }
@@ -262,15 +262,19 @@ public:
     str_length += data_len;
   }
 
-  void WriteAtPosition(int position, uint32 value)
+  void write_at_position(int position, uint32 value)
   {
     int4store(Ptr + position,value);
   }
 
-  void qs_append(const char *str);
+  void qs_append(const char *str, uint32 len);
   void qs_append(double d);
   void qs_append(double *d);
-  void qs_append(const char &c);
+  inline void qs_append(const char c)
+  {
+     Ptr[str_length]= c;
+     str_length++;
+  }
 
   /* Inline (general) functions used by the protocol functions */
 

sql/structs.h

@@ -20,11 +20,21 @@
 struct st_table;
 class Field;
 
-typedef struct lex_string {
+typedef struct st_lex_string
+{
   char *str;
   uint length;
 } LEX_STRING;
 
+typedef struct st_lex_string_with_init :public st_lex_string
+{
+  st_lex_string_with_init(const char *str_arg, uint length_arg)
+  {
+    str= (char*) str_arg;
+    length= length_arg;
+  }
+} LEX_STRING_WITH_INIT;
+
 
 typedef struct st_date_time_format {
   uchar positions[8];