Also improved the atomicity and affected row count logic for
insert/replace with sqlite.
Also remove unused "fileHandle" code from insert().
Change-Id: If7b9148fd44f3a958899885753c7c86ba66bf193
The documentation says "This must be a valid SQL fragment", but as
written it breaks if given anything other than a field name. It's easy
enough to fix by adding an alias to the internal select() call.
Bug: T201781
Change-Id: I76428af6d3aadc266254fdb24109a0ac2db3761f
LoadBalancer uses Database::getDomainId() for deciding which keys to use
in the foreign connection handle arrays. This method should reflect any
changes made to the DB selection.
If the query fails, then do not change domain field. This is the sort of
approach that LoadBalancer is expects in openForeignConnection(). Also,
throw an exception when selectDB() fails.
The db/schema/prefix fields of Database no longer exist in favor of just
using the newer currentDomain field.
Also:
* Add IDatabase::selectDomain() method and made selectDB() wrap it.
* Extract the DB name from sqlite files if not explicitly provided.
* Fix inconsistent open() return values from Database subclasses.
* Make a relationSchemaQualifier() method to handle the concern of
omitting schema names in queries. The means that getDomainId() can
still return the right value, rather than confusingly omitt the schema.
* Make RevisionStore::checkDatabaseWikiId() account for the domain schema.
Unlike d2a4d614fc, this does not incorrectly assume the storage is
always for the current wiki domain. Also, LBFactorySingle sets the local
domain so it is defined even in install.php.
* Make RevisionStoreDbTestBase actually set the LoadBalancer local domain.
* Make RevisionTest::testLoadFromTitle() account for the domain schema.
Bug: T193565
Change-Id: I6e51cd54c6da78830b38906b8c46789c79498ab5
Move unclosed transaction error reporting to Database so it can
include information about the caller that started the transaction.
Change-Id: I834d957f172c03005de522f3029bb634b3c7220e
Transactional databases normally roll back when a connection is closed
with an open transaction rather than committing them, so MediaWiki
committing them is unexpected.
There are two cases being changed here: automatic transactions without
writes and manual transactions. For the former it shouldn't make a
difference if we commit or roll back since no writes were done anyway.
The latter has logged a message since MW 1.31 (I0992f9a8), and that
warning has not been logged in Wikimedia production in the past 60 days
so we should be ok there too.
Bug: T206147
Change-Id: Ieceef4deb49044db8f0622d38ee76c9d9f39704c
The decision to treat COMMIT/BEGIN as a "read" in isWriteQuery()
for the benefit of ChronologyProtector was first introduced
in r47360 (8653947b, 2009).
* Re-order strings in isTransactableQuery() to match the regular
expression in isWriteQuery() for quicker mental comparison
* Add missing visibility to DatabaseSqlite->isWriteQuery, matching
the parent class implementation.
Bug: T201900
Change-Id: Ic90f6455a2e696ba9428ad5835d0f4be6a0d9a5c
This is not called externally and there is little reason for that to
change. The current caller pattern is to use factory(), possibly with
initConnection() afterwards, or to use a LoadBalancer to begin with.
Change-Id: Ib1fdd5c960f1ed877fcd17bcb99b999d5d894716
This will hopefully provide more diagnostic info when DBTransactionSizeError
rollbacks are triggered.
Bug: T190260
Change-Id: Ib8bea5a9ec7d3ffeaf423adb930dc6fb14314449
Sqlite used the base implementation of trying a SELECT 1 query and
seeing if it failed. Instead, make it use the sqlite_master table.
Also remove the base version of that method since it would always
cause this problem and all subclasses have proper implementations.
Make LoadBalancerTest::assertWriteAllowed() more explicit and add
more assertions there.
Change-Id: I6c7b0bea8894c45dfe8931748d6687f0e5d1e101
Find: /isset\(\s*([^()]+?)\s*\)\s*\?\s*\1\s*:\s*/
Replace with: '\1 ?? '
(Everywhere except includes/PHPVersionCheck.php)
(Then, manually fix some line length and indentation issues)
Then manually reviewed the replacements for cases where confusing
operator precedence would result in incorrect results
(fixing those in I478db046a1cc162c6767003ce45c9b56270f3372).
Change-Id: I33b421c8cb11cdd4ce896488c9ff5313f03a38cf
Since there might be important view snapshots, temp tables, or effects
from SET statements or the like, go into TRX_ERROR state for "possible
transaction level errors" even if no recognized writes took place and
the transaction was not explicit.
Change-Id: I32c34bc28b845e343d0167a220412824838eaed8
If a pre-commit callback caused a new LoadBalancer object to be created,
that object will be in the "cursory" state rather than the "finalized"
state. If any callbacks run on an LB instance, make LBFactory iterate
over them all again to finalize these new instances.
Make LoadBalancer::finializeMasterChanges allow calls to
already-finalized instances for simplicity.
Bug: T193668
Change-Id: I4493e9571625a350c0a102219081ce090967a4ac
* Handle the case where an onTransaction* callback for one handle
adds more onTransaction* callbacks to a different handle. Instead
of supporting only a short chain of such callbacks, try to resolve
the whole chain by using a loop in LoadBalancer and LBFactory.
* Add sanity checks to enforce the proper call order of LoadBalancer
transaction methods, such as those that execute callbacks. This is
the order that LBFactory already uses. Use ROUND_ERROR for problems
that can ruin the instance state. Such problems require rollback.
* Correct setTrxEndCallbackSuppression() calls in beginMasterChanges()
that were making tests fail.
* Make Database handle callback suppression for FLUSHING_ALL_PEERS
instead of making LoadBalancer/LBFactory have to manage it.
* Simplify finalizeMasterChanges() given that suppression does not
actually effect runOnTransactionPreCommitCallbacks().
* Make dangling callback warning in Database::close work properly.
* Actually use $fname in flushReplicaSnapshots().
* Use DBTransactionError instead of DBExpectedError in some places
where stages fail.
* Fix failing testGetScopedLock() unit tests so everything passes.
Add more comments to setTransactionListener and onTransactionIdle.
Change-Id: I6a25a6e4e5ba666e0da065a24846cbab7e786c7b
When reading through DatabaseMysqlBase::open(), it was not
obvious that execution would not continue after the conditional
`!$this->conn` block, given it ends in a method call, without
return or throw. I considered adding a return statement after it
for clarity, but it seems in this case it might make more sense
to throw directly given $error here has already gone through a
fallback to getLastError() a few lines up.
Replace the other three calls to reportConnectionError() as well,
which previously passed a useful string that was overwritten
with lastError(). Instead, log both. And make their call to
queryLogger->error() match the previous ones to have an 'error' as well.
This leaves reportConnectionError() as being unused, except for
a call from LoadBalancer. That call was problematic because
it was inside a conditional for IDatabase, but the method isn't
part of that interface. Replace it with a direct throw as well.
Deprecate the method as its now unused in core, and also remove its
'# New method' comment which hasn't made sense since r75341 (16cded8b32).
Change-Id: I0f2ef00ba44bf7090a3ce54edeb8c7e8e543e46a
Using FOR UPDATE or LOCK IN SHARE MODE with aggregation leads to
query errors with PostgreSQL.
Bug: T160910
Change-Id: Iaed964e7e59468365cbc62cb4bfd3ad44b898452
Make transaction callbacks aware of cancelled sections. If the
statements of a section are reverted via cancelAtomic(), then the
dependant callbacks are now cancelled as well. Any callbacks for
onTransactionResolution(), which does not depend on COMMIT, will
see the triggering event as a ROLLBACK, since the unit of work it
was part of was rolled back.
Also fix the handling of topmost atomic sections with DBO_TRX.
These still need their own savepoint to make cancelAtomic() work.
Follow-up to 52aeaa7a5.
Change-Id: If4d455c98155283797678cfb9df31d5317dd91a2
We need to make sure a DBO_TRX transaction was started before doing the
CREATE TABLE, because CREATE TABLE itself won't start one and sqlite
breaks if schema changes are done on one handle while another is open.
Also, incidentally, have the handles in these LoadBalancerTests log to
the standard channel. And clean up the auto-rollback of DBO_TRX
transactions to use ->rollback() instead of ->doRollback() plus
incorrect manual setting of trxStatus.
Bug: T191863
Change-Id: Ib422ef89e7eba21281e6ea98def9f98ae762b9fe
Use the right IDatabase constants for the $flush parameter to
the commit() and rollback() calls.
This fixes a regression from 3975e04cf4.
Also validate the mode/flush parameters to begin() and commit().
Bug: T191916
Change-Id: I0992f9a87f2add303ed309efcc1adb781baecfdc
* Make startAtomic() return a token that can be used with cancelAtomic()
cancel any nested atomic sections that have not yet been ended.
* Make doAtomicSection() clear dangling nested sections by default.
* Also give doAtomicSection() a $cancelable parameter, having the
same default as startAtomic().
Change-Id: I75fa234cb1dcfef17dc9a973a3b02d2607efa98e
I532bc5201 added code to put the Database into an error state on error,
to prevent callers from catching and ignoring exceptions without rolling
back. But to avoid breaking everything relying on the ability to do so,
it didn't set the error state for certain types of errors.
To allow those broken callers to be cleaned up, log a deprecation
warning when we detect that someone has indeed ignored one of these
errors.
Bug: T189999
Change-Id: Ib7aca59639f30959e106fd4f1a1209e28bad2857
