Track the insert ID value in Database, similar to the affected rows.
This makes it possible for subclasses to stash or override the value,
which is useful when emulating a write operation using multiple queries.
This includes the case of internal use of atomic sections, where the
COMMIT/RELEASE can reset the last_insert_id tracked in the PECL driver
itself.
Use separate methods and fields for "last query statement" information
and "last query method" information.
Make insertId() for SQLite and Postgres better match MySQL:
* Return 0 if the last query statement did not change any rows.
This helps protect against callers that fail to check affectedRows().
* Make it return the existing ROWID/SERIAL column when upsert() updates
an existing row. This adds a new getInsertIdColumnForUpsert() helper
function.
Directly use query() in doReplace() and doInsertSelectGeneric() to make
the affected row/ID logic easier to follow.
Improve insertId() and affectedRows() documentation.
Add more integration tests of row insertion methods.
Bug: T314100
Change-Id: I7d43a2e52260e66acb713554bb883f5f4a14d010
Use str_starts_with, str_ends_with or string offset where appropriate.
This fixes a bug in MimeAnalyzer where the "UTF-16LE" header could not
be identified because of wrong constant. This is the exact type of bug
that the new functions can avoid.
Change-Id: I9f30881e7e895f011db29cf5dcbe43bc4f341062
Subclasses no longer have to implement fetchAffectedRowCount()
and affectedRows() no longer depends on the driver connection
handle.
Set "port" to $wgDBport in LBFactoryTest/LoadBalancerTest to
avoid postgres failures.
Bug: T314100
Change-Id: Ib31a9d2db18d7ba7dcf61fb110d0fef53f455464
Notable changes:
* In SqlBagOStuff::getConnectionFromServerInfo, only two loggers were
injected. The rest implicitly got a NullLogger due to being absent.
These are now effectively unsilenced.
* Database::__construct() required almost all parameters, even the
loggers. I've wanted to move some of DatabaseFactory into the ctor
here for a while. In order to make this change not a breaking
change, the new 'logger' parameter is optional with NullLogger as
default. This allowed some of the test cases, which were simply
passing NullLogger, to be fixed by passing nothing instead of
passing the new option name.
The Database class is behind a dozen layers of indirection for
real use, so this will still be injected just fine (DBF, LB, LBF,
MWLBF, etc.).
* In LegacyLogger, the handling for $wgDBerrorLog was previously
limited to DBConnection and DBQuery. This now includes errors
from other (generally, newer) parts of Rdbms as well, which were
previously missing.
This only affects sites (typically CI and dev setup) where
$wgDBerrorLog is used, as opposed to the more common
$wgDebugLogGroups by-channel configuration.
* TransactionProfiler gets its logger injected in a rather odd way,
via entrypoints (MediaWiki.php, ApiMain.php, and MaintenanceRunner)
as opposed to service wiring. This is kept as-is for now.
* In LBFactoryTest, in particular testInvalidSelectDBIndependent2,
there are cases that intentionally produce failures of which
the result is then observed. In CI we assert that dberror.log is
empty so instead of adding the missing logger fields to that
LBFactory instance, the only one set (replLogger) is removed.
The alternative is to set 'logger' now, which would naturally
cause CI failures due to unexpected entries coming through to
non-mocked error log.
Bug: T320873
Change-Id: I7ca996618e41b93f488cb5c4de82000bb36e0dd3
Remove 'insertSelectIsSafe' option, unused.
Remove 'topologicalPrimaryConnRef' option, no longer used as of two
months ago with I41a57247503 (8c9398f7f9).
Remove unneeded DatabaseSqlite::getTopologyBasedServerId
implementation which can inherit null instead of overriding with string
of "0". Only caller is SqlBagOStuff::makeTimestampedModificationToken
which can be used as MainStash DB, where its important that a given
server always has the same unique name within a set of db hosts that
may replicate to each other. By inheriting null as topology server ID,
it SqlBagOStuff will use IDatabase::getServerName instead. That in turn
uses the 'host' connection parameter, which defaults to null in
DatabaseFactory, and then falls back to the string "unknown" which is
as good as "0" for this purpose.
Bug: T299691
Change-Id: Iceb65c28cdd3c4a89b3c8b34c3f95d3285718ec0
This method is unused and the naming is not very clear in terms of what
it returns (server name vs config map). Removing it reduces externally
exposed complexity.
Optimize DBConnRef::getServerName() when the server index is known.
Use this to eliminate the "topologicalMaster" parameter from Database.
Rename internal fields and paramters in Database to use "primary"
instead of "master", for consistency.
Add some additional clarifying comments.
Bug: T299691
Change-Id: I98515fa02a58a4c72a06f1ff283b249b1617c886
This is really hard to read. What is code, what is string? These
places are so simple, they really don't need the "{$var}" syntax.
Change-Id: I589dedb8c0193eec4eef500bbb896b5b790b727b
Add Database::queryMulti(), which will execute an array of
queries as a batch with minimal roundtrips.
SQLite fallbacks to looping through each statement and
invoking doQuery().
Add QueryStatus class to reduce complexity in Database.
Rewrite doQuery() as doSingleStatementQuery().
Change-Id: I3d51083e36ab06fcc1d94558e51b38e106f71bb9
The first of many changes decoupling SQL building blocks from Database
class.
Bug: T299691
Depends-On: I5d1d5b9b875bced7bda234f45d6d22ed59db4871
Change-Id: I784e78361f5ee629d31c68629d669ee0ddddf929
Pass the error number as an argument, similar to isConnectionError()
Fix related mysql documentation links
Change-Id: Id32ef2fd27de65376960de3f5138ffdf7654ff71
Rename canRecoverFromDisconnect() in order to better describe
its function. Make it use the transaction ID and query walltime
as arguments and return an ERR_* class constant instead of a bool.
Avoid retries of slow queries that yield lost connection errors.
Add methods and class constants to track session state errors
caused by the loss of named locks or temp tables. Such errors can
be resolved by a "session flush" method.
Make assertQueryIsCurrentlyAllowed() better distinguish ROLLBACK
queries from ROLLBACK TO SAVEPOINT queries. For some scenarios,
only full tranasction ROLLBACK queries should be allowed.
Add flushSession() method to Database and flushPrimarySessions()
methods to LBFactory/LoadBalancer.
Also:
* Rename wasKnownStatementRollbackError() and make it take the
error number as an argument, similar to wasConnectionError().
Add mysql error codes for query timeouts since they only cause
statement rollbacks.
* Rename wasConnectionError() and mark it as protected. This is an
internal method with no outside callers.
* Rename wasQueryTimeout(), remove some HHVM-specific code, and
simplify the arguments.
* Make executeQuery() use a for loop for the query retry logic
to reduce code duplication.
* Move the error state setting logic in executeQueryAttempt() up
in order to reduce code duplication.
* Move the beginIfImplied() call in executeQueryAttempt() up to the
retry loop in executeQuery(). This narrows the executeQueryAttempt()
concerns to sending a single query and updating tracking fields.
* Make closeConnection() and doHandleSessionLossPreconnect() in
DatabaseSqlite more consistent with the base class by releasing named locks.
* Mark trxStatus() as @internal.
Bug: T281451
Bug: T293859
Change-Id: I200f90e413b8a725828745f81925b54985c72180
Make getServer() always return a string, as documented, even with new
Database::NEW_UNCONNECTED handles that have yet to call open(). If the
'host' parameter to __construct() is ''/null, getServer() now returns
'localhost' instead of null. This avoids problems like fatal errors in
calls to TransactionProfiler::recordConnection().
Use Database constants for "connectionParams" field keys for better
static analysis.
Also:
* Add Database::getServerName() method that returns "readable" server
names in the style of LoadBalancer::getServerName(). Note that the
"hostName" field is already passed in from LoadBalancer.
* Migrate most getServer() callers to getServerName() for easier
debugging and more readable logging.
* Also, normalize Database/LoadBalancer SPI logging context to use
"db_server" and reduce logging code duplication in LoadBalancer.
Bug: T277056
Change-Id: I00ed4049ebb45edab1ea07561c47e226a423ea3b
This checks that the Database state has not diverged from the driver DB
handle nor server-side connection state due to an exception being thrown
in an unexpected place within internal Database methods. DB handles with
possible state corruption will not accept queries.
For example, a PHP extension like Excimer might be used to throw request
timeout exceptions. Such exceptions can trigger after any PHP or Zend
function returns, e.g. within DatabaseMysqlBase::doSelectDomain() after
the "USE" query completes but before $this->currentDomain gets updated.
Also:
* Make getApproximateLagStatus() catch getLag() errors since begin()
expects it to simply use "false" for the lag value on failure. This
helps assure that $this->trxAutomatic gets properly set.
* Unsuppress exceptions in runOnTransactionPreCommitCallbacks()
as the transaction needs to get aborted anyway (as already happens).
* Unsuppress exceptions in runOnAtomicSectionCancelCallbacks() since
the safest thing to do is just roll back the transaction.
* Only suppress DBError exceptions in runOnTransactionIdleCallbacks().
and runTransactionListenerCallbacks(). Return the array of errors
rather than throw the first one. Most of the callers had to catch
the errors, so it's easier to avoid throwing them to begin with.
* Avoid blanket try/catch in sourceStream(), doReplace(), upsert(),
doInsertSelectGenericand().
* Clarify various code comments and add missing @internal tags.
Bug: T193565
Change-Id: I6b7b02c02b24c2ff01094af3df54c989fe504af7
Also:
* Add more documentation and phan annotations
* Avoid code duplication in resetExpectations()
* Make setExpectation() a bit more readable
* Move non-static field initialization to __construct()
* Convert two fields into simple class constants
* Mark TransactionProfiler with @internal
* Use reportExpectationViolated() in more cases in order to
avoid similar logging code
* Make all transactionWritingOut() and recordQueryCompletion()
arguments mandatory since the only callers already gave them
Bug: T269789
Bug: T277056
Change-Id: I6315e9c6a96f65343d45184a60d12665cbc32fc9
Also:
* Add more documentation and phan annotations
* Avoid code duplication in resetExpectations()
* Make setExpectation() a bit more readable
* Add more type hints to methods
* Move non-static field initialization to __construct()
* Convert two fields into simple class constants
* Mark TransactionProfiler with @internal
* Use reportExpectationViolated() in more cases in order to
avoid similar logging code
* Make all transactionWritingOut() and recordQueryCompletion()
arguments mandatory since the only callers already gave them
Bug: T269789
Change-Id: I5d106ac5c95d0ea94c4f6bdee90ca51f8f7ddbad
Add several new internal methods to help with wrangling
the various formats that rows, conditions, options, and
unique key lists can come in. Remove now unused method
isMultiRowArray().
Add various sanity checks and logging for parameters to
upsert(), replace(), insert(), and insertSelect().
Move DatabasePostgresTest to the integration/ directory.
Change-Id: If5988a6f0816e8da2cbf2fd612e1a3e3a2e9c52f
Add a query builder class which encapsulates the parameters to
IDatabase::select() and related functions.
Override useIndexClause() and ignoreIndexClause() in DatabaseTestHelper
so that index hints can be tested.
Bug: T243051
Change-Id: I58eec4eeb23bd7fb05b8b77d0a748f1026afee52
Move the DBO_TRX init logic out of Database::__construct() and into
LoadBalancer since the later already handles setting and clearing this
flag based on transaction rounds starting and ending.
Add 'lazyMasterHandle', 'topologyRole', and 'topologicalMaster' parameters
to Database::factory() and inject them via LoadBalancer all at once in order
to avoid worrying about call order. Move some type casting code to
Database::__construct().
Add IDatabase::getTopologyRole()/getTopologicalMaster().
Use constants for getLBInfo()/setLBInfo() for better usage tracking and
typo resistance.
Change-Id: I437ce434326601e6ba36d9aedc55db396dfe4452
Add public, protected or private to function missing a visibility
Enable the tests folder for the phpcs sniff
Change-Id: Ibefce76ea9984c47e08c94889ea2eafca7565e2c
Rename Database::hasFlags() to sound less similar to getFlag()/setFlag().
The order of class fields is roughly:
* Objects and resources
* Configuration
* Mutable options that do not have to be kept in sync with the connection
* Session level state tracking (should be in sync with the connection)
* Transaction level state tracking (should be in sync with the connection)
* Results from or timestamp of the last time a certain event occurred
Change-Id: Ibdca2fefe7ed2c792344c5602b5191a950eed933
This avoids having two similar fields that have to stay
in sync. Clean up the related error handling for connections.
If a connection handle is unusable, like when essential SET
queries fail, then destroy it.
Also:
* Avoid use of transactions in DatabasePostgres::determineCoreSchema.
* Make sure all subclasses log on connection failure.
* Add schema sanity checks to mysql/sqlite classes.
* Add IDatabase::QUERY_NO_RETRY flag to simplify reasoning about
queries that already run on open() to begin with.
* Remove unused return value of Database::open.
* Remove deprecated Database::reportConnectionError method.
Change-Id: I97beba7ead1523085bda8784234d00c69ef1accc
Make IDatabase::lastDoneWrites() reflect creation and changes to
the cloned temporary unit test tables but not other temporary tables.
This effects the LB method hasOrMadeRecentMasterChanges(). Other tables
are assumpted to really just be there for temporary calculations rather
acting as test-only ephemeral versions of permanent tables. Treating
writes to the "fake permanent" temp tables more like real permanent
tables means that the tests better align with production.
At the moment, temporary tables still have to use DB_MASTER, given
the assertIsWritableMaster() check in query(). This restriction
can be lifted at some point, when RDBMs compatibility is robust.
Bug: T218388
Change-Id: I4c0d629da254ac2aaf31aae35bd2efc7bc064ac6
Mainly:
* Stash trxLevel as the variable $priorTransaction since
Database::replaceLostConnection might make it 0 when called.
* Factor out Database::beginIfImplied method and call it on
each query attempt of query(), not just the first one.
* Do not bother setting STATUS_TRX_ERROR if a query fails due to
connection issues and was recoverable since requiring ROLLBACK
in order to continue has no real advantage.
* Do not bother setting trxDoneWrites/lastWriteTime for temporary
table operations.
* Make Database::handleTransactionLoss() keep TransactionProfiler
cleaner by calling Database::transactionWritingOut().
Also:
* Make sure Database::wasKnownStatementRollbackError() calls are
right after the corresponding queries so it is easy to follow.
Having connection attempts in between seems fragile.
* Rename Database::doProfiledQuery => Database::attemptQuery and
move more logic to that method.
* Factor out Database::assertNeitherReplicaNorReadOnly method.
* Rename Database::assertOpen => Database::assertHasConnectionHandle.
* Fix wording of Database::wasKnownStatementRollbackError comments.
* Use $isEffectiveWrite variable name instead of $isNonTempWrite
and $isWrite in some places.
Bug: T218226
Change-Id: I2063e4080b41d5fc504f9207a56312ce92130ed7
The Profiler::profileIn and Profiler::profileOut methods are just stubs.
Use a callback to the Profiler::scopedProfileIn method instead.
Change-Id: I16068bce583bb880250fe91235f2283453be5e4c
Also improved the atomicity and affected row count logic for
insert/replace with sqlite.
Also remove unused "fileHandle" code from insert().
Change-Id: If7b9148fd44f3a958899885753c7c86ba66bf193
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
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
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
If we're not going to set trxStatus to an error state in this case, we
need to issue a rollback to be sure the database (i.e. PostgreSQL) isn't
still in an error state too.
Bug: T189999
Change-Id: Id6e203b216fff937b6a97d779b36c278e3366409
Handle all errors in query() that might have caused rollback by
putting the Database handle into an error state that can only be
resolved by cancelAtomic() or rollback(). Other queries will be
rejected until then.
This results in more immediate exceptions in some cases where
atomic section mismatch errors would have been thrown, such as a
an error bubbling up from a child atomic section. Most cases were
a try/catch block assumes that only the statement was rolled back
now result in an error and rollback.
Callers using try/catch to handle key conflicts should instead use
SELECT FOR UPDATE to find conflicts beforehand, or use IGNORE, or
the upsert()/replace() methods. The try/catch pattern is unsafe and
no longer allowed, except for some common errors known to just
rollback the statement. Even then, such statements can come from
child atomic sections, so committing would be unsafe. Luckily, in
such cases, there will be a mismatch detected on endAtomic() or a
dangling section detected in close(), resulting in rollback.
Remove caching from DatabaseMyslBase::getServerVariableSettings
in case some SET query changes the values.
Bug: T189999
Change-Id: I532bc5201681a915d0c8aa7a3b1c143b040b142e
Done using the PhpStorm refactor->rename tool.
Also move "defaultBigSelects" declaration to DatabaseMysqlBase
as no other classes uses that.
Change-Id: I424a2d9815de3a5d4cca2522f3db23a5efe6b592
* Update replace()/upsert() to combine the affected row
count for the non-native case
* Also make replace() atomic in the non-native case,
similar to how upsert() already works
Change-Id: I6c9bcba54eca6bcf4a93a9b230aaedf7f36aa877
