SQL join fails when screen_name case doesn't match
(User entered in different case to that stored on twitter.com)

added LOWER() to SQL so that join succeeds.

-  $sql = "SELECT ta.*, tu.uid, tu.password, tu.import FROM {twitter_user} tu LEFT JOIN {twitter_account} ta ON (tu.screen_name = ta.screen_name) WHERE tu.uid = %d";
+  $sql = "SELECT ta.*, tu.uid, tu.password, tu.import FROM {twitter_user} tu LEFT JOIN {twitter_account} ta ON (LOWER(tu.screen_name) = LOWER(ta.screen_name)) WHERE tu.uid = %d";
CommentFileSizeAuthor
#12 lowercase_screennames_3.patch1.37 KBShiny
#11 lowercase_screennames_2.patch1.37 KBShiny
#10 lowercase_screennames.patch853 bytesShiny
#5 Twitter-349639-5.patch449 bytesAnonymous (not verified)
twitter.inc_.patch685 bytesShiny

Comments

Shiny’s picture

Shiny commented
Status: Active » Needs review
rogermugs’s picture

rogermugs commented

great... this solved the problem for me.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

Over in #292173: Problems with different case in 'screen_name' (marked as a duplicate since this one has a patch), Eaton says:

I'll take a look -- I'd much prefer to simply lowercase the username whenever it's inserted into the database so it's consistent -- using LOWER() in queries can really kill performance, is it makes the use of indexes impossible.

So probably change this to use strtolower() on form submit, and create an update hook to lowercase in the database.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: blank entries in twitter account list » Twitter screen names are case sensitive

More descriptive title.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new Twitter-349639-5.patch449 bytes

Added strtolower() to the validation function, but didn't know how to do the update hook...
The bit I added seems to work ok.

Leeteq’s picture

Leeteq commented

Subscribing.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
Shiny’s picture

Shiny commented
Status: Needs review » Needs work

Patch on #7 causes existing accounts to not display. (including the lowercase ones)

Shiny’s picture

Shiny commented

tried testing against CVS branches instead of the tar ball - but errors
on 6--2 the hook_schema doesn't match up with the fields in selects
on 6--3 there's missing functions.

very keen to test and/or fix is someone can point me at where the latest is.

Shiny’s picture

Shiny commented
Status: Needs work » Needs review
StatusFileSize
new lowercase_screennames.patch853 bytes

Patch against twitter-6.x-2.6.tar.gz release

Lower cased the screen_name in 2 places
* Where inserted into {twitter_user}, from user input
* Where data returned by twitter.com is written to {twitter_account}

therefore the join across tables always works.

Shiny’s picture

Shiny commented
StatusFileSize
new lowercase_screennames_2.patch1.37 KB

found a 3rd place. -- patch revised to also include where screen_name is written to the {twitter} table.

Shiny’s picture

Shiny commented
StatusFileSize
new lowercase_screennames_3.patch1.37 KB

whoops - that diff was reversed. better now.

steinmb’s picture

steinmb commented
Version: 6.x-2.x-dev » 6.x-3.x-dev

Trying to clean up old bugs from the issue q. Reading up on the history of this issue and if I got this right was the start of it that twitter_get_user_accounts() used the screen name to perform a join (have not looked at the 6.x-2-code). This is not the case any more, right?

function twitter_get_user_accounts($uid) {
  $drupal_user = user_load($uid);
  return module_invoke_all('twitter_accounts', $drupal_user);
}

If my assumption is correct could we perhaps just close this?

steinmb’s picture

steinmb commented
Status: Needs review » Active
dddave’s picture

dddave commented
Status: Active » Closed (won't fix)

This branch is no longer supported. If this issue is still relevant feel free to reactivate it against relevant version.