2

I have a requirement to save an Order Item attribute as a string up to 400 char. While it could be implemented using a text attribute, I would rather use a varchar(400). However, the _addFlatAttribute() method in Mage_Sales_Model_Resource_Setup hardcodes the length of varchar to 255. It could be changed after the fact by a setup script with DDL, but I'm wondering if there are likely to be downstream dependencies on VARCHAR being 255 char.

Any thoughts?

Jonathan Day
  • 18,519
  • 10
  • 84
  • 137

3 Answers3

2

I believe that you will be fine & well-advised to do this given the ever-expanding size of order entity data in an instance. I'm curious what version of Magento though, as CE1.6+ is the first release to have the Mage_Sales_Model_Resource_Setup [link] class. Prior to CE1.6 the class was Mage_Sales_Model_Mysql4_Setup [link]. This is an important distinction because the column definition method in these classes differs to accomodate the DB-agnostic approach in 1.6+

Mage_Sales_Model_Mysql4_Setup::_getAttributeColumnDefinition [link]:

protected function _getAttributeColumnDefinition($code, $data)
{
    $columnDefinition = '';
    $type   = isset($data['type']) ? $data['type'] : 'varchar';
    $req    = isset($data['required']) ? $data['required'] : false;

    switch ($type) {
        case 'int':
            $columnDefinition = 'int(10) unsigned';
            break;
        case 'decimal':
            $columnDefinition = 'decimal(12,4)';
            break;
        case 'text':
            $columnDefinition = 'text';
            break;
        case 'date':
            $columnDefinition = 'datetime';
            break;
        default:
            $columnDefinition = 'varchar(255)';
            break;
    }

    if ($req) {
        $columnDefinition.= ' NOT NULL';
    }
    return $columnDefinition;
}

And Mage_Sales_Model_Resource_Setup::_getAttributeColumnDefinition [link]:

protected function _getAttributeColumnDefinition($code, $data)
{
    // Convert attribute type to column info
    $data['type'] = isset($data['type']) ? $data['type'] : 'varchar';
    $type = null;
    $length = null;
    switch ($data['type']) {
        case 'timestamp':
            $type = Varien_Db_Ddl_Table::TYPE_TIMESTAMP;
            break;
        case 'datetime':
            $type = Varien_Db_Ddl_Table::TYPE_DATETIME;
            break;
        case 'decimal':
            $type = Varien_Db_Ddl_Table::TYPE_DECIMAL;
            $length = '12,4';
            break;
        case 'int':
            $type = Varien_Db_Ddl_Table::TYPE_INTEGER;
            break;
        case 'text':
            $type = Varien_Db_Ddl_Table::TYPE_TEXT;
            $length = 65536;
            break;
        case 'char':
        case 'varchar':
            $type = Varien_Db_Ddl_Table::TYPE_TEXT;
            $length = 255;
            break;
    }
    if ($type !== null) {
        $data['type'] = $type;
        $data['length'] = $length;
    }

    $data['nullable'] = isset($data['required']) ? !$data['required'] : true;
    $data['comment']  = isset($data['comment']) ? $data['comment'] : ucwords(str_replace('_', ' ', $code));
    return $data;
}
benmarks
  • 23,384
  • 1
  • 62
  • 84
  • Thanks Ben, this is in 1.12 (EE) - separate development to the module dependency question :) I do think its a good idea to restrict the field to 400 char rather than allow the 65k of TEXT due to the row limit (which I think is what you are referring to). – Jonathan Day Dec 01 '12 at 12:24
  • 1
    AH, I was getting hung up on "hardcodes the length of **varchar**" - as you can see above, it should be TEXT. That one cannot specify the length of the field (without class rewrite) in this context is a pity, but your solution to update that column after added should work. However, note that if you take this approach you wil be altering the table twice which may be an issue on arbitrarily large tables - often the case with sales entity tables. – benmarks Dec 01 '12 at 12:33
  • excellent point about the double-alter - means having the tables locked for a loooooooong time. – Jonathan Day Dec 01 '12 at 12:39
1

I can't imagine so, it might be different if you were to decrease the size of varchar, though.

See also: Is there a good reason I see VARCHAR(255) used so often (as opposed to another length)?

Community
  • 1
  • 1
Nathan
  • 1,445
  • 9
  • 20
  • Thanks Nathan, I agree it should be fine on the MySQL side, its more about the Magento side that I'm concerned... – Jonathan Day Nov 30 '12 at 01:39
  • 1
    Some of the PHP pages in the front-end have validation limiting certain inputs to 255 characters, but what you're changing shouldn't affect those as you are adding your own attribute. Make sure you have the appropriate validation in your PHP pages too, though. – Nathan Nov 30 '12 at 05:30
1

Use type TEXT instead of VARCHAR. Mage_Sales_Model_Resource_Setup assumes a length of 64KB in this case.

clockworkgeek
  • 37,650
  • 9
  • 89
  • 127