[sql] Verify control of close-time WAL checkpoint.

sql/sqlite_features_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <string>
 
 #include "base/bind.h"
@@ skipping 14 matching lines @@
 #if defined(OS_MACOSX) && !defined(OS_IOS)
 #include <CoreFoundation/CoreFoundation.h>
 #include <CoreServices/CoreServices.h>
 
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #endif
 
 // Test that certain features are/are-not enabled in our SQLite.
 
+namespace sql {
 namespace {
 
+using sql::test::ExecuteWithResult;
+using sql::test::ExecuteWithResults;
+
 void CaptureErrorCallback(int* error_pointer, std::string* sql_text,
                           int error, sql::Statement* stmt) {
   *error_pointer = error;
   const char* text = stmt ? stmt->GetSQLStatement() : NULL;
   *sql_text = text ? text : "no statement available";
 }
 
+}  // namespace
+
 class SQLiteFeaturesTest : public sql::SQLTestBase {
  public:
   SQLiteFeaturesTest() : error_(SQLITE_OK) {}
 
   void SetUp() override {
     SQLTestBase::SetUp();
 
     // The error delegate will set |error_| and |sql_text_| when any sqlite
     // statement operation returns an error code.
     db().set_error_callback(
@@ skipping 41 matching lines @@
 // Originally history used fts2, which Chromium patched to treat "foo*" as a
 // prefix search, though the icu tokenizer would return it as two tokens {"foo",
 // "*"}.  Test that fts3 works correctly.
 TEST_F(SQLiteFeaturesTest, FTS3_Prefix) {
   const char kCreateSql[] =
       "CREATE VIRTUAL TABLE foo USING fts3(x, tokenize icu)";
   ASSERT_TRUE(db().Execute(kCreateSql));
 
   ASSERT_TRUE(db().Execute("INSERT INTO foo (x) VALUES ('test')"));
 
-  sql::Statement s(db().GetUniqueStatement(
-      "SELECT x FROM foo WHERE x MATCH 'te*'"));
-  ASSERT_TRUE(s.Step());
-  EXPECT_EQ("test", s.ColumnString(0));
+  EXPECT_EQ("test",
+            ExecuteWithResult(&db(), "SELECT x FROM foo WHERE x MATCH 'te*'"));
 }
 #endif
 
 #if !defined(USE_SYSTEM_SQLITE)
 // Verify that Chromium's SQLite is compiled with HAVE_USLEEP defined.  With
 // HAVE_USLEEP, SQLite uses usleep() with millisecond granularity.  Otherwise it
 // uses sleep() with second granularity.
 TEST_F(SQLiteFeaturesTest, UsesUsleep) {
   base::TimeTicks before = base::TimeTicks::Now();
   sqlite3_sleep(1);
   base::TimeDelta delta = base::TimeTicks::Now() - before;
 
-  // It is not impossible for this to be over 1000 if things are compiled the
-  // right way.  But it is very unlikely, most platforms seem to be around
-  // <TBD>.
-  LOG(ERROR) << "Milliseconds: " << delta.InMilliseconds();
+  // It is not impossible for this to be over 1000 if things are compiled
+  // correctly, but that is very unlikely.  Most platforms seem to be exactly
+  // 1ms, with the rest at 2ms, and the worst observed cases was ASAN at 7ms.
   EXPECT_LT(delta.InMilliseconds(), 1000);
 }
 #endif
 
 // Ensure that our SQLite version has working foreign key support with cascade
 // delete support.
 TEST_F(SQLiteFeaturesTest, ForeignKeySupport) {
   ASSERT_TRUE(db().Execute("PRAGMA foreign_keys=1"));
   ASSERT_TRUE(db().Execute("CREATE TABLE parents (id INTEGER PRIMARY KEY)"));
   ASSERT_TRUE(db().Execute(
       "CREATE TABLE children ("
       "    id INTEGER PRIMARY KEY,"
       "    pid INTEGER NOT NULL REFERENCES parents(id) ON DELETE CASCADE)"));
+  const char kSelectParents[] = "SELECT * FROM parents ORDER BY id";
+  const char kSelectChildren[] = "SELECT * FROM children ORDER BY id";
 
   // Inserting without a matching parent should fail with constraint violation.
   // Mask off any extended error codes for USE_SYSTEM_SQLITE.
-  int insertErr = db().ExecuteAndReturnErrorCode(
-      "INSERT INTO children VALUES (10, 1)");
-  EXPECT_EQ(SQLITE_CONSTRAINT, (insertErr&0xff));
-
-  size_t rows;
-  EXPECT_TRUE(sql::test::CountTableRows(&db(), "children", &rows));
-  EXPECT_EQ(0u, rows);
+  EXPECT_EQ("", ExecuteWithResult(&db(), kSelectParents));
+  const int insert_error =
+      db().ExecuteAndReturnErrorCode("INSERT INTO children VALUES (10, 1)");
+  EXPECT_EQ(SQLITE_CONSTRAINT, (insert_error & 0xff));
+  EXPECT_EQ("", ExecuteWithResult(&db(), kSelectChildren));
 
   // Inserting with a matching parent should work.
   ASSERT_TRUE(db().Execute("INSERT INTO parents VALUES (1)"));
+  EXPECT_EQ("1", ExecuteWithResults(&db(), kSelectParents, "|", "\n"));
   EXPECT_TRUE(db().Execute("INSERT INTO children VALUES (11, 1)"));
   EXPECT_TRUE(db().Execute("INSERT INTO children VALUES (12, 1)"));
-  EXPECT_TRUE(sql::test::CountTableRows(&db(), "children", &rows));
-  EXPECT_EQ(2u, rows);
+  EXPECT_EQ("11|1\n12|1",
+            ExecuteWithResults(&db(), kSelectChildren, "|", "\n"));
 
-  // Deleting the parent should cascade, i.e., delete the children as well.
+  // Deleting the parent should cascade, deleting the children as well.
   ASSERT_TRUE(db().Execute("DELETE FROM parents"));
-  EXPECT_TRUE(sql::test::CountTableRows(&db(), "children", &rows));
-  EXPECT_EQ(0u, rows);
+  EXPECT_EQ("", ExecuteWithResult(&db(), kSelectParents));
+  EXPECT_EQ("", ExecuteWithResult(&db(), kSelectChildren));
 }
 
 #if defined(MOJO_APPTEST_IMPL) || defined(OS_IOS)
 // If the platform cannot support SQLite mmap'ed I/O, make sure SQLite isn't
 // offering to support it.
 TEST_F(SQLiteFeaturesTest, NoMmap) {
 #if defined(OS_IOS) && defined(USE_SYSTEM_SQLITE)
   if (base::ios::IsRunningOnIOS10OrLater()) {
     // iOS 10 added mmap support for sqlite.
     return;
@@ skipping 276 matching lines @@
     s.Reset(true);
     ASSERT_TRUE(s.Run());
 
     ASSERT_TRUE(base::GetFileSize(db_path(), &file_size_after));
     ASSERT_EQ(file_size_after, file_size_before - 4096);
   }
 #endif
 }
 #endif  // !defined(USE_SYSTEM_SQLITE)
 
-}  // namespace
+#if !defined(USE_SYSTEM_SQLITE)

[pwnall, 2017/04/19 20:02:23]

Would tests fail anywhere if we removed this #if?

I'd imagine we're OK with tests failing on people's custom builds if their
system libraries don't implement everything we need.

[Scott Hess - ex-Googler, 2017/04/19 20:15:33]

iOS uses the system SQLite.  Also maybe Linux distros bundling Chromium, but I'm
not sure that works any longer.

Perhaps a different question is whether this feature is relevant for iOS, given
that we generally don't actually shutdown in a way that makes this relevant. 
But here I'm trying to make assertions about features for Chromium proper to
depend on, so it's not in the regular shutdown context, so that may also not
really be relevant.

[pwnall, 2017/04/19 20:56:54]

TL;DR: If the test passes on bots, I'd leave it on.

Our tests don't have to pass on Linux distros that replace our libraries with
arbitrary versions. That was the case I was thinking of. In that case, I claim
that it's better to have the test fail, so the distributor knows all bets are
off.

Is the system SQLite on our iOS bots old enough / built with a config that
doesn't support the feature? If newer iOS has WAL, I'd leave the test in. If
not, I'd be tempted to have the test skipped for iOS, with a comment explaining
the situation.

[Scott Hess - ex-Googler, 2017/04/19 22:03:16]

The SQLite change landed last fall, so it's pretty new.  iOS has pretty good
uptake when new OS versions come out, but I expect that older versions of iOS
which are still supported by Chrome will not bump their SQLite version.

I haven't done the research to figure out the exact SQLite version the change
landed in, but based on
https://github.com/yapstudios/YapDatabase/wiki/SQLite-version-(bundled-with-OS)
I suspect no iOS versions of SQLite with the change being tested.

+// SQLite WAL mode defaults to checkpointing the WAL on close.  This would push
+// additional work into Chromium shutdown.  Verify that SQLite supports a config
+// option to not checkpoint on close.
+TEST_F(SQLiteFeaturesTest, WALNoClose) {

[Scott Hess - ex-Googler, 2017/04/18 23:53:39]

Um, this is the main test for whether the desired feature is present.  The other
changes above are mostly "Now that I've imported the ExecuteWithResult() helper,
what else would I change?"

+  base::FilePath wal_path(db_path().value() + FILE_PATH_LITERAL("-wal"));
+
+  // Turn on WAL mode, then verify that the mode changed (WAL is supported).
+  ASSERT_TRUE(db().Execute("PRAGMA journal_mode = WAL"));
+  ASSERT_EQ("wal", ExecuteWithResult(&db(), "PRAGMA journal_mode"));
+
+  // The WAL file is created lazily on first change.
+  ASSERT_TRUE(db().Execute("CREATE TABLE foo (a, b)"));
+
+  // By default, the WAL is checkpointed then deleted on close.
+  ASSERT_TRUE(GetPathExists(wal_path));
+  db().Close();
+  ASSERT_FALSE(GetPathExists(wal_path));
+
+  // Reopen and configure the database to not checkpoint WAL on close.
+  ASSERT_TRUE(Reopen());
+  ASSERT_TRUE(db().Execute("PRAGMA journal_mode = WAL"));
+  ASSERT_TRUE(db().Execute("ALTER TABLE foo ADD COLUMN c"));
+  ASSERT_EQ(
+      SQLITE_OK,
+      sqlite3_db_config(db().db_, SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE, 1, NULL));
+  ASSERT_TRUE(GetPathExists(wal_path));
+  db().Close();
+  ASSERT_TRUE(GetPathExists(wal_path));
+}
+#endif
+
+}  // namespace sql