#At file:///home/cpowers/work/dev/dev-04/mysql/
2741 Christopher Powers 2009-06-23
Bug #37565 "Crash in Field_blob::pack"
Online ALTER ADD COLUMN creates a new record format that has
more fields than the existing rows.
Previously, the disparity between the old record formats and
the newer, larger record format was handled strictly by
StorageInterface::decodeRecord(), which set the undefined
fields to NULL prior to returning the decoded record to
the server.
This was generally sufficient except when the older records
were updated. Although Table::update() assigned the new format
to the record and Record::setEncodedRecord() resized the field
vector, the new fields in the record were never initialized to NULL.
StorageInterface::encodeRecord() now initializes undefined fields
to NULL prior to storing the encoded record.
modified:
storage/falcon/ha_falcon.cpp
storage/falcon/ha_falcon.h
per-file messages:
storage/falcon/ha_falcon.cpp
StorageInterface::encodeRecord() initializes undefined fields
to NULL.
Created FieldMap class.
storage/falcon/ha_falcon.h
Created FieldMap class to store field mappings
between the server and Falcon.
=== modified file 'storage/falcon/ha_falcon.cpp'
--- a/storage/falcon/ha_falcon.cpp 2009-06-23 13:20:36 +0000
+++ b/storage/falcon/ha_falcon.cpp 2009-06-24 00:34:01 +0000
@@ -1,4 +1,4 @@
-/* Copyright � 2006-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc.
+/* Copyright � 2006-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc.
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
@@ -471,7 +471,7 @@ StorageInterface::StorageInterface(handl
activeBlobs = NULL;
freeBlobs = NULL;
errorText = NULL;
- fieldMap = NULL;
+ fieldMap.storageInterface = this;
indexOrder = false;
if (table_arg)
@@ -483,7 +483,6 @@ StorageInterface::StorageInterface(handl
tableFlags = default_table_flags;
}
-
StorageInterface::~StorageInterface(void)
{
if (storageTable)
@@ -510,7 +509,7 @@ StorageInterface::~StorageInterface(void
storageConnection = NULL;
}
- unmapFields();
+ fieldMap.unmapFields();
}
int StorageInterface::rnd_init(bool scan)
@@ -585,7 +584,7 @@ int StorageInterface::open(const char *n
// Map fields for Falcon record encoding
- mapFields(table);
+ fieldMap.mapFields(table);
// Map server indexes to Falcon internal indexes
@@ -607,7 +606,7 @@ int StorageInterface::close(void)
if (storageTable)
storageTable->clearCurrentIndex();
- unmapFields();
+ fieldMap.unmapFields();
// Temporarily comment out DTrace probes in Falcon, see bug #36403
// FALCON_CLOSE();
@@ -925,7 +924,7 @@ int StorageInterface::create(const char
// Map fields for Falcon record encoding
- mapFields(form);
+ fieldMap.mapFields(form);
// Map server indexes to Falcon indexes
@@ -2585,6 +2584,8 @@ int StorageInterface::addColumn(THD* thd
if ((ret = storageTable->upgrade(gen.getString(), incrementValue)))
return (error(ret));
+ fieldMap.remapFields(alteredTable);
+
return 0;
}
@@ -2977,15 +2978,27 @@ void StorageInterface::encodeRecord(ucha
storageTable->preInsert(&dataStream);
my_ptrdiff_t ptrDiff = buf - table->record[0];
my_bitmap_map *old_map = dbug_tmp_use_all_columns(table, table->read_set);
- FieldFormat *fieldFormat = storageShare->format->format;
- int maxId = storageShare->format->maxId;
+
+ // Default format for the table, possibly newer than the record format
+
+ FieldFormat *tableFormat = storageShare->format->format;
+
+ // Maximum field definitions for the table
- for (int n = 0; n < maxId; ++n, ++fieldFormat)
+ int tableMaxId = storageShare->format->maxId;
+
+ // Maximum field definitions for the current record
+
+ int recordMaxId = (updateFlag ? storageTable->format->maxId : tableMaxId);
+
+ for (int id = 0; id < tableMaxId; ++id, ++tableFormat)
{
- if (fieldFormat->fieldId < 0 || fieldFormat->offset == 0)
+ if (tableFormat->fieldId < 0 || tableFormat->offset == 0)
continue;
- Field *field = fieldMap[fieldFormat->fieldId];
+ // Get the field definition
+
+ Field *field = fieldMap[tableFormat->fieldId];
ASSERT(field);
if (ptrDiff)
@@ -2993,8 +3006,17 @@ void StorageInterface::encodeRecord(ucha
if (updateFlag && !bitmap_is_set(table->write_set, field->field_index))
{
- const unsigned char *p = storageTable->getEncoding(n);
- dataStream.encodeEncoding(p);
+ // Online ALTER ADD COLUMN may have extended the default record format
+ // of the table beyond that of the existing rows. If the current field
+ // is not defined for this record, set the field to NULL.
+
+ if (id >= recordMaxId)
+ dataStream.encodeNull();
+ else
+ {
+ const unsigned char *p = storageTable->getEncoding(id);
+ dataStream.encodeEncoding(p);
+ }
}
else if (field->is_null())
dataStream.encodeNull();
@@ -3180,40 +3202,33 @@ void StorageInterface::decodeRecord(ucha
// Format of this record
- FieldFormat *fieldFormat = storageTable->format->format;
+ FieldFormat *recordFormat = storageTable->format->format;
int maxId = storageTable->format->maxId;
- // Current format for the table, possibly newer than the record format
+ // Default format for the table, possibly newer than the record format
int tableMaxId = storageTable->share->format->maxId;
- FieldFormat *tableFieldFormat = storageTable->share->format->format;
+ FieldFormat *tableFormat = storageTable->share->format->format;
- for (int n = 0; n < tableMaxId; ++n, ++fieldFormat, ++tableFieldFormat)
- {
- // Online ALTER ADD COLUMN creates a new record format for the table
- // that will have more fields than the older formats associated with
- // existing rows.
- //
- // Currently, online ALTER ADD COLUMN only supports nullable columns and
- // no default value. If the format of this record has fewer fields
- // than the default format of the table, then there are no fields to
- // decode beyond maxId, so set them to NULL.
-
- if (n >= maxId)
- {
- Field *newField = fieldMap[tableFieldFormat->fieldId];
- newField->set_null();
- newField->reset();
- continue;
- }
+ // Update the field map if it was changed on another thread
+
+ if (fieldMap.maxFields < tableMaxId)
+ fieldMap.remapFields(table);
- // If the format doesn't have an offset, the field doesn't exist in the record
-
- if (fieldFormat->fieldId < 0 || fieldFormat->offset == 0)
- continue;
-
+ for (int id = 0; id < tableMaxId; ++id, ++recordFormat, ++tableFormat)
+ {
dataStream.decode();
- Field *field = fieldMap[fieldFormat->fieldId];
+
+ Field *field = NULL;
+
+ // Get the field definition id from the format associated with the current record.
+ // If the field is undefined for this record, then get the field id from the
+ // default table format and set the field to NULL.
+
+ if (id < maxId)
+ field = fieldMap[recordFormat->fieldId];
+ else
+ field = fieldMap[tableFormat->fieldId];
// If we don't have a field for the physical field, just skip over it and don't worry
@@ -3950,17 +3965,41 @@ int StorageInterface::recover (handlerto
DBUG_RETURN(count);
}
+FieldMap::FieldMap(StorageInterface *storageInt, TABLE *srvTable)
+ : storageInterface(storageInt)
+{
+ if (!srvTable || !storageInterface)
+ {
+ fields = NULL;
+ maxFields = 0;
+ return;
+ }
+
+ mapFields(srvTable);
+}
+
+FieldMap::~FieldMap()
+{
+ unmapFields();
+}
+
+Field* FieldMap::operator[](int id)
+{
+ return ((fields && id < maxFields) ? fields[id] : NULL);
+}
+
// Build a record field map for use by encode/decodeRecord()
-void StorageInterface::mapFields(TABLE *srvTable)
+void FieldMap::mapFields(TABLE *srvTable)
{
- if (!srvTable)
+ if (!srvTable || !storageInterface)
return;
- maxFields = storageShare->format->maxId;
+ StorageTableShare *storageShare = storageInterface->storageShare;
unmapFields();
- fieldMap = new Field*[maxFields];
- memset(fieldMap, 0, sizeof(fieldMap[0]) * maxFields);
+ maxFields = storageShare->format->maxId;
+ fields = new Field*[maxFields];
+ memset(fields, 0, sizeof(fields[0]) * maxFields);
char nameBuffer[256];
for (uint n = 0; n < srvTable->s->fields; ++n)
@@ -3970,17 +4009,24 @@ void StorageInterface::mapFields(TABLE *
int id = storageShare->getFieldId(nameBuffer);
if (id >= 0)
- fieldMap[id] = field;
+ fields[id] = field;
}
}
-void StorageInterface::unmapFields(void)
+void FieldMap::unmapFields(void)
{
- if (fieldMap)
+ if (fields)
{
- delete []fieldMap;
- fieldMap = NULL;
+ delete[] fields;
+ fields = NULL;
}
+ maxFields = 0;
+}
+
+void FieldMap::remapFields(TABLE *srvTable)
+{
+ unmapFields();
+ mapFields(srvTable);
}
static MYSQL_SYSVAR_STR(serial_log_dir, falcon_serial_log_dir,
=== modified file 'storage/falcon/ha_falcon.h'
--- a/storage/falcon/ha_falcon.h 2009-01-15 20:29:54 +0000
+++ b/storage/falcon/ha_falcon.h 2009-06-24 00:34:01 +0000
@@ -13,6 +13,7 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
+class StorageInterface;
class StorageConnection;
class StorageTable;
class StorageTableShare;
@@ -32,6 +33,22 @@ struct TABLE_SHARE;
class StorageIndexDesc;
struct StorageBlob;
+class FieldMap
+{
+public:
+ FieldMap(void) : storageInterface(NULL), fields(NULL), maxFields(0) {};
+ FieldMap(StorageInterface *storageInt, TABLE *srvTable = NULL);
+ ~FieldMap(void);
+ void mapFields(TABLE *srvTable);
+ void unmapFields(void);
+ void remapFields(TABLE *srvTable);
+ Field* operator[](int id);
+
+ StorageInterface *storageInterface;
+ Field **fields;
+ int maxFields;
+};
+
class StorageInterface : public handler
{
public:
@@ -145,10 +162,10 @@ public:
void decodeRecord(uchar *buf);
void unlockTable(void);
void checkBinLog(void);
- int scanRange(const key_range *startKey,
+ int scanRange(const key_range *startKey,
const key_range *endKey,
bool eqRange);
- int fillMrrBitmap();
+ int fillMrrBitmap();
void mapFields(TABLE *table);
void unmapFields(void);
@@ -194,12 +211,12 @@ public:
StorageConnection* storageConnection;
StorageTable* storageTable;
StorageTableShare* storageShare;
- Field **fieldMap;
+ FieldMap fieldMap;
const char* errorText;
THR_LOCK_DATA lockData; // MySQL lock
THD *mySqlThread;
TABLE_SHARE *share;
- uint recordLength;
+ uint recordLength;
int lastRecord;
int nextRecord;
int indexErrorId;
@@ -207,12 +224,12 @@ public:
int maxFields;
StorageBlob *activeBlobs;
StorageBlob *freeBlobs;
- bool haveStartKey;
- bool haveEndKey;
- bool tableLocked;
- bool tempTable;
- bool lockForUpdate;
- bool indexOrder;
+ bool haveStartKey;
+ bool haveEndKey;
+ bool tableLocked;
+ bool tempTable;
+ bool lockForUpdate;
+ bool indexOrder;
key_range startKey;
key_range endKey;
uint64 insertCount;
| Thread |
|---|
| • bzr commit into mysql-6.0-falcon-team branch (christopher.powers:2741)Bug#37565 | Christopher Powers | 24 Jun |
