Re: bzr commit into mysql-6.0-falcon-team branch (lars-erik.bjork:2901) Bug#40158

[Olav Sandstaa]

Lars-Erik,

I have only looked at the changes to the code since Hakan has commented 
on the test case. The change seems to implement the intended change to 
handling quotes. OK to push.

Olav

Lars-Erik.Bjork@stripped wrote:
> #At file:///home/lb200670/devel/mysql/nanna21/
>
>  2901 lars-erik.bjork@stripped	2008-11-05
>       This is a commit for bug#40158 
>       (Falcon assertion in StorageInterface::encodeRecord() line 2635 on CREATE
> TABLE)
>       
>       The nature of this bug is explained earlier. 
>       
>       This patch adds a bool argument to StorageTableShare::cleanupFieldName
>       telling if the function should double the number of quotes or not. So
>       far, StorageInterface::mapFields, is the only place this method is
>       called, passing 'false'. All other places it is called passing
>       'true'. This will ensure that the parser still gets the expected
>       number of quotes and we are able to look up the fields correctly.
>       
>       I have also added a regression test for this bug, creating - and
>       inserting into - a table with a field name containing quotes.
>       
>       
>       added mysql-test/suite/falcon/r/falcon_bug_40158.result
>       -------------------------------------------------------
>       Result file for the regression test
>       
>       
>       
>       added mysql-test/suite/falcon/t/falcon_bug_40158.test
>       -----------------------------------------------------
>       * Regression test, testing differently quoted field names
>       
>       modified storage/falcon/StorageTableShare.cpp
>       ---------------------------------------------
>       * Modified StorageTableShare::cleanupFieldName to handle the new
>         argument.
>       
>       modified storage/falcon/StorageTableShare.h
>       -------------------------------------------
>       * Added  new argument to the signature of
>         StorageTableShare::cleanupFieldName
>       * Made sure that all usages of StorageTableShare::cleanupFieldName
>         passes the new argument
>       
>       modified storage/falcon/ha_falcon.cpp
>       -------------------------------------
>       * Made sure that all usages of StorageTableShare::cleanupFieldName
>         passes the new argument
> added:
>   mysql-test/suite/falcon/r/falcon_bug_40158.result
>   mysql-test/suite/falcon/t/falcon_bug_40158.test
> modified:
>   storage/falcon/StorageTableShare.cpp
>   storage/falcon/StorageTableShare.h
>   storage/falcon/ha_falcon.cpp
>
> === added file 'mysql-test/suite/falcon/r/falcon_bug_40158.result'
> --- a/mysql-test/suite/falcon/r/falcon_bug_40158.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/falcon/r/falcon_bug_40158.result	2008-11-05 12:48:46 +0000
> @@ -0,0 +1,25 @@
> +*** Bug #40158 ***
> +SET @@storage_engine = 'Falcon';
> +DROP TABLE IF EXISTS t1;
> +CREATE TABLE t1 (`"strangename"` int);
> +INSERT INTO t1 VALUES (1);
> +SELECT * FROM t1;
> +"strangename"
> +1
> +DROP TABLE t1;
> +SET LOCAL SQL_MODE=ANSI_QUOTES;
> +CREATE TABLE t1 ("""strangename""" int);
> +INSERT INTO t1 VALUES (1);
> +SELECT * FROM t1;
> +"strangename"
> +1
> +DROP TABLE t1;
> +CREATE TABLE t1 (`""strangename""` int);
> +INSERT INTO t1 VALUES (1);
> +SELECT * FROM t1;
> +""strangename""
> +1
> +SELECT COUNT(*) FROM t1;
> +COUNT(*)
> +1
> +DROP TABLE t1;
>
> === added file 'mysql-test/suite/falcon/t/falcon_bug_40158.test'
> --- a/mysql-test/suite/falcon/t/falcon_bug_40158.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/falcon/t/falcon_bug_40158.test	2008-11-05 12:48:46 +0000
> @@ -0,0 +1,47 @@
> +--source include/have_falcon.inc
> +
> +#
> +# Bug #40158: Falcon assertion in StorageInterface::encodeRecord() line 2635 on
> CREATE TABLE
> +#
> +--echo *** Bug #40158 ***
> +
> +# ----------------------------------------------------- #
> +# --- Initialisation                                --- #
> +# ----------------------------------------------------- #
> +let $engine = 'Falcon';
> +eval SET @@storage_engine = $engine;
> +
> +--disable_warnings
> +DROP TABLE IF EXISTS t1;
> +--enable_warnings
> +
> +# ----------------------------------------------------- #
> +# --- Test                                          --- #
> +# ----------------------------------------------------- #
> +
> +CREATE TABLE t1 (`"strangename"` int);
> +INSERT INTO t1 VALUES (1);
> +SELECT * FROM t1;
> +DROP TABLE t1;
> +
> +SET LOCAL SQL_MODE=ANSI_QUOTES;
> +
> +CREATE TABLE t1 ("""strangename""" int);
> +INSERT INTO t1 VALUES (1);
> +SELECT * FROM t1;
> +DROP TABLE t1;
> +
> +CREATE TABLE t1 (`""strangename""` int);
> +INSERT INTO t1 VALUES (1);
> +SELECT * FROM t1;
> +
> +# ----------------------------------------------------- #
> +# --- Check                                         --- #
> +# ----------------------------------------------------- #
> +
> +SELECT COUNT(*) FROM t1;
> +
> +# ----------------------------------------------------- #
> +# --- Final cleanup                                 --- #
> +# ----------------------------------------------------- #
> +DROP TABLE t1;
>
> === modified file 'storage/falcon/StorageTableShare.cpp'
> --- a/storage/falcon/StorageTableShare.cpp	2008-10-22 20:44:09 +0000
> +++ b/storage/falcon/StorageTableShare.cpp	2008-11-05 12:48:46 +0000
> @@ -262,7 +262,7 @@ int StorageTableShare::truncateTable(Sto
>  	return res;
>  }
>  
> -const char* StorageTableShare::cleanupFieldName(const char* name, char* buffer, int
> bufferLength)
> +const char* StorageTableShare::cleanupFieldName(const char* name, char* buffer, int
> bufferLength, bool doubleQuotes)
>  {
>  	char *q = buffer;
>  	char *end = buffer + bufferLength - 1;
> @@ -273,7 +273,11 @@ const char* StorageTableShare::cleanupFi
>  		{
>  		if (*p == '"')
>  			{
> -			*q++ = UPPER(*p);
> +			if (doubleQuotes)
> +				{
> +				*q++ = UPPER(*p);
> +				}
> +
>  			quotes = !quotes;
>  			}
>  	
> @@ -321,7 +325,7 @@ char* StorageTableShare::createIndexName
>  	else
>  		{
>  		char nameBuffer[indexNameSize];
> -		cleanupFieldName(rawName, nameBuffer, sizeof(nameBuffer));
> +		cleanupFieldName(rawName, nameBuffer, sizeof(nameBuffer), true);
>  		sprintf(indexName, "%s$%s", name.getString(), nameBuffer);
>  		}
>  	
>
> === modified file 'storage/falcon/StorageTableShare.h'
> --- a/storage/falcon/StorageTableShare.h	2008-10-22 20:44:09 +0000
> +++ b/storage/falcon/StorageTableShare.h	2008-11-05 12:48:46 +0000
> @@ -123,7 +123,7 @@ public:
>  	virtual INT64		getSequenceValue(int delta);
>  	virtual int			setSequenceValue(INT64 value);
>  	virtual int			haveIndexes(int indexCount);
> -	virtual const char*	cleanupFieldName(const char* name, char* buffer, int
> bufferLength);
> +	virtual const char*	cleanupFieldName(const char* name, char* buffer, int
> bufferLength, bool doubleQuotes);
>  	virtual void		setTablePath(const char* path, bool tempTable);
>  	virtual void		registerCollation(const char* collationName, void* arg);
>  
>
> === modified file 'storage/falcon/ha_falcon.cpp'
> --- a/storage/falcon/ha_falcon.cpp	2008-10-31 15:49:48 +0000
> +++ b/storage/falcon/ha_falcon.cpp	2008-11-05 12:48:46 +0000
> @@ -2486,7 +2486,7 @@ int StorageInterface::genTable(TABLE* ta
>  		if (charset)
>  			storageShare->registerCollation(charset->name, charset);
>  
> -		storageShare->cleanupFieldName(field->field_name, nameBuffer,
> sizeof(nameBuffer));
> +		storageShare->cleanupFieldName(field->field_name, nameBuffer,
> sizeof(nameBuffer), true);
>  		gen->gen("%s  \"%s\" ", sep, nameBuffer);
>  		int ret = genType(field, gen);
>  
> @@ -2632,7 +2632,7 @@ void StorageInterface::genKeyFields(KEY*
>  		{
>  		KEY_PART_INFO *part = key->key_part + n;
>  		Field *field = part->field;
> -		storageShare->cleanupFieldName(field->field_name, nameBuffer,
> sizeof(nameBuffer));
> +		storageShare->cleanupFieldName(field->field_name, nameBuffer,
> sizeof(nameBuffer), true);
>  
>  		if (part->key_part_flag & HA_PART_KEY_SEG)
>  			gen->gen("%s\"%s\"(%d)", sep, nameBuffer, part->length);
> @@ -3557,7 +3557,7 @@ void StorageInterface::mapFields(TABLE *
>  	for (uint n = 0; n < table->s->fields; ++n)
>  		{
>  		Field *field = table->field[n];
> -		storageShare->cleanupFieldName(field->field_name, nameBuffer,
> sizeof(nameBuffer));
> +		storageShare->cleanupFieldName(field->field_name, nameBuffer,
> sizeof(nameBuffer), false);
>  		int id = storageShare->getFieldId(nameBuffer);
>  		
>  		if (id >= 0)
>
>
>