summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--components/history/core/browser/thumbnail_database.cc6
-rw-r--r--components/history/core/browser/top_sites_database.cc5
-rw-r--r--sql/recovery.cc9
-rw-r--r--sql/recovery.h9
-rw-r--r--sql/recovery_unittest.cc49
5 files changed, 38 insertions, 40 deletions
diff --git a/components/history/core/browser/thumbnail_database.cc b/components/history/core/browser/thumbnail_database.cc
index c295bb4..8569d64 100644
--- a/components/history/core/browser/thumbnail_database.cc
+++ b/components/history/core/browser/thumbnail_database.cc
@@ -352,18 +352,18 @@ void RecoverDatabaseOrRaze(sql::Connection* db, const base::FilePath& db_path) {
return;
}
- if (!recovery->AutoRecoverTable("favicons", 0, &favicons_rows_recovered)) {
+ if (!recovery->AutoRecoverTable("favicons", &favicons_rows_recovered)) {
sql::Recovery::Rollback(std::move(recovery));
RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICONS);
return;
}
- if (!recovery->AutoRecoverTable("favicon_bitmaps", 0,
+ if (!recovery->AutoRecoverTable("favicon_bitmaps",
&favicon_bitmaps_rows_recovered)) {
sql::Recovery::Rollback(std::move(recovery));
RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICON_BITMAPS);
return;
}
- if (!recovery->AutoRecoverTable("icon_mapping", 0,
+ if (!recovery->AutoRecoverTable("icon_mapping",
&icon_mapping_rows_recovered)) {
sql::Recovery::Rollback(std::move(recovery));
RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_ICON_MAPPING);
diff --git a/components/history/core/browser/top_sites_database.cc b/components/history/core/browser/top_sites_database.cc
index faa61fc..646a85c 100644
--- a/components/history/core/browser/top_sites_database.cc
+++ b/components/history/core/browser/top_sites_database.cc
@@ -277,9 +277,8 @@ void RecoverDatabaseOrRaze(sql::Connection* db, const base::FilePath& db_path) {
return;
}
- // The |1| is because v2 [thumbnails] has one less column than v3 did. In the
- // v2 case the column will get default values.
- if (!recovery->AutoRecoverTable("thumbnails", 1, &thumbnails_recovered)) {
+ // In the v2 case the missing column will get default values.
+ if (!recovery->AutoRecoverTable("thumbnails", &thumbnails_recovered)) {
sql::Recovery::Rollback(std::move(recovery));
RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_THUMBNAILS);
return;
diff --git a/sql/recovery.cc b/sql/recovery.cc
index 5e101b1..5c4223c 100644
--- a/sql/recovery.cc
+++ b/sql/recovery.cc
@@ -349,7 +349,6 @@ void Recovery::Shutdown(Recovery::Disposition raze) {
}
bool Recovery::AutoRecoverTable(const char* table_name,
- size_t extend_columns,
size_t* rows_recovered) {
// Query the info for the recovered table in database [main].
std::string query(
@@ -459,14 +458,6 @@ bool Recovery::AutoRecoverTable(const char* table_name,
if (pk_column_count == 1 && !rowid_decl.empty())
create_column_decls[rowid_ofs] = rowid_decl;
- // Additional columns accept anything.
- // TODO(shess): ignoreN isn't well namespaced. But it will fail to
- // execute in case of conflicts.
- for (size_t i = 0; i < extend_columns; ++i) {
- create_column_decls.push_back(
- base::StringPrintf("ignore%" PRIuS " ANY", i));
- }
-
std::string recover_create(base::StringPrintf(
"CREATE VIRTUAL TABLE temp.recover_%s USING recover(corrupt.%s, %s)",
table_name,
diff --git a/sql/recovery.h b/sql/recovery.h
index c03ebb2..2c191cf 100644
--- a/sql/recovery.h
+++ b/sql/recovery.h
@@ -118,9 +118,8 @@ class SQL_EXPORT Recovery {
// in database [main]. Data is copied using INSERT OR REPLACE, so
// duplicates overwrite each other.
//
- // |extend_columns| allows recovering tables which have excess
- // columns relative to the target schema. The recover virtual table
- // treats more data than specified as a sign of corruption.
+ // If the source table has fewer columns than the target, the target
+ // DEFAULT value will be used for those columns.
//
// Returns true if all operations succeeded, with the number of rows
// recovered in |*rows_recovered|.
@@ -134,9 +133,7 @@ class SQL_EXPORT Recovery {
//
// TODO(shess): Flag for INSERT OR REPLACE vs IGNORE.
// TODO(shess): Handle extended table names.
- bool AutoRecoverTable(const char* table_name,
- size_t extend_columns,
- size_t* rows_recovered);
+ bool AutoRecoverTable(const char* table_name, size_t* rows_recovered);
// Setup a recover virtual table at temp.recover_meta, reading from
// corrupt.meta. Returns true if created.
diff --git a/sql/recovery_unittest.cc b/sql/recovery_unittest.cc
index 11f2a87..73d146f 100644
--- a/sql/recovery_unittest.cc
+++ b/sql/recovery_unittest.cc
@@ -429,7 +429,7 @@ TEST_F(SQLRecoveryTest, AutoRecoverTable) {
ExecuteWithResults(recovery->db(), kTempSchemaSql, "|", "\n"));
size_t rows = 0;
- EXPECT_TRUE(recovery->AutoRecoverTable("x", 0, &rows));
+ EXPECT_TRUE(recovery->AutoRecoverTable("x", &rows));
EXPECT_EQ(2u, rows);
// Test that any additional temp tables were cleaned up.
@@ -452,7 +452,7 @@ TEST_F(SQLRecoveryTest, AutoRecoverTable) {
// TODO(shess): Should this failure implicitly lead to Raze()?
size_t rows = 0;
- EXPECT_FALSE(recovery->AutoRecoverTable("y", 0, &rows));
+ EXPECT_FALSE(recovery->AutoRecoverTable("y", &rows));
sql::Recovery::Unrecoverable(std::move(recovery));
}
@@ -509,7 +509,7 @@ TEST_F(SQLRecoveryTest, AutoRecoverTableWithDefault) {
ASSERT_TRUE(recovery->db()->Execute(final_schema.c_str()));
size_t rows = 0;
- EXPECT_TRUE(recovery->AutoRecoverTable("x", 0, &rows));
+ EXPECT_TRUE(recovery->AutoRecoverTable("x", &rows));
EXPECT_EQ(4u, rows);
ASSERT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
@@ -545,7 +545,7 @@ TEST_F(SQLRecoveryTest, AutoRecoverTableNullFilter) {
ASSERT_TRUE(recovery->db()->Execute(kFinalSchema));
size_t rows = 0;
- EXPECT_TRUE(recovery->AutoRecoverTable("x", 0, &rows));
+ EXPECT_TRUE(recovery->AutoRecoverTable("x", &rows));
EXPECT_EQ(1u, rows);
ASSERT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
@@ -584,7 +584,7 @@ TEST_F(SQLRecoveryTest, AutoRecoverTableWithRowid) {
ASSERT_TRUE(recovery->db()->Execute(kCreateSql));
size_t rows = 0;
- EXPECT_TRUE(recovery->AutoRecoverTable("x", 0, &rows));
+ EXPECT_TRUE(recovery->AutoRecoverTable("x", &rows));
EXPECT_EQ(2u, rows);
ASSERT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
@@ -629,7 +629,7 @@ TEST_F(SQLRecoveryTest, AutoRecoverTableWithCompoundKey) {
ASSERT_TRUE(recovery->db()->Execute(kCreateSql));
size_t rows = 0;
- EXPECT_TRUE(recovery->AutoRecoverTable("x", 0, &rows));
+ EXPECT_TRUE(recovery->AutoRecoverTable("x", &rows));
EXPECT_EQ(3u, rows);
ASSERT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
@@ -642,29 +642,40 @@ TEST_F(SQLRecoveryTest, AutoRecoverTableWithCompoundKey) {
ASSERT_EQ(orig_data, ExecuteWithResults(&db(), kXSql, "|", "\n"));
}
-// Test |extend_columns| support.
-TEST_F(SQLRecoveryTest, AutoRecoverTableExtendColumns) {
+// Test recovering from a table with fewer columns than the target.
+TEST_F(SQLRecoveryTest, AutoRecoverTableMissingColumns) {
const char kCreateSql[] = "CREATE TABLE x (id INTEGER PRIMARY KEY, t0 TEXT)";
+ const char kAlterSql[] = "ALTER TABLE x ADD COLUMN t1 TEXT DEFAULT 't'";
ASSERT_TRUE(db().Execute(kCreateSql));
ASSERT_TRUE(db().Execute("INSERT INTO x VALUES (1, 'This is')"));
ASSERT_TRUE(db().Execute("INSERT INTO x VALUES (2, 'That was')"));
- // Save aside a copy of the original schema and data.
+ // Generate the expected info by faking a table to match what recovery will
+ // create.
const std::string orig_schema(GetSchema(&db()));
const char kXSql[] = "SELECT * FROM x ORDER BY 1";
- const std::string orig_data(ExecuteWithResults(&db(), kXSql, "|", "\n"));
+ std::string expected_schema;
+ std::string expected_data;
+ {
+ ASSERT_TRUE(db().BeginTransaction());
+ ASSERT_TRUE(db().Execute(kAlterSql));
- // Modify the table to add a column, and add data to that column.
- ASSERT_TRUE(db().Execute("ALTER TABLE x ADD COLUMN t1 TEXT"));
- ASSERT_TRUE(db().Execute("UPDATE x SET t1 = 'a test'"));
- ASSERT_NE(orig_schema, GetSchema(&db()));
- ASSERT_NE(orig_data, ExecuteWithResults(&db(), kXSql, "|", "\n"));
+ expected_schema = GetSchema(&db());
+ expected_data = ExecuteWithResults(&db(), kXSql, "|", "\n");
+ db().RollbackTransaction();
+ }
+
+ // Following tests are pointless if the rollback didn't work.
+ ASSERT_EQ(orig_schema, GetSchema(&db()));
+
+ // Recover the previous version of the table into the altered version.
{
scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(&db(), db_path());
ASSERT_TRUE(recovery->db()->Execute(kCreateSql));
+ ASSERT_TRUE(recovery->db()->Execute(kAlterSql));
size_t rows = 0;
- EXPECT_TRUE(recovery->AutoRecoverTable("x", 1, &rows));
+ EXPECT_TRUE(recovery->AutoRecoverTable("x", &rows));
EXPECT_EQ(2u, rows);
ASSERT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
}
@@ -672,8 +683,8 @@ TEST_F(SQLRecoveryTest, AutoRecoverTableExtendColumns) {
// Since the database was not corrupt, the entire schema and all
// data should be recovered.
ASSERT_TRUE(Reopen());
- ASSERT_EQ(orig_schema, GetSchema(&db()));
- ASSERT_EQ(orig_data, ExecuteWithResults(&db(), kXSql, "|", "\n"));
+ ASSERT_EQ(expected_schema, GetSchema(&db()));
+ ASSERT_EQ(expected_data, ExecuteWithResults(&db(), kXSql, "|", "\n"));
}
// Recover a golden file where an interior page has been manually modified so
@@ -697,7 +708,7 @@ TEST_F(SQLRecoveryTest, Bug387868) {
ASSERT_TRUE(recovery->db()->Execute(kCreateSql));
size_t rows = 0;
- EXPECT_TRUE(recovery->AutoRecoverTable("x", 0, &rows));
+ EXPECT_TRUE(recovery->AutoRecoverTable("x", &rows));
EXPECT_EQ(43u, rows);
// Successfully recovered.
generated by cgit v1.1 at 2024-05-16 21:50:08 +0000