transaction log incorrectly records NULL INSERT into an ENUM as the first permissible value

[Patrick Crews]

The transaction log is incorrectly registering a NULL insert into an ENUM column as the first permissible value:

test case: Run with ./test-run --start-and-exit --mysqld=--transaction_log_enable
DROP TABLE IF EXISTS t1;

CREATE TABLE t1 (a INT NOT NULL, b ENUM ('1','2'), PRIMARY KEY(a));

INSERT INTO t1 VALUES (1,NULL);

# This will let us see what the trans_log has for the last registered transaction
SELECT PRINT_TRANSACTION_MESSAGE('transaction.log',(select max(entry_offset) from DATA_DICTIONARY.TRANSACTION_LOG_TRANSACTIONS));

Results in:
| transaction_context {
  server_id: 1
  transaction_id: 60354
  start_timestamp: 1276642396658190
  end_timestamp: 1276642396700337
}
statement {
  type: INSERT
  start_timestamp: 1276642396658192
  end_timestamp: 1276642396700336
  insert_header {
    table_metadata {
      schema_name: "test"
      table_name: "t1"
    }
    field_metadata {
      type: INTEGER
      name: "a"
    }
    field_metadata {
      type: ENUM
      name: "b"
    }
  }
  insert_data {
    segment_id: 1
    end_segment: true
    record {
      insert_value: "1"
      insert_value: "1" <--- Not NULL
    }
  }
}
 |

[Patrick Crews (patrick-crews)]

NOTE:
This bug is not present in lp:~/mordred/drizzle/publisher

It only appeared when I merged that branch with trunk.

[Joe Daly (skinny.moey)]

This looks to be happening now and not before, because of the enum rework in val_int() adding the 1 to the return value.

int64_t Field_enum::val_int(void)
{
  ASSERT_COLUMN_MARKED_FOR_READ;

  switch (packlength) {
  case 1:
    return ((int64_t) ptr[0]) + 1; /* SQL is from 1, we store from 0 */
  case 2:
  {
    uint16_t tmp;
#ifdef WORDS_BIGENDIAN
    if (getTable()->s->db_low_byte_first)
      tmp=sint2korr(ptr);
    else
#endif
      shortget(tmp,ptr);
    return ((int64_t) tmp) + 1; /* SQL is from 1, we store from 0 */
  }
  default:
    assert(packlength <= 2);
  }
  return 0; // impossible
}

I think the correct fix for this probably is to rework the null handling in transaction_services.cc (again) from:

  while ((current_field= *table_fields++) != NULL)
  {
    string_value= current_field->val_str(string_value);
    record->add_is_null(current_field->is_null());
    record->add_insert_value(string_value->c_ptr(), string_value->length());
    string_value->free();
  }

to:

  while ((current_field= *table_fields++) != NULL)
  {
    if (current_field->is_null())
    {
      record->add_is_null(true);
      record->add_insert_value(NULL, 0);
    }
    else
    {
      string_value= current_field->val_str(string_value);
      record->add_is_null(false);
      record->add_insert_value(string_value->c_ptr(), string_value->length());
      string_value->free();
    }
  }