Adds the result of `Database::GetErrorMessage()` to the diagnostics
struct uploaded to Chrometto.
This is necessary because the existing data didn't allow us to
diagnose the error which occurs inside a transaction.
Google3 side CL that adds this field:
cl/473102826
Bug: 1321483
Change-Id: I67a2e5aa012cb95f001a50402248ab7f5f7c6936
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3885062
Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1046610}
This CL performs the following cleanups.
* SQLDatabaseTest.Rollback is deleted, because it's redundant with
SQLTransactionTest.ExplicitRollback and
SQLTransactionTest.NestedRollback.
* SQLDatabaseTest.RazeLock is broken into
SQLDatabaseTest.Raze_OtherConnectionHas{Read,Write}lock.
* SQLDatabaseTest.RazeEmptyDB is renamed to EmptyDatabaseFile
* SQLDatabaseTest.RazeAndClose and RazeAndClose_Diagostics are broken
into RazeAndClose, RazeAndClose_{DeletesData, IsOpen,
Reopen_NoChanges, OpenTransaction, Preload_NoCrash, DoesTableExist,
IsSQLValid, Execute, GetUniqueStatement, GetCachedStatement,
TransactionBegin} and Close_IsSQLValid.
* RazeAndClose_Invalidates{Unique,Cached}Statement are added, based on
the tests involving `valid_statement` in SQLDatabaseTest.Poison.
* SQLDatabaseTest.Poison is broken into sub-tests that parallel the
RazeAndClose_* tests.
* SQLDatabaseTest.AttachDatabase{WithOpenTransaction} are cleaned up and
ported away from sql::Database's direct transaction APIs.
* The modified tests have their SQL statements updated to follow the SQL
style guide.
This CL is intended to give us better coverage around
Database::RazeAndClose() and Database::Poison(), and start migrating
away from Database::{Begin,Rollback,Commit}Transaction() APIs.
Bug: 1310577, 1311718
Change-Id: I1c970988851dd098c0deddb297242e37f4654340
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3558074
Reviewed-by: Austin Sullivan <asully@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#987134}
https://crrev.com/c/3520185 removed error checks in the code calling
sqlite3_backup_init(), based on a DCHECK that suggested the function
should not error out given our usage.
Crash data from the field proved this assumption was incorrect. This
CL adds a test demonstrating one failure case, and adds error handling
back in the code calling sqlite3_backup_init().
Bug: 1308325
Change-Id: I6c54a521faeb7812d01f7c9645646aba4ccb693f
Tested: New test hit the deleted DCHECK (before it was deleted).
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3554638
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986071}
The use of SQLite foreign keys is discouraged in Chrome feature code.
After this CL lands, setting
DatabaseOptions::enable_foreign_keys_discouraged will enable foreign
key enforcement during Database::Open*(). This creates a supported path
for enablign foreign key enforcement, in a world where sql::Database
will no longer allow executing arbitrary PRAGMA statements.
On databases where foreign key enforcement is disabled, creating foreign
key constraints still works, but the constraints are not enforced.
This change applies to all Chrome feature code, but not to WebSQL, which
uses its own SQLite abstraction.
Disabling foreign key constraint enforcement in Chrome features is the
best we can do for now, because WebSQL still needs SQLite support for
foreign keys. After we remove WebSQL, we can remove all foreign key
support from SQLite.
Bug: 910955
Change-Id: I1472fb8fe19041a8ac3381c1dd85199c069812ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3032142
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983508}
The tests ensure that our sql::Database::Open*() implementations
configure SQLite with the values in sql::DatabaseOptions. Some tests in
this CL are new, while some tests are moved over from sql::Database's
unit tests.
Change-Id: I709c5380726e1a3f6cd9022cce0734b6d2cbe713
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3526264
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#981393}
This CL performs the following changes.
1. The BackupDatabase() helper is renamed to BackupDatabaseForRaze() to
reflect the only supported use case, and receives better
documentation, via comments and DCHECKs.
2. The GetSqlite3File() helper is moved to Database::GetSqliteVfsFile()
and receives better documentation, via comments and DCHECKs. Thanks
to the documentation fixes, we can now close issue 329982.
3. The GetSqlite3FileAndSize() helper is removed, and inlined in the two
callers. The reasoning behind handling GetSqlite3File() failures is now
documented at each call site.
4. Database::GetAppropriateMmapSize() is renamed to
Database::ComputeMmapSizeForOpen() and receives more thorough
documentation.
Bug: 329982
Change-Id: Ia98bad4489a78c607f60e3da787f809f45298782
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3520185
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#980674}
This functionality is only used by a couple of //sql tests. Using
tested, without adding a significant readability burden.
Database: :set_error_callback() reduces the complexity of the code being
Change-Id: Id00407c57cf5249f178e85bdae6bda5aa98b8b84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3515427
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#979969}
This is a reland of commit a8d0a5b284
The original CL failed to compile in some configuration because of
incorrect use of the GUARDED_BY() macro with a SequenceChecker. This CL
corrects the use to GUARDED_BY_CONTEXT(). Sample compilation error for
the original CL: https://ci.chromium.org/b/8820233398401035361
Original change's description:
> sql: Add guards around the database error callback.
>
> This CL makes the following code changes.
>
> 1. Adds a DCHECK() verifying that sql::Database::set_error_callback() is
> only called with a non-null callback when the current error callback
> is null. This is intended to prevent confusion during development.
> 2. Turns sql::Database::has_error_callback() into a HasErrorCallback()
> that is only accessible to sql::Recovery.
>
> This CL makes following changes to the tests covering database error
> callbacks.
>
> 1. A monolithic test is broken up into multiple test cases.
> 2. The object used to track callback storage lifetime is conceptually
> simplified (RefCount -> LifeTracker) and beefed up with sequence
> checks.
> 3. The test covering set_error_callback() inside the error callback
> is erased, as the newly introduced DCHECK disallows that usage.
> 4. The SQL statements are brought in line with the style guide.
>
> Change-Id: I6a00ff875d976ce17017b6655a49e2b111f91d78
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170894
> Commit-Queue: Victor Costan <pwnall@chromium.org>
> Auto-Submit: Victor Costan <pwnall@chromium.org>
> Reviewed-by: Austin Sullivan <asully@chromium.org>
> Commit-Queue: Austin Sullivan <asully@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#978796}
Change-Id: I4a1ea0eb922ef518f0ca46a3543ff9cb68ef6e3c
Cq-Include-Trybots: luci.chromium.try:fuchsia-arm64-cast,linux-trusty-rel
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3510951
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#978940}
This reverts commit a8d0a5b284.
Reason for revert: Breaks compilation, see https://ci.chromium.org/ui/p/chromium/builders/ci/linux-trusty-rel/34403/overview
Original change's description:
> sql: Add guards around the database error callback.
>
> This CL makes the following code changes.
>
> 1. Adds a DCHECK() verifying that sql::Database::set_error_callback() is
> only called with a non-null callback when the current error callback
> is null. This is intended to prevent confusion during development.
> 2. Turns sql::Database::has_error_callback() into a HasErrorCallback()
> that is only accessible to sql::Recovery.
>
> This CL makes following changes to the tests covering database error
> callbacks.
>
> 1. A monolithic test is broken up into multiple test cases.
> 2. The object used to track callback storage lifetime is conceptually
> simplified (RefCount -> LifeTracker) and beefed up with sequence
> checks.
> 3. The test covering set_error_callback() inside the error callback
> is erased, as the newly introduced DCHECK disallows that usage.
> 4. The SQL statements are brought in line with the style guide.
>
> Change-Id: I6a00ff875d976ce17017b6655a49e2b111f91d78
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170894
> Commit-Queue: Victor Costan <pwnall@chromium.org>
> Auto-Submit: Victor Costan <pwnall@chromium.org>
> Reviewed-by: Austin Sullivan <asully@chromium.org>
> Commit-Queue: Austin Sullivan <asully@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#978796}
Change-Id: I2b643cfdca266bb79bbaf1eabe52a04761bd7320
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3510371
Auto-Submit: Dominique Fauteux-Chapleau <domfc@chromium.org>
Owners-Override: Dominique Fauteux-Chapleau <domfc@google.com>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#978800}
This CL makes the following code changes.
1. Adds a DCHECK() verifying that sql::Database::set_error_callback() is
only called with a non-null callback when the current error callback
is null. This is intended to prevent confusion during development.
2. Turns sql::Database::has_error_callback() into a HasErrorCallback()
that is only accessible to sql::Recovery.
This CL makes following changes to the tests covering database error
callbacks.
1. A monolithic test is broken up into multiple test cases.
2. The object used to track callback storage lifetime is conceptually
simplified (RefCount -> LifeTracker) and beefed up with sequence
checks.
3. The test covering set_error_callback() inside the error callback
is erased, as the newly introduced DCHECK disallows that usage.
4. The SQL statements are brought in line with the style guide.
Change-Id: I6a00ff875d976ce17017b6655a49e2b111f91d78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170894
Commit-Queue: Victor Costan <pwnall@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#978796}
In SQLite 3.38.0, "PRAGMA integrity_check" recovers from an incorrect
page count in the database header, without recording an error. This is
consistent with
https://www.sqlite.org/fileformat2.html#in_header_database_size which
says that SQLite may fall back to the file size, instead of relying on
the database size in the header.
This CL switches the SQLDatabaseTest.FullIntegrityCheck test from
sql::test::CorruptSizeInHeader() to sql::test::CorruptIndexRootPage().
This allows the test to pass in both our current SQLite version
(3.37.1) and in the upcoming SQLite 3.38.0.
This CL also updates the FullIntegrityCheck() documentation, adding API
guarantees in case the database is corrupted.
Bug: 1300442
Change-Id: I70d373a4156fd710f96a50125affe439e7cb5780
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3489937
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#975408}
The current implementation only reports the errors from the last row
returned by "PRAGMA integrity_check". This CL fixes the bug. The test is
updated to show more useful error information on failures.
Bug: 1300442
Change-Id: I4e91134df942f8ba2e7ade4e92a3ad84a2046929
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3486112
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#974711}
The implementation is migrated to base::File and
base::{Read,Write}BigEndian().
The unit test is broken into three separate tests that cover individual
aspects (database open, SQL statement exection, schema checks) of the
expected behavior.
Bug: 1300442
Change-Id: I58833185e005cedb098e0f9e42e28273a0191e8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3487146
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#974708}
This method is not used anywhere. Removing it leaves exactly one way to
do an integrity check.
Furthermore, removing this method may facilitate upgrading to SQLite
3.38.0, as its unit tests are currently failing when ran against the
3.38.0 code.
Bug: 1300262
Change-Id: I6f8fc45d31044d8d0a9c1cb619701c87da720cf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3485179
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#974351}
This CL adds DCHECKs with clear messages covering the cases when
Database::Get{Cached,Unique}Statement() and Database::IsSQLValid() are
passed SQL strings that don't contain any SQL statements.
Currently, Get{Cached,Unique}Statement() DCHECK with an obscure message
in the Database::StatementRef constructor, and Database::IsSQLValid()
does not DCHECK.
Bug: 1230443
Change-Id: I129769c849a441adeb6fcd45ed128e41a1673ef0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3042773
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#904204}
The Database::Execute() guidance currently points to
ExecuteAndReturnErrorCode() for situations where errors would be
ignored. However, the guidance is ignored [1, 2], and
ExecuteAndReturnErrorCode() is not used outside of //sql.
This CL makes ExecuteAndReturnErrorCode() private to Database, and
updates testing code that relied on it. This is an architectural
improvement, as code outside //sql should not have to know about SQLite
error codes.
[1] https://source.chromium.org/search?q=ignore_result%5C(.*%5C.Execute%5C(
[2] https://source.chromium.org/search?q=ignore_result%5C(.*-%3EExecute%5C(
Change-Id: I8d58180a40094c50e1da12b1445fb94c88531a35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3043151
Auto-Submit: Victor Costan <pwnall@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#904027}
Double-quoted string literals are non-standard SQL. Allowing
double-quoted string literals is now considered a misfeature by SQLite
authors. See https://www.sqlite.org/quirks.html#dblquote
Disabling double-quoted string literals for sql::Database users is the
best we can do for now, because WebSQL still needs SQLite support for
this misfeature. After we remove WebSQL, we can remove double-quoted
string literal support from SQLite.
Bug: 1230199
Change-Id: Id285fd442e512491206434864f3488e7b1ef6e6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3035813
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#903562}
The use of SQLite views is discouraged in Chrome feature code.
After this CL lands, accessing views in SQL statements requires setting
DatabaseOptions::enable_views_discouraged to true. This applies to all
Chrome feature code, but not to WebSQL, which uses its own SQLite
abstraction. CREATE VIEW and DROP VIEW will still work, but the views
will not be accessible to SQL statements such as SELECT.
The change will make Chrome feature developers aware when using this
discouraged feature, which is expected to curb usage.
Disabling access to views in Chrome features is the best we can do for
now, because WebSQL still needs SQLite support for views. After we
remove WebSQL, we can remove all view support from SQLite.
Bug: 910955
Change-Id: I9f6c6c40b3a60e9eef4d0e1f07769664d5a9bf4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3015310
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#903148}
The extra text gets ignored by SQLite's prepared statements API. At
best, the ignored text wastes bytes in the binary. At worst, the extra
text may confuse readers, or cause a bug.
Bug: 1230443
Change-Id: Ia3bce0288d036ac7ff7cdfc8a52bd9f45241d139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3034486
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#903092}
The use of SQLite virtual tables is discouraged in Chrome feature code.
After this CL lands, accessing virtual tables in SQL statements requires
setting DatabaseOptions::enable_virtual_tables_discouraged to true. This
applies to all Chrome feature code, but not to WebSQL, which uses its
own SQLite abstraction. CREATE VIRTUAL TABLE and DROP VIRTUAL TABLE will
still work, but the virtual tables will not be accessible to SQL
statements such as SELECT and INSERT.
The change will make Chrome feature developers aware when using this
discouraged feature, which is expected to curb usage.
Disabling access to virtual tables in Chrome features is the best we can
do for now, because WebSQL still needs SQLite support for the fts3
virtual table. After we remove WebSQL, we can consider removing all
virtual table support from SQLite.
Bug: 910955
Change-Id: Ib89837d0f874c0f2a0c1a0ca2ebef6b14ada1335
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3034241
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#902751}
Database::GetUntrackedStatement()'s stated purpose is to facilitate
const methods that execute SQL statements. This is a broken premise --
executing SQL statements modifies internal SQLite state, and in some
cases (shared locking), even read-only queries perform file system
calls.
This CL removes GetUntrackedStatement(), removes the const qualifiers on
methods that used it, and simplifies a few internal checks.
Change-Id: I8a2da85121c4cdf40b643e32081beaca7d57fb1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3033576
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#902747}
This database operation parameter should be immutable during database
operation. Moving to DatabaseOptions makes this explicit.
This CL also makes it clear that the mmap alternative status feature is
discouraged, so new feature designers will use the supported alternative
(meta tables).
The alternative status storage method is discouraged because the status
is stashed in the database schema, and changing it invalidates all
precompiled statements.
Bug: 1126968
Change-Id: I3530721f13e0e2192369d79b03df86ac7a67cf90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3019891
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: David Van Cleve <davidvc@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#901145}
The use of SQLite triggers is discouraged in Chrome feature code.
This CL disables trigger execution from databases that use sql::Database
infrastructure -- this applies to all Chrome feature code, but not to
WebSQL, which uses its own SQLite abstraction. CREATE TRIGGER and DROP
TRIGGER will still work, but the triggers will not be executed.
Disabling trigger execution in Chrome features is the best we can do for
now, because WebSQL still needs SQLite support for triggers. After we
remove WebSQL, we can remove all trigger support from SQLite.
Bug: 910955
Change-Id: I1f994c67341e44253f1f78188a1eaa7c4d966f88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3020167
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900805}
SQL compilation errors can be caused by database schema corruption,
which can occur in normal operation -- data written to disk is always
subject to corruption.
Bug: 839186
Change-Id: I639214dee6db6d6df9c4611b5dae3e4f70cca207
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3015299
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#899985}
This is a reland of 525b30ab9b
This reland removes the call to GetDBOptions() from the DatabaseTest
constructor, which was flagged as undefined behavior by the ubsan-vptr
bot. sql::Database construction is deferred to the SetUp() method by
turning the test's sql::Database member into a
std::unique_ptr<sql::Database>.
Original change's description:
> sql: Remove SQLTestBase.
>
> Change-Id: I87bf9499ef590b006660d3b8ab305b0192ec405c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2866306
> Auto-Submit: Victor Costan <pwnall@chromium.org>
> Commit-Queue: Ayu Ishii <ayui@chromium.org>
> Reviewed-by: Ayu Ishii <ayui@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#879443}
Change-Id: Ie83bf28eaebb88883b9eb37a7d8407e8bfc619ad
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2878638
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880595}
Currently, the SQLDatabaseTestExclusiveMode suite is not instantiated,
so the tests are not run. This CL instantiates the suite.
This problem was discovered by attempting to roll googletest past CL
315255779, which causes test binaries to fail when they include test
suites / parametrized tests that are not instantiated, and when they
include empty test suites.
Bug: 1163396
Change-Id: Ieb47f8d612c48af04f3d9d05e1873fc437c2f6c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615579
Commit-Queue: Victor Costan <pwnall@chromium.org>
Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841220}
This change makes it so that all tests in database_unittest.cc now run
both with WAL mode on and off. Better test coverage will help us
transition to using WAL mode by default in the future.
Bug: 78507
Change-Id: I116a8151c1ab4919a78657e46df98b26eddb7f11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490664
Commit-Queue: Shubham Aggarwal <shuagga@microsoft.com>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821790}
sql::Database uses internal state to manage its configuration options
like page size, journal mode etc. These options need to be set before
Database::Open() is called. Any change to this state has no effect if
done after the database has already been opened. There are also some
SQLite subtleties around different behaviour of this state depending on
whether the database is being opened for the first time or being
re-opened.
This change encapsulates all such state into a sql::DatabaseOptions
struct which is passed to the sql::Database constructor. This ensures
that the required options have been configured before calling Open().
We migrate sql/ to use the new constructor instead of the setters for
the state. In upcoming changes, we plan to remove the setters for this
state. This will make the Database API enforce that the options are set
only once and stay the same over the lifetime of the object.
Bug: 1126968
Change-Id: Id44b6b3883f7a695f3b5cd96ad071e15cb1dc44a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427035
Commit-Queue: Shubham Aggarwal <shuagga@microsoft.com>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817386}
Chrome used the default SQLite configuration where databases were first
opened in the NORMAL (not exclusive) locking mode, and then databases
that opt in were switched to the EXCLUSIVE locking mode via a "PRAGMA
locking_mode" statement.
This CL switches the default. Databases are first opened in EXCLUSIVE
mode by changing the SQLITE_DEFAULT_LOCKING_MODE build configuration
macro. Databases that don't opt into exclusive mode are switched to the
NORMAL locking mode via a "PRAGMA locking_mode" statement.
This CL should not introduce any behavior changes in most cases. The
result of the "PRAGMA locking_mode" statement was previously ignored,
and is now checked. So, if the desired locking mode cannot be set,
sql::Database::Open() will now fail.
In the long run, Chrome's SQLite usage should be migrated to use the
exclusive locking mode, for the benefits outlined in
https://crbug.com/1120969. When that happens, the "PRAGMA locking_mode"
can go away.
Bug: 56559, 1120969
Change-Id: Ia9ac996fae45890bb5ca11168422e15e594087ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2460982
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815540}
CorruptSizeInHeader is a test helper function that allows to simulate
a corrupt database by altering the header of the DB file. In WAL mode,
transactions don't always touch the DB file, which means the database
can behave normally even when corrupted in this way. In this change,
the CorruptSizeInHeader method checkpoints and truncates the WAL
file. This ensures that any subsequent transaction is guaranteed to hit
the DB file and hence be affect by the header corruption. This change
also adds a test that verifies that the function works correctly.
Change-Id: Ic584e29f89b09a886d96e4424a17de8d37683314
Bug: 78507
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2278413
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785702}
As per issue 78507, we are looking to add support for SQLite databases
to use Write-ahead logging (https://www.sqlite.org/wal.html) mode in
Chromium. WAL mode should give us significant performance gains across
almost all use-cases.
This change is a first step towards achieving this. It adds opt-in
support to enable WAL mode for a database connection and perform a
checkpoint. It also adds a feature flag to enable WAL mode for all
databases by default to investigate its feasibility and impact on
performance.
Bug: 78507
Change-Id: I7fc5edcc39b50d2a13755d587cf342bded1af60a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095927
Commit-Queue: Shubham Aggarwal <shuagga@microsoft.com>
Reviewed-by: Brandon Maslen <brandm@microsoft.com>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Cr-Commit-Position: refs/heads/master@{#780318}
This reverts commit 146b9f1b1d.
Reason for revert: These tests were actually failing because death
tests weren't working on win-asan. That problem has been fixed as of
r771124. Also win-asan is not accurately named as it does not define
ADDRESS_SANITIZER.
Original change's description:
> [Sheriff] Disable SQLDatabaseTest on Windows ASAN because they are flaky
>
> Bug: 1027481
> Change-Id: I7fc1806eada39c58f3f6035736bf9d5c595fbb0b
> TBR: pwnall@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946331
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#720463}
TBR=guidou@chromium.org,gab@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 1027481
Change-Id: I91ab2178d555e7ee852d7a8bf6bb5b0ca90f806d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216358
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771889}
this information was de-staffed in 2017, so it's time to remove the
instrumentation code.
sql: :Database can report diagnostic information via
base: :DumpWithoutCrashing(). The project that was supposed to analyze
Change-Id: I8bfd012b67a81e95e9d3598c7dddcd0eb748e015
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1549589
Auto-Submit: Victor Costan <pwnall@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#647434}
