The 1.15 release of MediaWiki introduced some hardcoded bitwise operators to the core SQL. They were added to operate on the log_deleted column in the logging table by, I think, aaron. This is because the log_deleted column now has multiple states.
Unfortunately, bitwise operators have different syntax in different databases.
MySQL, PostgreSQL: log_deleted & 1
DB2, Oracle: BITAND(log_deleted, 1)
I think there are three options to make it compatible:
1. Refactor the database to not use an integer as a bit field. Just use four different boolean columns, which works well cross-database.
2. Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
3. Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
My preference is for option 1 or 3. Thoughts?
Regards,
Leons Petrazickis http://lpetr.org/blog/
Oracle abstraction solves this problem in makeList function ... the only weak point for this solution is if you write SQL statements manualy, if you use Database class functions to create the actual SQL statement this works and as i was told on #mediawiki manual sql creation should gradually be rooted out.
Leons Petrazickis wrote:
The 1.15 release of MediaWiki introduced some hardcoded bitwise operators to the core SQL. They were added to operate on the log_deleted column in the logging table by, I think, aaron. This is because the log_deleted column now has multiple states.
Unfortunately, bitwise operators have different syntax in different databases.
MySQL, PostgreSQL: log_deleted & 1
DB2, Oracle: BITAND(log_deleted, 1)
I think there are three options to make it compatible:
- Refactor the database to not use an integer as a bit field. Just
use four different boolean columns, which works well cross-database.
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
My preference is for option 1 or 3. Thoughts?
Regards,
Leons Petrazickis http://lpetr.org/blog/
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
This didn't even come up on my radar as I began working on integrating the 1.15.0 changes into the Microsoft SQL Server version. Here's why: It didn't break anything. The only noticeable database-related breakage recently has been with the Special:RecentChanges and Special:RecentChangesLinked pages, which use LIMIT and ORDER BY together with UNION in a way not supported by SQL Server. So, you can update your summary of the bitwise operator syntax in different databases to reflect that.
MySQL, PostgeSQL, SQL Server log_deleted & 1
On Fri, Jun 12, 2009 at 10:06 AM, Freako F. Freakolowskyfreak@drajv.si wrote:
Oracle abstraction solves this problem in makeList function ... the only weak point for this solution is if you write SQL statements manualy, if you use Database class functions to create the actual SQL statement this works and as i was told on #mediawiki manual sql creation should gradually be rooted out.
Leons Petrazickis wrote:
The 1.15 release of MediaWiki introduced some hardcoded bitwise operators to the core SQL. They were added to operate on the log_deleted column in the logging table by, I think, aaron. This is because the log_deleted column now has multiple states.
Unfortunately, bitwise operators have different syntax in different databases.
MySQL, PostgreSQL: log_deleted & 1
DB2, Oracle: BITAND(log_deleted, 1)
I think there are three options to make it compatible:
- Refactor the database to not use an integer as a bit field. Just
use four different boolean columns, which works well cross-database.
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
My preference is for option 1 or 3. Thoughts?
Regards,
Leons Petrazickis http://lpetr.org/blog/
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Fri, Jun 12, 2009 at 10:02 AM, Leons Petrazickisleons.petrazickis@gmail.com wrote:
- Refactor the database to not use an integer as a bit field. Just
use four different boolean columns, which works well cross-database.
In MySQL, four different boolean columns means four times the storage, as compared to one TINYINT used as a bitfield. So this isn't a good solution.
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
These would be the way to do it, I guess. We do something similar for things like conditionals already. I think 2 is preferable to 3, stylistically.
On Fri, Jun 12, 2009 at 11:06 AM, Freako F. Freakolowskyfreak@drajv.si wrote:
Oracle abstraction solves this problem in makeList function ... the only weak point for this solution is if you write SQL statements manualy, if you use Database class functions to create the actual SQL statement this works and as i was told on #mediawiki manual sql creation should gradually be rooted out.
This isn't a good solution:
foreach ($a as $key => $value) { if (strpos($key, ' & ') !== FALSE) $a2[preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)', $key)] = $value; elseif (strpos($key, ' | ') !== FALSE) $a2[preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $key)] = $value; elseif (!is_array($value)) { if (strpos($value, ' = ') !== FALSE) { if (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*?)\s=\s(.*)/', 'BITAND($1, $2) = $3', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*?)\s=\s(.*)/', 'BITOR($1, $2) = $3', $value); else $a2[$key] = $value; } elseif (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $value); else $a2[$key] = $value; }
It breaks on all sorts of possible input, like $dbr->select( 'revision', '*', 'rev_deleted&1' ) or $dbr->update( 'user', array( 'user_name' => 'Sam & Max'), $where ), or any number of other things. Not actually tested, but it definitely breaks *somewhere*.
I never said my solution was unbreakable, didn't even say it's good ... far from it ... I just said that the solution can be implemented there, but after your Sam&Max example i'm starting to doubt my way would work.
So on that note i'm for what's behind door number 3 ... the second version
$sql = $database->op(Database::BITAND, 'log_deleted', 1);
Aryeh Gregor wrote:
On Fri, Jun 12, 2009 at 10:02 AM, Leons Petrazickisleons.petrazickis@gmail.com wrote:
- Refactor the database to not use an integer as a bit field. Just
use four different boolean columns, which works well cross-database.
In MySQL, four different boolean columns means four times the storage, as compared to one TINYINT used as a bitfield. So this isn't a good solution.
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
These would be the way to do it, I guess. We do something similar for things like conditionals already. I think 2 is preferable to 3, stylistically.
On Fri, Jun 12, 2009 at 11:06 AM, Freako F.
Freakolowskyfreak@drajv.si wrote:
Oracle abstraction solves this problem in makeList function ... the only weak point for this solution is if you write SQL statements manualy, if you use Database class functions to create the actual SQL statement this works and as i was told on #mediawiki manual sql creation should gradually be rooted out.
This isn't a good solution:
foreach ($a as $key => $value) { if (strpos($key, ' & ') !== FALSE) $a2[preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)',$key)] = $value; elseif (strpos($key, ' | ') !== FALSE) $a2[preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $key)] = $value; elseif (!is_array($value)) { if (strpos($value, ' = ') !== FALSE) { if (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*?)\s=\s(.*)/', 'BITAND($1, $2) = $3', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*?)\s=\s(.*)/', 'BITOR($1, $2) = $3', $value); else $a2[$key] = $value; } elseif (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $value); else $a2[$key] = $value; }
It breaks on all sorts of possible input, like $dbr->select( 'revision', '*', 'rev_deleted&1' ) or $dbr->update( 'user', array( 'user_name' => 'Sam & Max'), $where ), or any number of other things. Not actually tested, but it definitely breaks *somewhere*.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
but on the other hand ... if you would just make sure that the bitwise comparison operation would always be in the "key" part of the array makeList solution would work ...
Freako F. Freakolowsky wrote:
I never said my solution was unbreakable, didn't even say it's good ... far from it ... I just said that the solution can be implemented there, but after your Sam&Max example i'm starting to doubt my way would work.
So on that note i'm for what's behind door number 3 ... the second version
$sql = $database->op(Database::BITAND, 'log_deleted', 1);
Aryeh Gregor wrote:
On Fri, Jun 12, 2009 at 10:02 AM, Leons Petrazickisleons.petrazickis@gmail.com wrote:
- Refactor the database to not use an integer as a bit field. Just
use four different boolean columns, which works well cross-database.
In MySQL, four different boolean columns means four times the storage, as compared to one TINYINT used as a bitfield. So this isn't a good solution.
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
These would be the way to do it, I guess. We do something similar for things like conditionals already. I think 2 is preferable to 3, stylistically.
On Fri, Jun 12, 2009 at 11:06 AM, Freako F.
Freakolowskyfreak@drajv.si wrote:
Oracle abstraction solves this problem in makeList function ... the only weak point for this solution is if you write SQL statements manualy, if you use Database class functions to create the actual SQL statement this works and as i was told on #mediawiki manual sql creation should gradually be rooted out.
This isn't a good solution:
foreach ($a as $key => $value) { if (strpos($key, ' & ') !== FALSE) $a2[preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)',$key)] = $value; elseif (strpos($key, ' | ') !== FALSE) $a2[preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $key)] = $value; elseif (!is_array($value)) { if (strpos($value, ' = ') !== FALSE) { if (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*?)\s=\s(.*)/', 'BITAND($1, $2) = $3', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*?)\s=\s(.*)/', 'BITOR($1, $2) = $3', $value); else $a2[$key] = $value; } elseif (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $value); else $a2[$key] = $value; }
It breaks on all sorts of possible input, like $dbr->select( 'revision', '*', 'rev_deleted&1' ) or $dbr->update( 'user', array( 'user_name' => 'Sam & Max'), $where ), or any number of other things. Not actually tested, but it definitely breaks *somewhere*.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Fri, Jun 12, 2009 at 2:22 PM, Freako F. Freakolowskyfreak@drajv.si wrote:
but on the other hand ... if you would just make sure that the bitwise comparison operation would always be in the "key" part of the array makeList solution would work ...
Having that kind of silent and fragile rule in place isn't good, because it's counterintuitive. People aren't going to realize, they'll just assume bitwise operators work and use them everywhere. If you have an explicit method, then the existence of the method serves as documentation of the fact that you need to use it.
Personally, I'd rather stay away from any regex-based SQL compatibility shims. I think they introduce tight-coupling to the system, and are by nature fragile.
Handling bitwise operation in a generic way is quite doable. The Database API already has similar abstraction for other things. I'll be happy to check in the addition to the Database class tomorrow June 13th.
Any more votes for which stylistic approach is preferable?
2. Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1); $sql = $database->bitor('log_deleted', 1); $sql = $database->bitxor('log_deleted', 1);
3. Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); $sql = $database->op('|', 'log_deleted', 1); $sql = $database->op('^', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1); $sql = $database->op(Database::BITOR, 'log_deleted', 1); $sql = $database->op(Database::BITXOR, 'log_deleted', 1);
So far we've had one vote for option 1 and two votes for option 2.
Regards,
Leons Petrazickis
On Fri, Jun 12, 2009 at 14:22, Freako F. Freakolowskyfreak@drajv.si wrote:
but on the other hand ... if you would just make sure that the bitwise comparison operation would always be in the "key" part of the array makeList solution would work ...
Freako F. Freakolowsky wrote:
I never said my solution was unbreakable, didn't even say it's good ... far from it ... I just said that the solution can be implemented there, but after your Sam&Max example i'm starting to doubt my way would work.
So on that note i'm for what's behind door number 3 ... the second version
$sql = $database->op(Database::BITAND, 'log_deleted', 1);
Aryeh Gregor wrote: > On Fri, Jun 12, 2009 at 10:02 AM, Leons > Petrazickisleons.petrazickis@gmail.com wrote: >> 1. Refactor the database to not use an integer as a bit field. Just >> use four different boolean columns, which works well cross-database. > > In MySQL, four different boolean columns means four times the storage, > as compared to one TINYINT used as a bitfield. So this isn't a good > solution. > >> 2. Add a function to the Database API for each bit operator. >> >> $sql = $database->bitand('log_deleted', 1); >> >> 3. Add a function to the Database API to handle all the operators. >> >> $sql = $database->op('&', 'log_deleted', 1); >> or >> $sql = $database->op(Database::BITAND, 'log_deleted', 1); > > These would be the way to do it, I guess. We do something similar for > things like conditionals already. I think 2 is preferable to 3, > stylistically. > > On Fri, Jun 12, 2009 at 11:06 AM, Freako F. Freakolowskyfreak@drajv.si wrote: >> Oracle abstraction solves this problem in makeList function ... the only >> weak point for this solution is if you write SQL statements manualy, if >> you use Database class functions to create the actual SQL statement this >> works and as i was told on #mediawiki manual sql creation should >> gradually be rooted out. > > This isn't a good solution: > > foreach ($a as $key => $value) { > if (strpos($key, ' & ') !== FALSE) > $a2[preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)', > $key)] = $value; > elseif (strpos($key, ' | ') !== FALSE) > $a2[preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', > $key)] = $value; > elseif (!is_array($value)) { > if (strpos($value, ' = ') !== FALSE) { > if (strpos($value, ' & ') !== FALSE) > $a2[$key] = > preg_replace('/(.*)\s&\s(.*?)\s=\s(.*)/', 'BITAND($1, $2) = $3', > $value); > elseif (strpos($value, ' | ') !== FALSE) > $a2[$key] = > preg_replace('/(.*)\s|\s(.*?)\s=\s(.*)/', 'BITOR($1, $2) = $3', > $value); > else $a2[$key] = $value; > } > elseif (strpos($value, ' & ') !== FALSE) > $a2[$key] = preg_replace('/(.*)\s&\s(.*)/', > 'BITAND($1, $2)', $value); > elseif (strpos($value, ' | ') !== FALSE) > $a2[$key] = preg_replace('/(.*)\s|\s(.*)/', > 'BITOR($1, $2)', $value); > else > $a2[$key] = $value; > } > > It breaks on all sorts of possible input, like $dbr->select( > 'revision', '*', 'rev_deleted&1' ) or $dbr->update( 'user', array( > 'user_name' => 'Sam & Max'), $where ), or any number of other things. > Not actually tested, but it definitely breaks *somewhere*. > > _______________________________________________ > Wikitech-l mailing list > Wikitech-l@lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/wikitech-l >
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
well anyway ... i'm having a working saturday tomorow, so just decide what form do you want and i'll go trough the source tomorow and fix the lot of it.
l8r
Leons Petrazickis wrote:
Personally, I'd rather stay away from any regex-based SQL compatibility shims. I think they introduce tight-coupling to the system, and are by nature fragile.
Handling bitwise operation in a generic way is quite doable. The Database API already has similar abstraction for other things. I'll be happy to check in the addition to the Database class tomorrow June 13th.
Any more votes for which stylistic approach is preferable?
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1); $sql = $database->bitor('log_deleted', 1); $sql = $database->bitxor('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); $sql = $database->op('|', 'log_deleted', 1); $sql = $database->op('^', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1); $sql = $database->op(Database::BITOR, 'log_deleted', 1); $sql = $database->op(Database::BITXOR, 'log_deleted', 1);
So far we've had one vote for option 1 and two votes for option 2.
Regards,
Leons Petrazickis
On Fri, Jun 12, 2009 at 14:22, Freako F. Freakolowskyfreak@drajv.si wrote:
but on the other hand ... if you would just make sure that the bitwise comparison operation would always be in the "key" part of the array makeList solution would work ...
Freako F. Freakolowsky wrote:
I never said my solution was unbreakable, didn't even say it's good ... far from it ... I just said that the solution can be implemented there, but after your Sam&Max example i'm starting to doubt my way would work.
So on that note i'm for what's behind door number 3 ... the second version
$sql = $database->op(Database::BITAND, 'log_deleted', 1);
Aryeh Gregor wrote:
On Fri, Jun 12, 2009 at 10:02 AM, Leons Petrazickisleons.petrazickis@gmail.com wrote:
- Refactor the database to not use an integer as a bit field. Just
use four different boolean columns, which works well cross-database.
In MySQL, four different boolean columns means four times the storage, as compared to one TINYINT used as a bitfield. So this isn't a good solution.
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
These would be the way to do it, I guess. We do something similar for things like conditionals already. I think 2 is preferable to 3, stylistically.
On Fri, Jun 12, 2009 at 11:06 AM, Freako F.
Freakolowskyfreak@drajv.si wrote:
Oracle abstraction solves this problem in makeList function ... the only weak point for this solution is if you write SQL statements manualy, if you use Database class functions to create the actual SQL statement this works and as i was told on #mediawiki manual sql creation should gradually be rooted out.
This isn't a good solution:
foreach ($a as $key => $value) { if (strpos($key, ' & ') !== FALSE) $a2[preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)',$key)] = $value; elseif (strpos($key, ' | ') !== FALSE) $a2[preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $key)] = $value; elseif (!is_array($value)) { if (strpos($value, ' = ') !== FALSE) { if (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*?)\s=\s(.*)/', 'BITAND($1, $2) = $3', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*?)\s=\s(.*)/', 'BITOR($1, $2) = $3', $value); else $a2[$key] = $value; } elseif (strpos($value, ' & ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s&\s(.*)/', 'BITAND($1, $2)', $value); elseif (strpos($value, ' | ') !== FALSE) $a2[$key] = preg_replace('/(.*)\s|\s(.*)/', 'BITOR($1, $2)', $value); else $a2[$key] = $value; }
It breaks on all sorts of possible input, like $dbr->select( 'revision', '*', 'rev_deleted&1' ) or $dbr->update( 'user', array( 'user_name' => 'Sam & Max'), $where ), or any number of other things. Not actually tested, but it definitely breaks *somewhere*.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Aryeh Gregor wrote:
On Fri, Jun 12, 2009 at 10:02 AM, Leons Petrazickisleons.petrazickis@gmail.com wrote:
- Refactor the database to not use an integer as a bit field. Just
use four different boolean columns, which works well cross-database.
In MySQL, four different boolean columns means four times the storage, as compared to one TINYINT used as a bitfield. So this isn't a good solution.
That's nonsense. We use a single TINYINT because it was introduced to the schema as an unused boolean, and Brion later had the bright idea of using it as a bitfield to expand its applications while avoiding another schema change. It wouldn't make a significant difference to database size if it were a text field with some complex ACL format, like the old page.page_restrictions, and it's quite possible we would have done that if the schema for it were designed from scratch.
However, we still want to avoid unnecessary schema changes to large tables.
- Add a function to the Database API for each bit operator.
$sql = $database->bitand('log_deleted', 1);
- Add a function to the Database API to handle all the operators.
$sql = $database->op('&', 'log_deleted', 1); or $sql = $database->op(Database::BITAND, 'log_deleted', 1);
These would be the way to do it, I guess. We do something similar for things like conditionals already. I think 2 is preferable to 3, stylistically.
We also have the same scheme for string concatenation.
-- Tim Starling
bitNot, bitAnd, bitOr functions implemented and replaced hardcoded operators in SQL code with function calls. Did some testing, but i never trust my own testing so, please have click or two.
l8r, Jure
wikitech-l@lists.wikimedia.org