Commit graph

33 commits

Author SHA1 Message Date
Aaron Schulz
9eff263e8e rdbms: add IDatabase::lockForUpdate() convenience method
Change-Id: I238fd96407e1122e90058e2c4acf743044a267ec
2018-07-10 20:09:01 +01:00
Bartosz Dziewoński
485f66f174 Use PHP 7 '??' operator instead of '?:' with 'isset()' where convenient
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
2018-05-30 18:06:13 -07:00
Aaron Schulz
0b5ed025e9 rdbms: do not silently rollback empty transactions on error
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
2018-05-30 03:10:02 +00:00
Aaron Schulz
b6cd5421b9 rdbms: rename onTransactionIdle() to onTransactionCommitOrIdle()
This is clearer and is consistent with onTransactionPreCommitOrIdle()

Change-Id: I3a34a0e9adea69ec55ed6ddfef47703e31e7c3b5
2018-05-09 21:07:06 +00:00
Aaron Schulz
c8085ad43f rdbms: make IDatabase::onTransaction* methods pass the DB handle for convenience
Change-Id: Ia45a26830d62326b103593268fbf34c907783c90
2018-04-24 16:45:11 -07:00
jenkins-bot
984381d6d9 Merge "rdbms: make select() warn when FOR UPDATE is used with aggregation" 2018-04-23 20:01:13 +00:00
Aaron Schulz
b992b3aea5 rdbms: make select() warn when FOR UPDATE is used with aggregation
Using FOR UPDATE or LOCK IN SHARE MODE with aggregation leads to
query errors with PostgreSQL.

Bug: T160910
Change-Id: Iaed964e7e59468365cbc62cb4bfd3ad44b898452
2018-04-20 03:42:26 +00:00
Aaron Schulz
20777d7399 rdbms: make cancelAtomic() handle callbacks and work with DBO_TRX
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
2018-04-19 12:43:01 -07:00
jenkins-bot
2d030f4576 Merge "rdbms: allow cancelation of dangling nested atomic sections" 2018-04-11 19:48:00 +00:00
Aaron Schulz
f9d10f9e03 rdbms: fix transaction flushing in Database::close
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
2018-04-10 22:31:31 -07:00
Aaron Schulz
477b835945 rdbms: allow cancelation of dangling nested atomic sections
* 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
2018-04-10 16:34:31 -07:00
Brad Jorsch
0618227f7f rdbms: Issue a deprecation warning if errors are ignored
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
2018-04-10 02:06:44 +00:00
Kunal Mehta
b4925e34d0 tests: Enable PHPUnit 4/6 compat layer in some tests that need it
Change-Id: I27a21fa9e97414fae02acbefb28011f0275cba63
2018-04-07 19:31:24 -07:00
Brad Jorsch
395462b7d5 rdbms: Roll back empty implicit transaction on error
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
2018-04-05 14:45:18 -04:00
Aaron Schulz
3975e04cf4 rdbms: make Database query error handling more strict
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
2018-04-04 21:26:11 -07:00
Brad Jorsch
3365e83d96 rdbms: Add ATOMIC_CANCELABLE flag for micro-optimization
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
2018-03-22 07:20:54 +00:00
Brad Jorsch
52aeaa7a5f rdbms: Add IDatabase::cancelAtomic()
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
2018-03-22 05:57:42 +00:00
Aaron Schulz
d395dfb039 rdbms: make selectRowCount() use $var argument to exclude NULLs
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
2018-03-18 01:34:33 +00:00
Brad Jorsch
83731192b2 Remove useless use
A use declaration for a non-namespaced class in a non-namespaced context
causes a PHP warning.

Bug: T189302
Change-Id: I023e64c8194dd03cc3a1098e2d60c73f99bb02e3
2018-03-09 14:11:36 -05:00
addshore
0526f2d671 Introduce IDatabase::buildIntegerCast
Change-Id: Ib24856d1ebe017ff07ae497972c764b4a3f3c7df
2018-03-07 13:00:18 +00:00
addshore
f3df984c79 Introduce IDatabase::buildSubstring
Change-Id: I96f3e0c4920d52f63175cb6767c149f20a8a8cde
2018-03-07 12:32:50 +00:00
daniel
fa9f1aca87 rdbms: in Database::selectSQLText, do not treat $conds = "0" as no condition
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
2018-03-02 10:25:44 +00:00
Brad Jorsch
99b65649c0 rdbms: allow callers to hint that native insertSelect() is safe
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
2018-02-28 13:58:37 -05:00
Brad Jorsch
aefb143a8e Database: Add batching to non-native insertSelect()
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
2018-02-28 13:58:37 -05:00
Aaron Schulz
a9af460c3d rdbms: make sure non-native replace() uses one transaction
This is similar to what upsert() already does

Change-Id: Ide83eefe0d937fb2cdc20aa3c7dc9654c4d34beb
2018-02-20 19:21:38 -08:00
Umherirrender
63d96c15fd build: Updating mediawiki/mediawiki-codesniffer to 16.0.0
Change-Id: I59b59f79bbf3ce4feff3b3a20c1c31bc16370531
2018-02-17 13:29:13 +01:00
Aaron Schulz
a3f51001c0 rdbms: clean up non-native Database::replace() code
* 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
2018-01-30 03:19:28 +00:00
Umherirrender
255d76f2a1 build: Updating mediawiki/mediawiki-codesniffer to 15.0.0
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
2018-01-01 14:10:16 +01:00
Kunal Mehta
75160bdd3b Use MediaWikiCoversValidator for tests that don't use MediaWikiTestCase
Change-Id: I8c4de7e9c72c9969088666007b54c6fd23f6cc13
2018-01-01 08:28:02 +00:00
Timo Tijhof
9491b74d00 rdbms: Complete coverage for Database::selectSQLText()
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
2017-07-27 21:30:07 -07:00
Timo Tijhof
f357c5194c rdbms: Increase coverage for Database::selectSQLText()
* Add case for `$tables[0] == ' '`.
* Add case for `$tables == ''`.
* Add case for 'DISTINCT' option.
* Add case for 'FOR UPDATE' option.
* Add case for 'LOCK IN SHARE MODE' option.
* Add case for 'EXPLAIN' option.

Change-Id: I4a5f4754bc30d31ec35a085f39321fd358b6aa49
2017-07-24 19:49:17 -07:00
Timo Tijhof
b70a3bb821 rdbms: Add more @covers to DatabaseSQLTest
Many of the main methods here have, over the years, been split up
into several protected/private methods.

Change-Id: I1b8489b1c61c0294288442a0a0cd28c9fa77f82e
2017-07-24 19:44:06 -07:00
Timo Tijhof
ddb7575e4e rdbms: Refactor DatabaseTest
* Move DatabaseTest and DatabaseSQLTest to libs,
  and remove MediaWikiTestCase dependency.

* Refactor DatabaseTest to be a test of the Database abstract class,
  not of whatever current DB backend is configured by LocalSettings.

  - Remove most switches/conditionals and other tests for specific
    database backends. Move those to individual test classes for
    those backends instead.
  - Some tests appear to have been integration tests for the PHP driver
    and/or the db backend itself. Moved to a new DatabaseIntegrationTest.
  - Now that only the abstract Database is invoked, the test runs a bit
    faster (no real connections/queries).

* Add missing @covers tags, and remove or fix broken ones
  (follows-up 26e52f0c49).

Change-Id: I9dc4a558e701d00e95789e7eb8e02926783b65ad
2017-07-20 18:23:37 -07:00
Renamed from tests/phpunit/includes/db/DatabaseSQLTest.php (Browse further)