Page MenuHomePhabricator

$wgSharedDB PostgreSQL support
Closed, DeclinedPublicFeature

Description

Author: overlordq

Description:
PG doesn't do cross database queries which breaks the easy route of sharing login data across wikis.

So, in a brief flash of insanity, a modification of the typical wikifarm setup. Single database, but different schema. Still use public for tsearch2 et al, but instead of having just mediawiki for the lone wiki schema, have multiple schemas.

So if the DB backend is PG, IMO it should treat $wgSharedDB as $wgSharedSchema.

Haven't thought much past that wrt search_path. But AFAIK if the PG DB driver was modified to explicitly use schemas when referring to tables it shouldn't matter what the search path is set to.


Version: 1.17.x
Severity: enhancement

Details

Reference
bz16794

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.
StatusSubtypeAssignedTask
InvalidNone
DeclinedFeatureNone

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 10:23 PM
bzimport set Reference to bz16794.
bzimport added a subscriber: Unknown Object (MLST).

Created attachment 5624
Completely untested patch

It has always struck me that MySQL's "databases" are about equivalent to PG's schemas, so theoretically it should work. I wonder if this copy-paste from Database.php is enough to make it work?

attachment x.diff ignored as obsolete

overlordq wrote:

I came up with something similar and this is what's wrong:

  1. Wont work as-is, takes $wgSharedTables into account after the PG'ification of table names. Should be moved to after the horrible && mess.
  2. Once moved, yes logging in works, reading pages works. Editing . . . is another matter. It doesn't take into account schema's so it inserts into $table, which depending on the search_path might not be valid.

overlordq wrote:

Ok, Ignore #2. Think I figured out why the editing is snafu.

say we have 2 wikis, awesomewiki && poolwiki with matching schemas.

Poolwiki has the shared usertable so $wgSharedDB = 'poolwiki';

User logs into awesomewiki. The search path gets set to "awesomewiki", public. Currently, it checks for $wgSharedDB, see's that user table is shared, and plops down the poolwiki schema before the queries. The lookup works since the schema is explicit. Next said logged in user goes to submit an edit. MW goes to insert a row into the revisions table with the user data. There is a foreign key constraint on rev_user to the mwuser table. However since we're sharing the poolwiki mwuser table & the searchpath is ("awesomewiki", public), it tries to lookup user in awesomewiki.mwuser which is empty and fails the foreign key constraint.

So I'm stumped on what 'Best Practices' would say on how to move on from here.

larson wrote:

So...what I'm hearing is that, if you want to have a wiki farm with a shared user table right now, you have to go with MySQL. *sigh* This seems like the kind of thing that ought to be explicitly mentioned in the shared-user-table doco.

I don't have time to dig into this much, but if it's just the user table being shared, couldn't you simply change the line in DatabasePostgres.php to something like this:

if ($wgSharedDB) {

return "$wgSharedDB.mwuser";

}
return 'mwuser';

You'd also have to ensure that the FKs are in the right place too, of course, which means modifying tables.sql slightly for a new install, or recreating the FK constraints on an existing database.

Hmm...all in all, sounds like some search_path munging might be easier overall.

*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*

tmp wrote:

PostgreSQL sharedDB with second connection

This patch should allow you to use the $wgSharedDB normally, as you would with mysql. Basically this patch creates a second connection with the shared database and when a query is made, we check on which connection we should send it.

attachment PostgreSQL.patch ignored as obsolete

tmp wrote:

Forgot to say:
The patch is in a "it works for me" state, I'm actually using it with a wiki family...
More testing is appreciated, since it's the first time I write anything for mediawiki (or php in general, but it took only 2 hours to write it, it's not that complicated)

Apply as usual, on the main mediawiki dir:

patch -p0 < PostgreSQL.patch

This patch was written for the latest mediawiki ( 1.16.5 ), so it might NOT work with previous version. It should not be difficoult to port it to older version trough...

tmp wrote:

PostgreSQL - $wgSharedDB - mediawiki 1.17.0beta1

Added patch to make this work with mediawiki 1.17.0beta1 ....
I just ported it from 1.16.5 patch, not really tested, don't have time now, but it is just a copy-paste of the 1.16.5 patch in the right places...

attachment PostgreSQL-1.17.0.patch ignored as obsolete

tmp wrote:

PostgreSQL - $wgsharedDB - mediawiki trunk

Added patch to make this work with mediawiki trunk (29/05/11) ....
I just ported it from 1.16.5 patch, not really tested, don't have time now, but it is just a copy-paste of the 1.16.5 patch in the right places... shouldn't break anything.

attachment PostgreSQL-trunk.patch ignored as obsolete

tmp wrote:

PostgreSQL - $wgSharedDB - mediawiki 1.17.0

Updated to patch smoothly to mediawiki 1.17.0

still working for me, i'm waiting for comments... :p

attachment PostgreSQL-1.17.0.patch ignored as obsolete

overlordq wrote:

Doesn't apply to trunk and unless it was significantly different then was was tried last time[1], it will likely be broken as well.

1 - http://www.mediawiki.org/wiki/Special:Code/MediaWiki/89393#code-comments

tmp wrote:

(In reply to comment #12)

Doesn't apply to trunk and unless it was significantly different then was was
tried last time[1], it will likely be broken as well.

1 - http://www.mediawiki.org/wiki/Special:Code/MediaWiki/89393#code-comments

sorry, didn't see it. i'll post back when i'll resolve it

tmp wrote:

PostgreSql $wgSharedDB support - mwiki 1.17.0

Updated patch: doesn't break update.php anymore.
Again, it works for me, but testing is appreciated.

attachment PostgreSQL-1.17.0.patch ignored as obsolete

tmp wrote:

PostgreSQL wgSharedDB with schemas

This patch (only for trunk) is totally different from the other ones: instead of having multiple connections to a single database, we use postgresql schemas.

we use $wgSharedDB as it was an other schema name.

tested with new wikis installations.
didn't test with update.php as right now in trunk it's broken even without my patch :p

should work trough...

attachment PostgreSQL-trunk.patch ignored as obsolete

tmp wrote:

PostgreSQL wgSharedDB with schemas

The patch was breaking update.php since it was still calling the parent in some cases.

fixed it, update.php works, multiple wiki seems to work.

testing is appreciated (patch only for trunk)

attachment Luker.patch ignored as obsolete

Luca, OverlordQ made this comment on that code:

"In Postgres temporary tables are put in to their own special schema,
 explicitly prefixing all table names with mediawiki's default schema breaks
 any usage of temporary tables.

"Either this needs reverted, or have to do some ugly hacks to the testing
 framework."

Could you look at what changes need to be made, either here or in the testing framework, to make temporary tables work properly?

tmp wrote:

trunk patch to fix temporary tables

this patch (trunk) should put the schema name in front of the table only if the requested table is a shared table.
Otherwise you get only the quoted table name.

This should fix the temporary tables problem. (test, please?)
Since the temp tables are per-connection sharing the same db should not pose any problem.

attachment path_patch.patch ignored as obsolete

You should be able to test using phpunit. Have you tried that yet?

tmp wrote:

I have a couple of problems with phpunit now... i'll test it tomorrow.

right now i only tested install and basic functionality with wgshareddb,

tmp wrote:

tested, seems ok.
since it doesn't force the main schema phpunit is now back to using temporary tables.

If you run phpunit with $wgshareddb set it still gives you the shared table instead of the temporary.
The tableName() for mysql seems to force the database if it's a shared table, so even for mysql phpunit + $wgshareddb gives you the shared table... which means it should now work as intended ( I hope :p)

I haven't tested it with mysql, trough, just looked at the code.

tmp wrote:

PostgreSql wgshareddb support - trunk

the patch has been reverted, but I didn't hear any negative comments, so this update is to keep it here for anyone who needs it or wants to work on it

attachment Mediawiki.patch ignored as obsolete

sumanah wrote:

Luca, if you need any help running PHPUnit, or want to improve your patch to the point where it can be committed, we can help you in the IRC channel: https://www.mediawiki.org/wiki/MediaWiki_on_IRC Thanks!

You have a comment that worries me a bit (it's not your fault though):

  1. Lets test for any bits of text that should never show up in a table
  2. name. Basically anything like JOIN or ON which are actually part of
  3. SQL queries, but may end up inside of the table value to combine
  4. sql. Such as how the API is doing.
  1. Note that we use a whitespace test rather than a \b test to avoid
  2. any remote case where a word like on may be inside of a table name
  3. surrounded by symbols which may be considered word breaks

and then you resort to a regex to catch them:

'/(^|\s)(DISTINCT|JOIN|ON|AS)(\s|$)/i'

Could you comment a bit more on this? (Probably in a separate bug
that blocks this one)?

It's some time since I looked at the API code but I understand that
our current SQL query infrastructure is not sufficient for all API
needs...

That's part of the MySQL code. It dates back to when I implemented shared prefix and shared tables. After implementing it I got bug reports on it breaking and found out that various bits of code had awfully being passing full joins into the list of tables because we didn't have a join syntax back then. I had to add that junk into tableName in order for it to work.

tmp wrote:

PostgreSql $wgSharedDb support - wiki 1.20

Patch updated to work against latest git (29/06).

Works also with phpunit tests (before it didn't apply the table prefix)

I hope it will be included in 1.20...

attachment Luker.patch ignored as obsolete

Thanks for the patch!

Unfortunately, I cannot install mediawiki with this patch:

[29-Jun-2012 18:15:15 UTC] PHP Fatal error: Call to undefined method DatabasePo
stgres::realTableName() in /usr/home/saper/public_html/pg/w/includes/db/Database
Postgres.php on line 1233

I think we should re-think what realTableName and tableName should be... realTableName is needed in some places where we know it's PostgreSQL

tmp wrote:

PostgreSql $wgSharedDb support - wiki 1.20

*Really* sorry, forgot to change one instance of realTableName() into tableName().... my bad... altrough the phpunit test passed...

a quick "grep -r" tells me there are no other realTableName(),and I tested this with raw installation and phpunit.

realTableName is not needed anymore. The distinction was created (I think) since tablename() didn't do proper escaping/quoting on all cases.

Now tableName() handles every escaping/quoting/schema/prefix you should need, Much of it is done by mimicking Database->tableName(), which was called before by realTableName()

attachment Luker.patch ignored as obsolete

Luca, can you check

https://gerrit.wikimedia.org/r/#/c/3323/
(or https://gerrit.wikimedia.org/r/gitweb?p=mediawiki%2Fcore.git;a=commit;h=556c5cf464b9103b04b247ed7dd7ee3051e9aef6)

I believe need real table names for the updater... we shouldn't cheat with renaming "user" and "text" tables to the PostgreSQL-aware parts of code...

tmp wrote:

PostgreSql $wgSharedDb support - wiki 1.20

You're right, I didn't understand the purpose of realTableName.

Corrected, now realTableName does the quoting/schema handling, tableName is untouched.

I also cleand up a little bit the patch, tested with multiple installation/shared tables/phpunit

Attached:

Jdforrester-WMF subscribed.

Migrating from the old tracking task to a tag for PostgreSQL-related tasks.

Aklapper changed the subtype of this task from "Task" to "Feature Request".Feb 4 2022, 11:01 AM
aaron subscribed.

For basic maintainability, no new "support" should be added for this "feature".