(bug 6200). [core] - 10https://gerrit.wikimedia.org/r/77961 [19:31:18] MatmaRex: ^ there you go, a fix for a 2005 bug (#1857). I'm *almost* as cool as you. [19:33:47] (03PS2) 10Cscott: Make line breaks in

behave like
(bug 6200). [core] - 10https://gerrit.wikimedia.org/r/77961 [19:34:06] ^ added release notes [19:42:04] (03CR) 10Wikinaut: [C: 031 V: 031] "++ so very good news ++" [extensions/OpenID] - 10https://gerrit.wikimedia.org/r/55287 (owner: 10Wikinaut) [19:45:17] ebernhardson: debugging posting a comment… It doesn't find the definition [19:46:09] werdna: hmm, no? i wonder if i broke it without noticing on a recent push, happens with no tests :) sec [19:46:24] Unknown flow id '6645733872272877609211450958295368226' requested [19:46:34] It's a good way to learn a bit about the storage architecture of course, debugging it :) [19:46:41] :) [19:52:54] Yeah here's your problem [19:52:55] Query mw_dev (4) (slave): SELECT /* Flow\Data\BasicDbStorage::find */ * FROM `flow_definition` WHERE definition_id = '6.64573387227E+36' LIMIT 1 [19:52:55] (03CR) 10Parent5446: [C: 032] Make line breaks in
behave like
(bug 6200). [core] - 10https://gerrit.wikimedia.org/r/77961 (owner: 10Cscott) [19:53:33] well, *a* potential problem. [19:54:14] parent5446: i expected that patch to linger in review-hell for longer than that [19:54:53] ebernhardson: interesting. [19:55:04] ebernhardson: you can search without the quotes and it works, and with the quotes and it doesn't [19:55:15] Lol well it was only a two-line diff, and it seemed simple enough. [19:55:27] (03Merged) 10jenkins-bot: Make line breaks in
behave like
(bug 6200). [core] - 10https://gerrit.wikimedia.org/r/77961 (owner: 10Cscott) [19:55:35] http://p.defau.lt/?xWU4rOnuhg2A2RHdqOG_rw [19:55:40] werdna: hmm [19:55:49] I think the general problem is these long meaningless numbers for IDs [19:56:00] what's the rationale against just using a simple auto_increment? [19:57:31] well initially they were there for sharding the main db, untill i met with ops and a few core devs and we decided to punt until its a more pressing issue [19:58:17] wanted to maintain the easy movement to sharding later, and i havn't really pulled it out yet because it would need some rearchitecting (the storage class that writes to db doesnt see the object, only the array that represents it) [19:58:53] Maybe if they're going to be long meaningless numbers, we might as well turn them into strings [19:59:00] maybe put an X or something in front of it [19:59:16] yes, i have been intending to use wfBaseConvert, or change the db representation havnt decided. Any opinions there? [19:59:17] and then mysql/php is less likely to treat them as big numbers [19:59:30] currently its decimal(39,0) in mysql, which can fully represent the 128bit id [20:00:14] yeah, except PHP thinks it's a floating point number [20:00:20] and approximates it [20:00:35] i've been treating it as a string the whole time, no issues yet. But yea php will do it for you without thinking twice [20:01:01] well long term I think hex might be a more sensible representation [20:01:15] but either way we need a prefix [20:01:21] because there are all-numeric hex numbers [20:01:40] hmm, i suppose any base that includes the 0-9 has that problem :) [20:01:49] I like the idea of just sticking a non-numeric char in front of it [20:01:58] and then at least it is very clearly a string and not a number [20:02:03] i dont like the idea of storing a 128bit number as a string of ascii chars though [20:02:09] seems, wastefull :) [20:02:10] mmm [20:02:13] true [20:02:36] the generator in mw can also generate 88 bit which i slightly shorter, not sure what the uniqueness tradeoff is there [20:02:50] well, besides a factor of 2^40 :) [20:02:52] the problem is really just that inconsistent treatment in mysql [20:03:04] apparently quotes versus no quotes is significant [20:03:06] and quotes is broken [20:03:11] and quotes is what mediawiki uses :D [20:04:18] hmm, what version of mysql are you on? [20:04:31] Server version: 5.1.66 Source distribution [20:04:51] update to 5.5, prod is mariadb matching either 5.5 or 5.6 [20:05:12] the query works on my local 5.5.31 (from ori's vagrant setup) [20:06:27] nod, but we'd have to be clear about this, since MW supports mysql 5.0.1+ [20:06:52] hmm [20:07:25] (03PS1) 10Aaron Schulz: Enforce version format and chronology [extensions/OAuth] - 10https://gerrit.wikimedia.org/r/77972 [20:07:50] As Asher said, build for Wikipedia [20:08:11] but, like, we have to make sure that people know that yes, we require a higher version of mysql than usual [20:08:28] for now i'll add a LIMITATIONS file to the root :) [20:08:53] ooh, i logged back in at a good time [20:09:52] hey binasher [20:09:57] what query? and i think it's very ok for an extension to have different mysql requirements than core [20:10:09] binasher: summary: http://p.defau.lt/?xWU4rOnuhg2A2RHdqOG_rw [20:10:48] binasher: definition_id is DECIMAL(39,0), older mysql interprets it as string vs decimal compare, always false [20:11:31] there also may be a better way to represent 128bit uuid in mysql? i dunno [20:14:55] (03CR) 10Wikinaut: "(2 comments)" [extensions/OpenID] - 10https://gerrit.wikimedia.org/r/55287 (owner: 10Wikinaut) [20:15:10] (03PS49) 10Wikinaut: (bug 44819) introducing array of $wgOpenIDProviders and $wgOpenIDForcedProvider [extensions/OpenID] - 10https://gerrit.wikimedia.org/r/55287 [20:15:27] macports is busy compiling mysql5.5 [20:15:31] just going to duck out and grab some food. [20:16:27] same [20:17:01] (03CR) 10Wikinaut: "(2 comments)" [extensions/OpenID] - 10https://gerrit.wikimedia.org/r/55287 (owner: 10Wikinaut) [20:18:47] binary(16)? [20:28:30] (03CR) 10Parent5446: [C: 04-1] "(1 comment)" [extensions/UserMerge] - 10https://gerrit.wikimedia.org/r/77814 (owner: 10Wikinaut) [20:32:06] (03PS4) 10Manybubbles: Split the index into Content & General. [extensions/CirrusSearch] - 10https://gerrit.wikimedia.org/r/77821 [20:38:17] (03PS1) 10Spage: More informative warning of validation errors [extensions/EventLogging] - 10https://gerrit.wikimedia.org/r/77974 [20:48:02] binasher: sounds tricky to query [20:48:24] binasher: SELECT * FROM flow_revision where frev_id='hlkPOI^%R^&IO&^E^RUTIRI%^UIY*&OTUY'; [20:48:41] I realise there's probably a hex 2 binary function in mysql somewhere [20:48:48] but we don't use it in mediawiki [20:51:11] (03CR) 10UnwashedMeme: "(2 comments)" [extensions/OpenID] - 10https://gerrit.wikimedia.org/r/55287 (owner: 10Wikinaut) [20:51:20] werdna: theres hex() and unhex() [20:51:51] http://nineofclouds.blogspot.com/2013/03/uuids-in-mysql.html [20:51:57] binasher: but of course we don't have that in MW's DB layer [20:52:33] good thing y'all are software engineers ;) [20:53:37] smartass :p [20:53:55] hehe [20:54:31] there are uuid gen functions in core though, so efficient storage methods specific to them would be appropriate [20:54:58] it would be a first but hey [20:55:10] nod. [20:55:34] maybe it'd inspire starting to store (and be able to sort ranges of) ip addresses efficiently too [20:56:08] binasher: you might be going a bit far there ;-) [20:56:59] cscott: yay [21:04:03] MatmaRex: hard to find open 2004 bugs still [21:04:30] (03PS50) 10Wikinaut: (bug 44819) introducing array of $wgOpenIDProviders and $wgOpenIDForcedProvider [extensions/OpenID] - 10https://gerrit.wikimedia.org/r/55287 [21:05:17] cscott: you could try to solve bug 4. :D [21:05:23] !bug 4 [21:05:23] https://bugzilla.wikimedia.org/buglist.cgi?quicksearch=4 [21:05:31] it's my all-time favorite [21:05:43] bug 1 looks straightforward [21:09:53] ebernhardson: oh you're back [21:10:00] ebernhardson: did you see what binasher had to say about uuids? [21:10:40] werdna: seems reasonable, not sure what the right way to integrate that in DatabaseBase.php is [21:11:05] (03CR) 10Anomie: "(2 comments)" [extensions/OAuth] - 10https://gerrit.wikimedia.org/r/77254 (owner: 10CSteipp) [21:11:14] i havn't seen anything yet that involves sql functions, but i havn't looked that closely [21:11:14] I would say you could pass a UUID (or DatabaseBinaryData) object instead of a string [21:11:31] and then when that serialises in DatabaseBase.php you use unhex() [21:11:57] hmm, and how to know what to turn into the DatabaseBinaryData when select() ? [21:12:36] ways i've seen it done before involve cached schema information, but i dont think we have that [21:14:02] (03PS5) 10Manybubbles: Split the index into Content & General. [extensions/CirrusSearch] - 10https://gerrit.wikimedia.org/r/77821 [21:14:42] (03CR) 10Manybubbles: "Last one fixed delete. SearchIndex->delete apparently is called with a title _string_ not a title object. Which is sad." [extensions/CirrusSearch] - 10https://gerrit.wikimedia.org/r/77821 (owner: 10Manybubbles) [21:19:37] ebernhardson: unhex("the hex'd data") [21:20:09] ugh, macports + mysql55 is fail [21:26:22] werdna: i mean, what goes in the select() ? $dbr->select( 'foo', 'unhex(definition_id)', ... ) ? [21:26:34] oh, no, you'd fix it further up the tree [21:26:48] werdna: how will it know that this row in this table needs that special case? [21:27:14] It would be a generic rule [21:27:16] and you'd do it like this [21:27:39] $dbr->select( 'foo', array( 'foo_id' => new UUIDWrapper( $uuid ) ), … [21:28:27] almost :) conditions are third param, second is the rows to return. [21:28:34] ah yeah [21:28:39] :p [21:28:42] what i'm thinking is it should be round-tripable like other stuff [21:28:58] and then in DatabaseBase::addQuotes() you'd turn a UUIDWrapper into unhex( 'hex'd value' ) [21:30:33] isn't HEX(x) in mysql equivalent to wfBaseConvert(x, 10, 16) in MW? [21:30:47] should be, yes [21:31:06] and UIDGenerator uses that, it's methods accept a param $base [21:31:27] so you could just generate newTimestampedUID128( 16 ) and have a hex UID right away? [21:32:00] yeah, but that's not the point [21:32:12] I mean, converting from hex to decimal or whatever is not a big deal [21:32:14] we know how to do that [21:32:23] it's just a matter of how to structure it in terms of the database access code. [21:33:18] the main issue is, want to store in db as 128bits of data, but cant transmit binary in the queries [21:33:34] right [21:33:45] but you can transmit unhex() [21:33:46] so it needs to change, the question is where in the call graph does it happen. werdna was thinking inside DatabaseBase, but i'm not sure thats possible without having schema knowledge [21:33:57] Like I say [21:34:07] instead of passing the binary data, pass a wrapper object [21:34:24] and then under DB::addQuotes, have it convert the wrapper object into the appropriate SQL phrase. [21:34:42] so then the value returned from select isn't round tripable? [21:34:55] what do you mean by that? [21:35:02] round trippable for what? [21:35:31] as in, you can't select(), pull the output, and then use what you get in a condition? [21:35:37] so first i select some thing, and i have a $row. dbr->select() with the $row['definition_id'] as a condition [21:35:47] I'm pretty sure you can do that anyway [21:35:55] that row attribute wont be wrapped in the thing [21:35:58] I mean, we insert() binary data anyway [21:36:08] for example, when we gzip() things and insert them [21:36:14] aand I don't think it takes any special treatment [21:36:19] mysql will just escape where necessary [21:36:28] so really, maybe we don't need any fancy unhex() ing in the first place [21:36:35] simple :) [21:37:15] I dunno [21:37:16] let me try it [21:37:26] so, that means pack the 128bit uuid into 4 bytes in php [21:37:39] probably a few more than 4 bytes [21:37:44] err, yea :) [21:37:45] 16 [21:37:47] unless you've invented some new compression algorithm [21:38:14] yeah, though it kinda freaks me out to just use some random binary string everywhere :D [21:38:36] I mean, rountripping through SQL is one thing [21:38:44] but sending it to clients is a totally different thing [21:38:57] and expecting it to roundtrip accurately [21:39:01] that sounds like a whole world of pain [21:39:08] so, basically a getPrettyId() that wfBaseConvert [21:39:42] yeah [21:39:48] but then we need to be careful passing them around [21:39:53] because you have to know what format it's in then [21:40:00] then our IDs have two different formats [21:40:01] heh, yea that gets interesting :) [21:40:25] so maybe it's better to have a FlowUUID object and a few methods to get its value. [21:40:27] so, then a non-roundtrippable object that represents the id, and has getBinary(), getId(), and handle round tripping manualy [21:40:35] right [21:40:54] exactly the idea I had [21:42:03] yup [21:43:23] what's the best way to move on this then? do you want to do it? should I? [21:43:27] should we ask binasher first? [21:43:33] (though it was basically his idea anyway) [21:45:24] brion: giving up on sleep? :D [21:45:34] it sounded like hes fine with it. You can do this one, sounds like you know whats going on in there :) [21:45:45] sleep is for suckers [21:58:15] (03CR) 10Aaron Schulz: [C: 032 V: 032] Fixed bogus method call [extensions/FlaggedRevs] - 10https://gerrit.wikimedia.org/r/75989 (owner: 10Aaron Schulz) [22:01:17] (03PS1) 10EBernhardson (WMF): Reusable external store insertion with fallback [core] - 10https://gerrit.wikimedia.org/r/77979 [22:04:21] ebernhardson: I haven't worked with PHP namespaces before… where should I put this guy? and can I say 'use Flow\Data\UUID' if that is indeed where it is? [22:04:35] and that will use just that class? [22:05:45] werdna: ok, no worries :) first off, typically a class gets its namespace from the namespace defined in the same file as it, typically the 2nd or 3rd line of the file, so if the top of the file UUID is in has 'namespace Flow\Data;' then it will be as you suggest [22:06:27] werdna: second, the use statement is strictly a parser alias, so use Flow\Data\UUID; in a file will replace all occurances of UUID with Flow\Data\UUID when the php engine sees it [22:07:13] and any un-use'd class you reference is auto namespaced to the namespace of the file you are in (gah, it sounds more complicated when i explain it :P ) [22:07:47] (03PD2) 10Wikinaut: adds an error message if old and new user names are the same [extensions/UserMerge] - 10https://gerrit.wikimedia.org/r/77980 [22:07:56] nod. Which namespace does this one belong in? Data or Model? [22:08:18] Model i think [22:08:59] (03CR) 10Wikinaut: [C: 031] "as discussed in https://bugzilla.wikimedia.org/show_bug.cgi?id=52549#c11 . tested." [extensions/UserMerge] - 10https://gerrit.wikimedia.org/r/77980 (owner: 10Wikinaut) [22:09:52] (03PS3) 10Wikinaut: adds an error message if old and new user names are the same [extensions/UserMerge] - 10https://gerrit.wikimedia.org/r/77980 [22:10:34] (03PS1) 10Jeroen De Dauw: Update TravisCI config [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77981 [22:10:49] (03CR) 10Jeroen De Dauw: [C: 032] Update TravisCI config [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77981 (owner: 10Jeroen De Dauw) [22:12:20] (03CR) 10Wikinaut: [C: 031] "as discussed in https://bugzilla.wikimedia.org/show_bug.cgi?id=52549#c11 . tested." [extensions/UserMerge] - 10https://gerrit.wikimedia.org/r/77980 (owner: 10Wikinaut) [22:13:16] (03Merged) 10jenkins-bot: Update TravisCI config [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77981 (owner: 10Jeroen De Dauw) [22:23:11] ebernhardson: hey here's a question, why are you using DB_MASTER in BasicDBStorage::find() [22:23:15] because it's going to be cached? or? [22:24:15] werdna: that was the initial idea, yes [22:24:33] I have another question, but would you prefer I put it on the comments for the rev instead of interrupting? [22:25:03] werdna: :) dont worry about it [22:25:30] in splitFrom Row, you have if ( !isset( $row[$key] ) ) {return null;} [22:25:52] that seems a bit extreme, why not set $split[$key] (if not already set) to nothing? [22:26:54] werdna: its typically used to extract either a primary key or a set of keys to use as memcache key [22:27:14] werdna: if it doesnt have the value, we could truncate to '' but then there is a colision between blank and null [22:27:59] i dunno, if trying to map to sql semantics easiest way is to say null doesnt exist :) [22:28:23] mmmm, the only thing is that with the current situation, if you, for example, put the same key in twice by mistake [22:28:24] you get NULL [22:28:27] instead of an array [22:30:47] werdna: depending on which func you pass it to, i think most handle a null from splitFromRow by throwing an exception, could perhaps throw directly [22:30:56] nod [22:31:08] doesn't matter too much, just something I spotted that was a bit weird :) [22:31:25] yea, it will probably cause bugs being inconsistent like that [22:34:12] (03PS1) 10Jeroen De Dauw: Attempt to get coveralls suppot working [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77982 [22:35:05] (03CR) 10Jeroen De Dauw: [C: 032] Attempt to get coveralls suppot working [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77982 (owner: 10Jeroen De Dauw) [22:36:27] hmm, dammit [22:36:34] hex2bin only exists in PHP5.4+ [22:36:49] (03Merged) 10jenkins-bot: Attempt to get coveralls suppot working [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77982 (owner: 10Jeroen De Dauw) [22:39:22] werdna: hmm, pack() out to be able to do it, probably have to pack as 'LLLL', and substr the hex into 4 pieces? [22:39:59] apparently you can do this [22:40:00] pack("H*" , $hex_string); [22:40:09] even more awsome :) [22:48:29] this is weird, those two UUIDs you have differ only by one byte [22:51:31] hi rfarrand [22:52:29] werdna: hello! [22:58:40] (03CR) 10CSteipp: "(1 comment)" [extensions/OAuth] - 10https://gerrit.wikimedia.org/r/77267 (owner: 10CSteipp) [23:11:00] (03CR) 10CSteipp: [C: 032] Allow caching of Special:CentralAutoLogin when safe [extensions/CentralAuth] - 10https://gerrit.wikimedia.org/r/76726 (owner: 10Anomie) [23:11:03] (03Merged) 10jenkins-bot: Allow caching of Special:CentralAutoLogin when safe [extensions/CentralAuth] - 10https://gerrit.wikimedia.org/r/76726 (owner: 10Anomie) [23:39:59] (03CR) 10Parent5446: [C: 032] adds an error message if old and new user names are the same [extensions/UserMerge] - 10https://gerrit.wikimedia.org/r/77980 (owner: 10Wikinaut) [23:40:01] (03Merged) 10jenkins-bot: adds an error message if old and new user names are the same [extensions/UserMerge] - 10https://gerrit.wikimedia.org/r/77980 (owner: 10Wikinaut) [23:41:49] (03CR) 10Parent5446: [C: 04-1] "(1 comment)" [core] - 10https://gerrit.wikimedia.org/r/77979 (owner: 10EBernhardson (WMF)) [23:51:03] (03CR) 10Ori.livneh: [C: 032] "Thanks" [extensions/EventLogging] - 10https://gerrit.wikimedia.org/r/77974 (owner: 10Spage) [23:51:22] (03Merged) 10jenkins-bot: More informative warning of validation errors [extensions/EventLogging] - 10https://gerrit.wikimedia.org/r/77974 (owner: 10Spage) [23:54:34] (03PS1) 10Jeroen De Dauw: Attempt to get coveralls suppot working [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77985 [23:55:01] (03CR) 10Jeroen De Dauw: [C: 032] Attempt to get coveralls suppot working [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77985 (owner: 10Jeroen De Dauw) [23:57:13] (03Merged) 10jenkins-bot: Attempt to get coveralls suppot working [extensions/SemanticMediaWiki] - 10https://gerrit.wikimedia.org/r/77985 (owner: 10Jeroen De Dauw)