DB2 only support "column LIKE column" or "column LIKE text", but not "text LIKE column". this patch fix this minor problem based on cross database compatibility concern.

CommentFileSizeAuthor
#9 rollback_2.patch735 byteschx
drupal-6.x-dev-like-0.1.diff1.64 KBhswong3i

Comments

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks! :)

Rok Žlender’s picture

Rok Žlender commented
Priority: Normal » Critical
Status: Fixed » Active

I think this patch broke at least user access rules on mysql. Now masks like '%block%' do not work anymore. If you switch back to "text LIKE column" it works again.

I tested this on mysql 5.0.19

Example:
mask = %block%

SELECT * FROM `access` WHERE mask LIKE 'tblockt'
returns 0 results

SELECT * FROM `access` WHERE 'tblockt' LIKE mask
returns the correct result

hswong3i’s picture

hswong3i commented

BTW, when i test with following SQL, on MySQL 5.0.32-Debian_7etch1, it works fine:

mysql> SELECT * FROM `term_data` WHERE LOWER('%') LIKE LOWER(name);
Empty set (0.00 sec)

mysql> SELECT * FROM `term_data` WHERE LOWER(name) LIKE LOWER('%');
...
...
52 rows in set (0.00 sec)

i test it with Oracle 10gR2, and the result is similar: case 1 comes with empty result set, where case 2 returns values. so at least, it is not an issue with my MySQL server; on the other hand, it is also not an issue for Oracle. as this patch is mainly target for DB2, so surly it is also functioning with it :)

hswong3i’s picture

hswong3i commented
Assigned: Unassigned » hswong3i
Status: Active » Fixed

@Rok Žlender: as there is no further update from you, and this patch is fully tested in both MySQL/Oracle/DB2/MSSQL, i will assume this issue as closed. if you still face some other problems, please feel free to attach more information, and i will try my best to repeat your case for testing :)

hswong3i’s picture

hswong3i commented
Status: Fixed » Closed (duplicate)

@Zlender: thanks for your bug report, and i will solve that bugs within other issue: http://drupal.org/node/168403, and so this issue will make as duplicated :)

mooffie’s picture

mooffie commented

>
> SELECT t.tid, t.* FROM {term_data} t WHERE WHERE LOWER('%s') LIKE LOWER(t.name)
>

Isn't this weird? Why not simply do WHERE LOWER('%s') = LOWER(t.name)?

I tried to trace the history of this line[1]. It seems that in those days the taxonomy module was quite different than what it is today[2] and the use of LIKE invaded this line as well. (Do you want me to prepare a patch that removes this LIKE?)

[1] http://cvs.drupal.org/viewvc.py/drupal/drupal/modules/taxonomy/taxonomy....

[2] http://cvs.drupal.org/viewvc.py/drupal/drupal/modules/taxonomy/taxonomy....

hswong3i’s picture

hswong3i commented

@mooffie: sorry that i can't find the line that you point out, from latest CVS HEAD. would you mind to have a double check?

mooffie’s picture

mooffie commented

>
> i can't find the line that you point out

I quoted this line from your patch. It's in one of the two lines that you've changed.

(I didn't criticize your code but the code of our "forefathers" :-)

chx’s picture

chx commented
Title: update LIKE as SQL friendly » Beta breaker: access rules are broken.
Status: Closed (duplicate) » Reviewed & tested by the community
StatusFileSize
new rollback_2.patch735 bytes

The must be rolled back as it broke MySQL. @hswong3i: replacing a trivial but critical bugfix with a complicated feature request is not a duplicate!

hswong3i’s picture

hswong3i commented

@chx: thanks about this patch. as i am now working for same thing, you save my time for duplicate work, many thanks :)

P.S. i can SIMPLIFY that implementation (http://drupal.org/node/168403) into VERY simple, or even lossy than current implementation. but will you accept for something like that? i don't really think so. i think we will not accept something that don't give any improvement, and that's why i try to extend it. and the result is not too bad: feature is extended, with no performance drop, and cross database compatible.

so why not accepting it? surly because it is something too late. i will postpone that extension into D7, so would you feel much better about that?

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)