Commit graph

97 commits

Author SHA1 Message Date
Tim Starling
673d496f2d Have Database::addQuotes() pass through bare integers without quoting
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
2019-11-18 11:40:28 +11:00
Aaron Schulz
fb621c26a3 rdbms: various cleanups to LoadBalancer::reallyOpenConnection()
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
2019-10-11 11:35:02 -07:00
Aaron Schulz
2f8be5ca9d rdbms: cleanup visibility of Database::makeSelectOptions()
Change-Id: I2e18dd3d450ceec9895d3143074743bc206a943d
2019-09-03 20:27:27 -07:00
Urbanecm
6ded91313c Revert "rdbms: make LoadBalancer::reallyOpenConnection() handle setting DBO_TRX"
This reverts commit 45831e619c.

Reason for revert: Caused beta not work at all.

Bug: T231162
Change-Id: Icc5c1fa0dc01082a622641ad96c22c939cd56d48
2019-08-25 16:39:33 +00:00
Aaron Schulz
45831e619c rdbms: make LoadBalancer::reallyOpenConnection() handle setting DBO_TRX
Make LoadBalancer::reallyOpenConnection() handle initializing DBO_TRX
instead of Database::__construct().

Also:
* Avoid having the "catch" block appear like it returns a
  half-constructed Database.
* Use the variable name $conn instead of $db to be consistent
  throughout the class. Only send Database::__construct() parameters
  that it recognizes instead of mixing in setLBInfo() data.

Change-Id: Iffc3d1d0713051a164adb51a4c4ee12e4ac887c3
2019-08-24 20:06:17 +00:00
jenkins-bot
436f8eb32f Merge "rdbms: remove various deprecated methods" 2019-07-26 15:16:38 +00:00
Aaron Schulz
4c7e4575f5 rdbms: remove various deprecated methods
Change-Id: I5ae923065a08078225b7df080cb92edca799ebaf
2019-07-26 15:56:02 +01:00
Aaron Schulz
023c73f612 rdbms: normalize Database open() code and error handling
Mainly:
* Use oci_new_connect() for Oracle to avoid broken connection reuse
  similar to the PGSQL_CONNECT_FORCE_NEW flag in DatabasePostgres
* Set 'client_min_messages' unconditionally for PostgreSQL
* Factor out Database::getConnectExceptionAndLog() helper method
* Use the same style of query() calls in DatabaseOracle::open() as
  the other subclasses
* Make sure the Database driver handle field is null on failure
  instead of false for sanity

Also:
* Disallow changing of Database handle DBO_* flags after construction
  where it does not make sense to change them
* Do not mention DBO_* flags meant for non-config use in $wgDBservers
* Ignore DBO_PERSISTENT for SQLite if DBO_TRX is also set for sanity
* Remove $wgDBOracleDRCP variable to discourage careless automatic
  setting of DBO_PERSISTENT that breaks LoadBalancer assumptions

Change-Id: Iea948f7f872294ea8fc5d897fc10c9d29b7141d5
2019-07-26 15:24:28 +01:00
jenkins-bot
3fda59f962 Merge "rdbms: fix some phpstorm warnings database classes" 2019-07-13 23:22:51 +00:00
Aaron Schulz
0848e9049c rdbms: fix some phpstorm warnings database classes
Change-Id: Ib3b8aaadda8101ed82158b1260f10f6d7be16783
2019-07-13 21:49:51 +00:00
Aaron Schulz
d322031d52 rdbms: switch to AtEast warning suppression in Database classes
Change-Id: Ia32f1ba048a540438f78b11a1e94f80acfc7bf50
2019-07-13 01:49:05 +00:00
Aaron Schulz
c351a9cab0 rdbms: cleanup some Database error message wording for consistency
Change-Id: I7b338e6e856c62ecaab2ef97f76431c2220b430d
2019-07-11 21:37:54 +00:00
Aaron Schulz
a830c14d0b rdbms: make implement IResultWrapper directly instead of via inheritence
Change-Id: If1b15c0c21d0ee336025fb99f47fc19ddf1d5435
2019-07-04 13:42:53 +00:00
Aaron Schulz
23ffabbf77 rdbms: combine trxLevel and trxShortId fields in Database
This avoids having to keep multiple fields in sync

Change-Id: If96267afe56a9b9cd660bab333e7667e4d8dc3d4
2019-06-28 14:40:27 -07:00
Aaron Schulz
7911da9c6f rdbms: remove $opened field from Database for simplicity
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
2019-06-27 18:29:12 +01:00
Fomafix
9cbb8f104d Use https://www.php.net/ instead of https://secure.php.net/
Change-Id: I0acca592c6909e91b28b904da49dcbd6a43cd2a5
2019-04-12 06:44:48 +02:00
jenkins-bot
970fbf54ab Merge "rdbms: add ATTR_SCHEMAS_AS_TABLE_GROUPS attribute" 2019-03-27 19:22:07 +00:00
jenkins-bot
1a62e51a00 Merge "rdbms: treat cloned temporary tables as "effective write" targets" 2019-03-26 22:02:10 +00:00
Aaron Schulz
314968036b rdbms: add ATTR_SCHEMAS_AS_TABLE_GROUPS attribute
Change-Id: Ia12a00c30039acd6c59726f35d03cc76aa957902
2019-03-26 14:37:27 -07:00
Aaron Schulz
108fd8b18c rdbms: treat cloned temporary tables as "effective write" targets
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
2019-03-26 14:24:42 -07:00
jenkins-bot
4905504fad Merge "rdbms: Use correct value for 'sslmode' in DatabasePostgres" 2019-03-26 18:51:26 +00:00
Mark A. Hershberger
2e5d114a99 rdbms: Use correct value for 'sslmode' in DatabasePostgres
Fix Postgres support by using ‘sslmode=require' instead of ‘sslmode=1'.

See https://www.postgresql.org/docs/current/static/libpq-ssl.html#LIBPQ-SSL-SSLMODE-STATEMENTS

Change-Id: I424b0e3e144bbe9f0a2bde9a3b4a674dde10c729
2019-03-26 18:22:07 +00:00
Aaron Schulz
4118762bbe rdbms: avoid connections on more lazy DBConnRef methods
Also:
* Fixed LoadBalancer::getAnyOpenConnection for both
  DB_MASTER and DB_REPLICA, which are not real indexes.
* Lock down DBConnRef::close since it can only cause trouble.
* Relax DBConnRef restrictions on tablePrefix()/dbSchema()
  for the harmless "getter" mode case.
* Remove redundant DatabasePostgres::getServer definition.

Change-Id: Ia855d901cc3c28147e52284fdabb1645805d4466
2019-03-21 17:35:27 -07:00
Aaron Schulz
6ab386219e rdbms: document Database::doQuery() return value
Removed random @throws tag from a subclass while at it

Change-Id: I1cad1f66b62bb4a306feb5c773ed5556891f82a7
2019-03-01 02:23:27 +00:00
Brad Jorsch
814605a979 DatabasePostgres: Ignore "IGNORE" option to update()
PostgreSQL doesn't support anything like this. For now, avoid generating
invalid SQL by just ignoring the option. If we come up with a use case
someday, that can guide implementation of a workalike.

Also, remove a pointless "IGNORE" from populateExternallinksIndex60.php.
el_index_60 isn't uniquely indexed, so it has no effect anyway.

Bug: T215169
Change-Id: I1409c80b39834d1977c82c489226255a8cc93fd0
2019-02-20 10:39:45 -05:00
Jeff Janes
27d342ef4b rdbms: Remove references to pg_attrdef.adsrc in Postgres code
PostgreSQL v12 will remove the long-deprecated column
pg_attrdef.adsrc.  The supported way to introspect into column
default values is pg_get_expr(adbin, adrelid), which works
back through all versions of PostgreSQL supported by wikimedia.

Changing to the supported method will allow the upcoming v12 of the
database to be used while maintaining compatibility with older
versions, without needing to write version-specific code.

This patch has been tested with maintenance/update.php and
with phpunit in PostgreSQL versions 9.2, 11, and 12dev.  It does
not harm the first two, and fixes errors that would otherwise
arise in the dev version.  All unit tests which pass under version
11 now pass under 12dev as well.

Change-Id: I874d347fd286b26773113d4f0c6c30d9a4055ad3
2019-01-22 19:23:06 +00:00
Fomafix
3ee1560232 No yoda conditions
Replace
  if ( 42 === $foo )
by
  if ( $foo === 42 )

Change-Id: Ice320ef1ae64a59ed035c20134326b35d454f943
2018-11-21 17:54:39 +01:00
Aaron Schulz
633eb437a3 rdbms: clean up return values of IDatabase write methods
Also improved the atomicity and affected row count logic for
insert/replace with sqlite.

Also remove unused "fileHandle" code from insert().

Change-Id: If7b9148fd44f3a958899885753c7c86ba66bf193
2018-10-30 03:34:52 +00:00
Aaron Schulz
fe0af6cad5 rdbms: Database::selectDB() update the domain and handle failure better
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
2018-10-10 12:03:30 -07:00
Brian Wolff
528042054f Fix some phan-taint-check false positives
Change-Id: Ic5ccbb3f97722476bee7188b83b80cdc652d2a64
2018-09-21 17:27:44 -07:00
Aaron Schulz
41a37d14fa rdbms: make Database::open() protected
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
2018-08-21 18:34:51 -07:00
Fomafix
0a0d5cb7f7 Fix typos
Bug: T201491
Change-Id: I25a27d11faabe2f5fa02950c7a4fb58b13fb3662
2018-08-14 09:52:19 +00:00
Aaron Schulz
f3a197e49b rdbms: add "maxReadRows" limit to TransactionProfiler
Change-Id: I008fc1857c7aa43d93f43361803d5e52c4ddf089
2018-08-13 13:30:08 -07: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
Reedy
765370a6db Add @deprecated tags to various class_alias calls
Bug: T195576
Change-Id: I10cd8415891bfe4a278eee06c9cfe905b3e036dc
2018-05-29 13:10:20 -07:00
Brad Jorsch
6318430fc8 PostgreSQL: Set owners for sequences
PostgreSQL allows setting an "owner" column for a sequence, so if that
column is dropped then the sequence will be dropped too. We should
certainly take advantage of that when creating duplicate tables for unit
testing (particularly when $temporary is false), and we may as well do
it for our permanent tables too.

Change-Id: I4822ac33298e3f3ef59f4372a24aa0866a6e66ae
2018-04-09 12:05:29 -04:00
Brad Jorsch
501b1fb7df tests: Reset Postgres sequences when cloning and truncating
This improves the repeatability of the unit tests by making the ID
values generated depend less on what previous tests might have done.

It also prevents tests from using up sequence numbers for the live DB's
tables.

Change-Id: Iaa8ae1e5cef4b9099bd1b4b8fc806f5af372a7ff
2018-04-05 23:57:24 +00:00
Brad Jorsch
43d1f706be rdbms: Allow PostgreSQL schema-check functions to find temporary tables
PostgreSQL puts temporary tables and such in a hidden, per-connection
"schema" that's checked for unqualified table accesses before the normal
search_path. We should check that in all the schema-checking functions.

Change-Id: I1194ac31f31133b177f624138afee19d00e454b9
2018-04-05 22:54:01 +00:00
Brad Jorsch
cc0473766a rdbms: Remove support for PostgreSQL < 9.2, and improve INSERT IGNORE for 9.5
MediaWiki doesn't support PostgreSQL < 9.2, so drop the support for
older versions.

At the same time, since we're messing with the DatabasePostgres::insert()
code anyway, let's start using ON CONFLICT DO NOTHING for PG >= 9.5.

And since we're doing that, let's do the same for
DatabasePostgres::nativeInsertSelect().

Change-Id: I7bf13c3272917ebafeaff11eb116714a099afdf3
2018-04-05 20:52:46 +00: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
Aaron Schulz
d8732ad042 rdbms: clean up session/transaction loss logic in Database
* Refactor the code to be less cluttered, moving some logic
  to reconnect(), which is now named replaceLostConnection()
* Handle the case of a second connection loss on query retry
* Make canRecoverFromDisconnect() check for temporary tables
* Do not clear session-level variables on deadlock errors
  since only the transaction is lost, not the whole session
* Make sure any empty transaction is destroyed on deadlock
* Attempt reconnection *before* triggering the transaction
  callbacks since some of them  might want to use the database
* Define wasConnectionError() for postgres
* Remove unused return value from handleSessionLoss()

Change-Id: Ic1dcab03f087ce310637210e8e9bc0771e44f045
2018-03-29 20:33:31 +00:00
jenkins-bot
90b02397d0 Merge "rdbms: make selectRowCount() use $var argument to exclude NULLs" 2018-03-21 16:45:33 +00:00
Aaron Schulz
c6ac3d182c rdbms: define wasLockTimeout() for postgres
Change-Id: Ic54530f0b48fb3e3a42c1e6e95a5a75c6b5c061d
2018-03-19 18:08:37 -07: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
6261b4187a rdbms: Add $join_conds to IDatabase::estimateRowCount()
So queries with joins can be estimated.

Change-Id: I9163cf9005d2c2001a88bb102eb4142f0322b0df
2018-03-12 12:15:14 -04:00
Aaron Schulz
55ab84e17d rdbms: avoid strange uses of empty()
Change-Id: Id1a8d1aae72cdee48e43ddb3227cd697516411e0
2018-03-01 20:30:07 -08:00
Aaron Schulz
60e8d3e16e rdbms: remove "m" member prefix from various classes
Change-Id: Iade8e8f70bb1307b96683d979d7e3650f4107515
2018-02-17 03:58:46 +00:00
Aaron Schulz
c9ad7037ce rdbms: do not bother making DBO_TRX transactions in IDatabase::lock()
Named locks are session-level constructs and this transaction agnostic.
Also make lockIsFree() a bit more consistent when the thread has the
lock itself.

Change-Id: Ief51196161bbc50c798740f3c738fd0e39880508
2018-02-15 16:32:35 -08:00
Aaron Schulz
ec550d4823 rdbms: remove "m" prefix from Database fields
Done using the PhpStorm refactor->rename tool.

Also move "defaultBigSelects" declaration to DatabaseMysqlBase
as no other classes uses that.

Change-Id: I424a2d9815de3a5d4cca2522f3db23a5efe6b592
2018-02-15 23:29:34 +00:00
Reedy
39f0f919c5 Update suppressWarning()/restoreWarning() calls
Bug: T182273
Change-Id: I9e1b628fe5949ca54258424c2e45b2fb6d491d0f
2018-02-10 08:50:12 +00:00