[sql] Use IGNORE conflict resolution in recovery.

One way databases get corrupted is when pages cross transaction
boundaries.  Both pages can be valid, but they could contain overlapping
data, resulting in UNIQUE constraint violations.  AutoRecoverTable()
handled this by using INSERT OR REPLACE to cause later items to
overwrite earlier items.

Change to INSERT OR IGNORE.  This drops later items for UNIQUE
violations, but also handles other constraint violations.

BUG=none

Review URL: https://codereview.chromium.org/1665913003

Cr-Commit-Position: refs/heads/master@{#373617}

3 files changed, 8 insertions, 16 deletions

diff --git a/sql/recovery.cc b/sql/recovery.cc
index 5c4223c..09e0c37 100644
--- a/sql/recovery.cc
+++ b/sql/recovery.cc
@@ -375,7 +375,6 @@ bool Recovery::AutoRecoverTable(const char* table_name,
   while (s.Step()) {
     const std::string column_name(s.ColumnString(1));
     const std::string column_type(s.ColumnString(2));
-    const bool not_null = s.ColumnBool(3);
     const int default_type = s.ColumnType(4);
     const bool default_is_null = (default_type == COLUMN_TYPE_NULL);
     const int pk_column = s.ColumnInt(5);
@@ -422,16 +421,6 @@ bool Recovery::AutoRecoverTable(const char* table_name,
       return false;
     }
 
-    // If column has constraint "NOT NULL", then inserting NULL into
-    // that column will fail.  If the column has a non-NULL DEFAULT
-    // specified, the INSERT will handle it (see below).  If the
-    // DEFAULT is also NULL, the row must be filtered out.
-    // TODO(shess): The above scenario applies to INSERT OR REPLACE,
-    // whereas INSERT OR IGNORE drops such rows.
-    // http://www.sqlite.org/lang_conflict.html
-    if (not_null && default_is_null)
-      column_decl += " NOT NULL";
-
     create_column_decls.push_back(column_decl);
 
     // Per the NOTE in the header file, convert NULL values to the
@@ -464,8 +453,10 @@ bool Recovery::AutoRecoverTable(const char* table_name,
       table_name,
       base::JoinString(create_column_decls, ",").c_str()));
 
+  // INSERT OR IGNORE means that it will drop rows resulting from constraint
+  // violations.  INSERT OR REPLACE only handles UNIQUE constraint violations.
   std::string recover_insert(base::StringPrintf(
-      "INSERT OR REPLACE INTO main.%s SELECT %s FROM temp.recover_%s",
+      "INSERT OR IGNORE INTO main.%s SELECT %s FROM temp.recover_%s",
       table_name,
       base::JoinString(insert_columns, ",").c_str(),
       table_name));
diff --git a/sql/recovery.h b/sql/recovery.h
index 2c191cf..f690c79 100644
--- a/sql/recovery.h
+++ b/sql/recovery.h
@@ -115,8 +115,8 @@ class SQL_EXPORT Recovery {
   // Attempt to recover the named table from the corrupt database into
   // the recovery database using a temporary recover virtual table.
   // The virtual table schema is derived from the named table's schema
-  // in database [main].  Data is copied using INSERT OR REPLACE, so
-  // duplicates overwrite each other.
+  // in database [main].  Data is copied using INSERT OR IGNORE, so
+  // duplicates are dropped.
   //
   // If the source table has fewer columns than the target, the target
   // DEFAULT value will be used for those columns.
diff --git a/sql/recovery_unittest.cc b/sql/recovery_unittest.cc
index 4990e83..e4fdb7b1 100644
--- a/sql/recovery_unittest.cc
+++ b/sql/recovery_unittest.cc
@@ -371,9 +371,10 @@ TEST_F(SQLRecoveryTest, RecoverCorruptTable) {
   EXPECT_EQ("10", ExecuteWithResults(&db(), kCountSql, "|", ","));
   EXPECT_EQ("10", ExecuteWithResults(&db(), kDistinctSql, "|", ","));
 
-  // The expected value was retained.
+  // Only one of the values is retained.
   const char kSelectSql[] = "SELECT v FROM x WHERE id = 0";
-  EXPECT_EQ("100", ExecuteWithResults(&db(), kSelectSql, "|", ","));
+  const std::string results = ExecuteWithResults(&db(), kSelectSql, "|", ",");
+  EXPECT_TRUE(results=="100" || results=="0") << "Actual results: " << results;
 }
 
 TEST_F(SQLRecoveryTest, Meta) {