Post-review fixes for WL#3154 "Enable REPAIR for CSV tables"

mysql-test/r/csv.result:
  update result file
mysql-test/t/csv.test:
  add more tests
storage/csv/ha_tina.cc:
  post-review fixes & cleanup
storage/csv/ha_tina.h:
  post-review fixes

mysql-test/r/csv.result

@@ -5025,6 +5025,67 @@ val
 1
 4
 DROP TABLE test_repair_table3;
+CREATE TABLE test_repair_table4 (
+num int not null,
+magic_no int(4) unsigned zerofill DEFAULT '0000' NOT NULL,
+company_name char(30) DEFAULT '' NOT NULL,
+founded char(4) DEFAULT '' NOT NULL
+) ENGINE = CSV;
+SELECT * FROM test_repair_table4;
+num	magic_no	company_name	founded
+Warnings:
+Error	1194	Table 'test_repair_table4' is marked as crashed and should be repaired
+SELECT * FROM test_repair_table4;
+num	magic_no	company_name	founded
+CHECK TABLE test_repair_table4;
+Table	Op	Msg_type	Msg_text
+test.test_repair_table4	check	status	OK
+INSERT INTO test_repair_table4 VALUES (2,101,'SAP','1972');
+INSERT INTO test_repair_table4 VALUES (1,101,'Microsoft','1978');
+INSERT INTO test_repair_table4 VALUES (2,101,'MySQL','1995');
+SELECT * FROM test_repair_table4;
+num	magic_no	company_name	founded
+2	0101	SAP	1972
+1	0101	Microsoft	1978
+2	0101	MySQL	1995
+CHECK TABLE test_repair_table4;
+Table	Op	Msg_type	Msg_text
+test.test_repair_table4	check	status	OK
+REPAIR TABLE test_repair_table4;
+Table	Op	Msg_type	Msg_text
+test.test_repair_table4	repair	status	OK
+SELECT * FROM test_repair_table4;
+num	magic_no	company_name	founded
+2	0101	SAP	1972
+1	0101	Microsoft	1978
+2	0101	MySQL	1995
+CHECK TABLE test_repair_table4;
+Table	Op	Msg_type	Msg_text
+test.test_repair_table4	check	status	OK
+REPAIR TABLE test_repair_table4;
+Table	Op	Msg_type	Msg_text
+test.test_repair_table4	repair	status	OK
+SELECT * FROM test_repair_table4;
+num	magic_no	company_name	founded
+2	0101	SAP	1972
+1	0101	Microsoft	1978
+2	0101	MySQL	1995
+DROP TABLE test_repair_table4;
+CREATE TABLE test_repair_table5 (
+num int not null,
+magic_no int(4) unsigned zerofill DEFAULT '0000' NOT NULL,
+company_name char(30) DEFAULT '' NOT NULL,
+founded char(4) DEFAULT '' NOT NULL
+) ENGINE = CSV;
+CHECK TABLE test_repair_table5;
+Table	Op	Msg_type	Msg_text
+test.test_repair_table5	check	error	Corrupt
+REPAIR TABLE test_repair_table5;
+Table	Op	Msg_type	Msg_text
+test.test_repair_table5	repair	status	OK
+SELECT * FROM test_repair_table5;
+num	magic_no	company_name	founded
+DROP TABLE test_repair_table5;
 create table t1 (a int) engine=csv;
 insert t1 values (1);
 delete from t1;

mysql-test/t/csv.test

@@ -1425,6 +1425,53 @@ REPAIR TABLE test_repair_table3;
 SELECT * FROM test_repair_table3;
 DROP TABLE test_repair_table3;
 
+# Test with more sophisticated table
+
+CREATE TABLE test_repair_table4 (
+  num int not null,
+  magic_no int(4) unsigned zerofill DEFAULT '0000' NOT NULL,
+  company_name char(30) DEFAULT '' NOT NULL,
+  founded char(4) DEFAULT '' NOT NULL
+) ENGINE = CSV;
+
+--exec rm $MYSQLTEST_VARDIR/master-data/test/test_repair_table4.CSM
+SELECT * FROM test_repair_table4;
+SELECT * FROM test_repair_table4;
+CHECK TABLE test_repair_table4;
+
+INSERT INTO test_repair_table4 VALUES (2,101,'SAP','1972');
+INSERT INTO test_repair_table4 VALUES (1,101,'Microsoft','1978');
+INSERT INTO test_repair_table4 VALUES (2,101,'MySQL','1995');
+
+# list table content
+SELECT * FROM test_repair_table4;
+CHECK TABLE test_repair_table4;
+REPAIR TABLE test_repair_table4;
+# check that nothing changed
+SELECT * FROM test_repair_table4;
+# verify that check/repair did non corrupt the table itself
+CHECK TABLE test_repair_table4;
+REPAIR TABLE test_repair_table4;
+SELECT * FROM test_repair_table4;
+DROP TABLE test_repair_table4;
+
+# Run CHECK/REPAIR on the CSV file with a single row, which misses a column.
+
+CREATE TABLE test_repair_table5 (
+  num int not null,
+  magic_no int(4) unsigned zerofill DEFAULT '0000' NOT NULL,
+  company_name char(30) DEFAULT '' NOT NULL,
+  founded char(4) DEFAULT '' NOT NULL
+) ENGINE = CSV;
+
+# Corrupt a table -- put a file with wrong # of columns
+--exec echo -n -e \"1\",\"101\",\"IBM\"\\n  > $MYSQLTEST_VARDIR/master-data/test/test_repair_table5.CSV
+
+CHECK TABLE test_repair_table5;
+REPAIR TABLE test_repair_table5;
+SELECT * FROM test_repair_table5;
+DROP TABLE test_repair_table5;
+
 
 #
 # BUG#13406 - incorrect amount of "records deleted"

storage/csv/ha_tina.cc

@@ -66,6 +66,10 @@
 #define CSM_EXT ".CSM"               // Meta file
 
 
+static TINA_SHARE *get_share(const char *table_name, TABLE *table);
+static int free_share(TINA_SHARE *share);
+static int read_meta_file(File meta_file, ha_rows *rows);
+static int write_meta_file(File meta_file, ha_rows rows, bool dirty);
 
 /* Stuff for shares */
 pthread_mutex_t tina_mutex;
@@ -208,7 +212,7 @@ static int tina_done_func()
 /*
   Simple lock controls.
 */
-TINA_SHARE *ha_tina::get_share(const char *table_name, TABLE *table, int *rc)
+static TINA_SHARE *get_share(const char *table_name, TABLE *table)
 {
   TINA_SHARE *share;
   char meta_file_name[FN_REFLEN];
@@ -218,7 +222,6 @@ TINA_SHARE *ha_tina::get_share(const char *table_name, TABLE *table, int *rc)
   if (!tina_init)
      tina_init_func();
 
-  *rc= 0;
   pthread_mutex_lock(&tina_mutex);
   length=(uint) strlen(table_name);
 
@@ -236,7 +239,6 @@ TINA_SHARE *ha_tina::get_share(const char *table_name, TABLE *table, int *rc)
                          NullS))
     {
       pthread_mutex_unlock(&tina_mutex);
-      *rc= HA_ERR_OUT_OF_MEM;
       return NULL;
     }
 
@@ -295,8 +297,6 @@ TINA_SHARE *ha_tina::get_share(const char *table_name, TABLE *table, int *rc)
     share->saved_data_file_length= share->file_stat.st_size;
   }
   share->use_count++;
-  if (share->crashed)
-    *rc= HA_ERR_CRASHED_ON_USAGE;
   pthread_mutex_unlock(&tina_mutex);
 
   return share;
@@ -307,8 +307,6 @@ TINA_SHARE *ha_tina::get_share(const char *table_name, TABLE *table, int *rc)
   thr_lock_delete(&share->lock);
   pthread_mutex_destroy(&share->mutex);
 error:
-  if (share->crashed)
-    *rc= HA_ERR_CRASHED_ON_USAGE;
   pthread_mutex_unlock(&tina_mutex);
   my_free((gptr) share, MYF(0));
 
@@ -332,10 +330,10 @@ TINA_SHARE *ha_tina::get_share(const char *table_name, TABLE *table, int *rc)
 
   RETURN
     0 - OK
-    non-zero - error occured
+    non-zero - error occurred
 */
 
-int ha_tina::read_meta_file(File meta_file, ha_rows *rows)
+static int read_meta_file(File meta_file, ha_rows *rows)
 {
   uchar meta_buffer[META_BUFFER_SIZE];
   uchar *ptr= meta_buffer;
@@ -387,10 +385,10 @@ int ha_tina::read_meta_file(File meta_file, ha_rows *rows)
 
   RETURN
     0 - OK
-    non-zero - error occured
+    non-zero - error occurred
 */
 
-int ha_tina::write_meta_file(File meta_file, ha_rows rows, bool dirty)
+static int write_meta_file(File meta_file, ha_rows rows, bool dirty)
 {
   uchar meta_buffer[META_BUFFER_SIZE];
   uchar *ptr= meta_buffer;
@@ -441,7 +439,7 @@ bool ha_tina::is_crashed() const
 /*
   Free lock controls.
 */
-int ha_tina::free_share(TINA_SHARE *share)
+static int free_share(TINA_SHARE *share)
 {
   DBUG_ENTER("ha_tina::free_share");
   pthread_mutex_lock(&tina_mutex);
@@ -569,7 +567,7 @@ int ha_tina::encode_quote(byte *buf)
 
 /*
   chain_append() adds delete positions to the chain that we use to keep
-  track of space. Then the chain will be used to cleanup "holes", occured
+  track of space. Then the chain will be used to cleanup "holes", occurred
   due to deletes and updates.
 */
 int ha_tina::chain_append()
@@ -812,18 +810,15 @@ bool ha_tina::check_if_locking_is_allowed(uint sql_command,
 int ha_tina::open(const char *name, int mode, uint open_options)
 {
   DBUG_ENTER("ha_tina::open");
-  int rc;
 
-  share= get_share(name, table, &rc);
+  if (!(share= get_share(name, table)))
+    DBUG_RETURN(HA_ERR_OUT_OF_MEM);
 
-  if (rc == HA_ERR_CRASHED_ON_USAGE && !(open_options & HA_OPEN_FOR_REPAIR))
+  if (share->crashed && !(open_options & HA_OPEN_FOR_REPAIR))
   {
     free_share(share);
-    DBUG_RETURN(rc);
+    DBUG_RETURN(HA_ERR_CRASHED_ON_USAGE);
   }
-  else
-    if (rc == HA_ERR_OUT_OF_MEM)
-      DBUG_RETURN(rc);
 
   /*
     Init locking. Pass handler object to the locking routines,
@@ -1165,6 +1160,24 @@ int ha_tina::rnd_end()
 }
 
 
+/*
+  Repair CSV table in the case, it is crashed.
+
+  SYNOPSIS
+    repair()
+    thd         The thread, performing repair
+    check_opt   The options for repair. We do not use it currently.
+
+  DESCRIPTION
+    If the file is empty, change # of rows in the file and complete recovery.
+    Otherwise, scan the table looking for bad rows. If none were found,
+    we mark file as a good one and return. If a bad row was encountered,
+    we truncate the datafile up to the last good row.
+
+   TODO: Make repair more clever - it should try to recover subsequent
+         rows (after the first bad one) as well.
+*/
+
 int ha_tina::repair(THD* thd, HA_CHECK_OPT* check_opt)
 {
   char repaired_fname[FN_REFLEN];
@@ -1190,6 +1203,9 @@ int ha_tina::repair(THD* thd, HA_CHECK_OPT* check_opt)
     the log tables). We set it manually here.
   */
   local_saved_data_file_length= share->file_stat.st_size;
+  /* set current position to the beginning of the file */
+  current_position= next_position= 0;
+
   /* Read the file row-by-row. If everything is ok, repair is not needed. */
   while (!(rc= find_current_row(buf)))
   {
@@ -1199,9 +1215,16 @@ int ha_tina::repair(THD* thd, HA_CHECK_OPT* check_opt)
 
   my_free((char*)buf, MYF(0));
 
-  // the file is ok
-  if (rc == HA_ERR_END_OF_FILE && share->rows_recorded == rows_repaired)
+  /* The file is ok */
+  if (rc == HA_ERR_END_OF_FILE)
+  {
+    /*
+      If rows_recorded != rows_repaired, we should update
+      rows_recorded value to the current amount of rows.
+    */
+    share->rows_recorded= rows_repaired;
     goto end;
+  }
 
   /*
     Otherwise we've encountered a bad row => repair is needed.
@@ -1217,6 +1240,7 @@ int ha_tina::repair(THD* thd, HA_CHECK_OPT* check_opt)
                MYF(MY_NABP)))
     DBUG_RETURN(HA_ERR_CRASHED_ON_USAGE);
   my_close(repair_file, MYF(0));
+  /* we just truncated the file up to the first bad row. update rows count. */
   share->rows_recorded= rows_repaired;
 
   if (my_munmap(share->mapped_file, share->file_stat.st_size))
@@ -1330,7 +1354,7 @@ int ha_tina::check(THD* thd, HA_CHECK_OPT* check_opt)
 
   if ((rc != HA_ERR_END_OF_FILE) || count)
   {
-    share->crashed= FALSE;
+    share->crashed= TRUE;
     DBUG_RETURN(HA_ADMIN_CORRUPT);
   }
   else

storage/csv/ha_tina.h

@@ -129,10 +129,6 @@ class ha_tina: public handler
   bool check_and_repair(THD *thd);
   int check(THD* thd, HA_CHECK_OPT* check_opt);
   bool is_crashed() const;
-  int read_meta_file(File meta_file, ha_rows *rows);
-  int write_meta_file(File meta_file, ha_rows rows, bool dirty);
-  TINA_SHARE *get_share(const char *table_name, TABLE *table, int *rc);
-  int free_share(TINA_SHARE *share);
   int rnd_end();
   int repair(THD* thd, HA_CHECK_OPT* check_opt);
   /* This is required for SQL layer to know that we support autorepair */