Quotes started being added to integers in r4984 (August 2004). Before
that, is_numeric() was used to determine whether to add quotes, so
quotes were omitted from numeric strings, which is obviously wrong.
The idea here is to use the type of the variable to hint to the database
as to whether quotes are needed. The results are somewhat inconsistent,
since some callers do not convert numeric strings obtained from user
input to integers. That makes it a more conservative change. Callers can
opt out of unquoted integers by casting them to string.
The reason for doing this is that quoting integers turns out to be not
as harmless as originally assumed. We found a case of it confusing the
MariaDB query planner, causing inappropriate indexes to be used.
I also made addQuotes() consistently return a string, instead of
returning an integer for boolean values. This was already the case for
MySQL, but it seems like a good idea everywhere.
Bug: T238378
Change-Id: I70473280f542ee5ecd79e187f580807410fbd548
assertSame() is guaranteed to not do any type conversion. This can be
critical when acciden tially comparing, for example, 0 to 0.0.
Change-Id: Iffcc9bda69573623ba14af655dcd697d0fcce525
Enforce this pattern for the remaining LoadBalancer methods.
Carry this over into Database::close() to decide how loud the
error handling should be.
In LBFactory, clean up ownership of newMainLB()/newExternalLB().
The should have a null owner if called from outside the class
since the LBFactory does not track nor care about them anymore
in that case. Disable newMainLB() for LBFactorySingle as it
makes no sense and was broken.
Also remove some redundant abstract LBFactory methods that
just duplciate ILBFactory.
Bug: T231443
Bug: T217819
Depends-On: I7ed5c799320430c196a9a8e81af901999e2de7d0
Change-Id: I90b30a79754cfcc290277d302052e60df4fbc649
Make flushSnapshot() logging more detailed and check for explicit transaction
rounds. Also removing periods and trailing newlines from log/exception messages.
Change-Id: I0f6520f563680ab3a65b6338ced59ba25a2ec7b5
The callback will be called immediately when the section is cancelled,
whether that occurs directly, or via cancelling of an outer section, or
via rollback of the entire transaction.
Change-Id: Id05296948b52b95544547bd38c4387496b6c83b9
This changeset implements T89432 and related tickets and is based on exploration
done at the Prague Hackathon. The goal is to identify tests in MediaWiki core
that can be run without having to install & configure MediaWiki and its dependencies,
and provide a way to execute these tests via the standard phpunit entry point,
allowing for faster development and integration with existing tooling like IDEs.
The initial set of tests that met these criteria were identified using the work Amir did in
I88822667693d9e00ac3d4639c87bc24e5083e5e8. These tests were then moved into a new subdirectory
under phpunit/ and organized into a separate test suite. The environment for this suite
is set up via a PHPUnit bootstrap file without a custom entry point.
You can execute these tests by running:
$ vendor/bin/phpunit -d memory_limit=512M -c tests/phpunit/unit-tests.xml
Bug: T89432
Bug: T87781
Bug: T84948
Change-Id: Iad01033a0548afd4d2a6f2c1ef6fcc9debf72c0d
This shares reconnection and retry logic but lacks some of the
restrictions applied to queries that go through the public query()
interface.
Use this in a few places such as doSelectDomain() for mysql/mssql.
Bug: T212284
Change-Id: Ie7341a0e6c4149fc375cc357877486efe9e56eb9
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
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
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
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
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
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
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
Aaron is concerned about the extra time added to atomic sections within
an outer transaction if we do a SAVEPOINT and RELEASE. He wants a flag
so callers have to specifically opt-in to use of savepoints.
Change-Id: I64cf5033ced464863d28dd49d9173856a9c1e1c0
Atomic sections are currently useful if you want to wrap some SQL
statements in a transaction when you might be called from inside someone
else's transaction, and you expect the caller to roll back everything if
you fail.
But there are some cases where you want to allow the caller to recover
from errors, in which case you need to roll back just the atomic
section. Savepoints are supported by all our databases and can be used
for this purpose, so let's do so.
Bug: T188660
Change-Id: Iee548619df89fd7fbd581b01106b8b41d3df71cc
If the $var argument is provided, then it will make the resulting
count exclude rows where the value for that column is NULL.
Also add buildSelectSubquery() method and Subquery
wrapper class for use with select() for calculated tables.
Change-Id: I549d629af99afdf370602de095f7fba6d1546c37
A use declaration for a non-namespaced class in a non-namespaced context
causes a PHP warning.
Bug: T189302
Change-Id: I023e64c8194dd03cc3a1098e2d60c73f99bb02e3
This fixes an issue that arises because empty( "0" ) is true in PHP.
The new behavior rejects any conditions that are not strings or arrays,
and lets $conds = "0" be passed to the databases as WHERE 0.
Some databases may reject this as invalid syntax, which is the expected
behavior here, instead of silently ignoring the 0, causing no condition to
be applied to the query.
Bug: T188314
Change-Id: I5bc4d7f41221a886c85e54d9da67c4c095a7d9ce
An INSERT SELECT in MySQL/MariaDB is unsafe for replication if a column
is getting values from auto-increment, statement-based replication is in
use, and the default innodb_autoinc_lock_mode is set.
I9173f655 added checks to force non-native insertSelect for the
statement-based replication and innodb_autoinc_lock_mode != 2 case, but
determining whether a column is getting values from auto-increment is
too hard to do automatically there.
Instead, let's add a flag to let the caller hint that the query isn't
getting any auto-increment values. And use it in MysqlUpdater when
appropriate.
Bug: T160993
Change-Id: If70450a64aa3bcbf763c62838bb21306d124ae3d
It would be easy for a call to nonNativeInsertSelect() to generate an
INSERT that's too big for the database to actually process. Add batching
to try to avoid that.
Bug: T160993
Change-Id: I1de994208d95926f0d75c0d7cab7b5fe1dd565c3
* Make sure all unique keys specified have all their values
provided to avoid large bogus DELETEs. Do not ignore them
in such cases either, as that would cause inconsistencies
between the native and non-native case. Use an exception.
* Make ChangeTags caller clearer that the list of indexes
is not a list of fields for a single index. Also, avoid
mentioning indexes for values not defined in the new
records, as this causes errors or inconsistencies with
the native vs non-native case.
* This also fixes the "Undefined index: ts_log_id" error
when running unit tests on postgres.
Change-Id: I30263df22066bd6d4836202b1bcad5d1aa1e7383
Clean up use of @codingStandardsIgnore
- @codingStandardsIgnoreFile -> phpcs:ignoreFile
- @codingStandardsIgnoreLine -> phpcs:ignore
- @codingStandardsIgnoreStart -> phpcs:disable
- @codingStandardsIgnoreEnd -> phpcs:enable
For phpcs:disable always the necessary sniffs are provided.
Some start/end pairs are changed to line ignore
Change-Id: I92ef235849bcc349c69e53504e664a155dd162c8
Only missing cases where 'USE INDEX' and 'IGNORE INDEX'.
The test doesn't do much since the underlying methods are no-ops
by default, but at least it ensures there are no PHP errors from
these branches.
We can later re-use some of these test cases in tests specific
to one backend.
Change-Id: Id004a2ae41efaa7a367f964013e25d98ecc591ff
