I found several other mentions of errors related to:
Notice: Trying to get property of non-object in twitter_views_handler_field_formatted_tweet->render() (line 146 of /var/www/html/drupal2/sites/all/modules/twitter/twitter_views_field_handlers.inc).

They were supposedly fixed but I was still getting this message on my fresh install.

I tracked the issue down to a blank return from twitter_status_load in twitter.inc. I traced the code and found that for some reason this works:
Query SELECT * FROM twitter WHERE twitter_id = '362627144758722561'
but this doesn't:
Query SELECT * FROM twitter WHERE twitter_id = '362626066835177474'

If you manually do the query as:
Query SELECT * FROM twitter WHERE twitter_id = 362626066835177474
it works fine.

So I tried changing it to a like clause (duct tape band-aid) and it worked.
Query SELECT * FROM twitter WHERE twitter_id like '362626066835177474'

One solution is to cast it so that it's looking up the right data type and the conversion isn't handled by a possible unknown:

original twitter.inc:

function twitter_status_load($status_id) {
  return db_query("SELECT * FROM {twitter} WHERE twitter_id = :status_id",
           array(':status_id' => $status_id))->fetchObject();
}

one fix:

function twitter_status_load($status_id) {
  return db_query("SELECT * FROM {twitter} WHERE twitter_id = cast(:status_id as decimal(20,0))",
           array(':status_id' => $status_id))->fetchObject();
}
CommentFileSizeAuthor
#41 twitter-n2055951-41.patch1.35 KBDamienMcKenna
#38 twitter-n2055951-31.patch1.53 KBDamienMcKenna
#31 twitter-n1936598-31.patch1.14 KBDamienMcKenna
#23 twitter-2055951-switch-twitter_id-to-bigint-7.x-6.x.patch1.2 KBxurizaemon
#22 twitter-2055951-switch-twitter_id-to-bigint.patch1.15 KBxurizaemon
#11 switch-twitter_id-to-bigint-2055951.patch1.2 KBleewillis77
#5 twitter-2055951-5.patch998 bytesdrupal_jon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mrfreshly’s picture

mrfreshly CreditAttribution: mrfreshly commented

I've figured out the why.

MySQL is converting the 'value' to a float and losing some digits. These 20 digit ID values are all going to have to be either handled as numeric and not string OR will need to be cast to "decimal(20,0)" or "unsigned" in order for this to not happen. It is a known MySQL behavior.

See: http://dev.mysql.com/doc/refman/5.1/en/type-conversion.html

Comparisons that use floating-point numbers (or values that are converted to floating-point numbers) are approximate because such numbers are inexact. This might lead to results that appear inconsistent:

Such results can occur because the values are converted to floating-point numbers, which have only 53 bits of precision and are subject to rounding:

mrfreshly’s picture

mrfreshly CreditAttribution: mrfreshly commented
Status: Active » Needs review

It looks like there are only 2 queries that need to be updated. The 2nd one is in twitter_account_load in twitter.inc :

function twitter_account_load($id) {
  $values = db_query('SELECT *
                      FROM {twitter_account}
                      WHERE twitter_uid = cast( :id_1 as decimal(20,0))
                      OR screen_name  = :id_2',
                      array(':id_1' => $id, ':id_2' => $id))
              ->fetchAssoc();
  if (!empty($values)) {
    $values['id'] = $values['twitter_uid'];
    $account = new TwitterUser($values);
    $account->set_auth($values);
    $account->import = $values['import'];
    $account->mentions = $values['mentions'];
    $account->is_global = $values['is_global'];
    return $account;
  }
  return NULL;
}
masdongar’s picture

masdongar CreditAttribution: masdongar commented

Hi,

I have similar errors :

    Notice : Trying to get property of non-object dans twitter_views_handler_field_formatted_tweet->render() (ligne 146 dans ... twitter_views_field_handlers.inc).
    Notice : Undefined property: stdClass::$text dans twitter_views_handler_field_formatted_tweet->render() (ligne 157 dans ... twitter_views_field_handlers.inc).
    Notice : Trying to get property of non-object dans include() (ligne 10 dans ... tweet.tpl.php).
    Notice : Trying to get property of non-object dans include() (ligne 10 dans ... tweet.tpl.php).
    Notice : Trying to get property of non-object dans include() (ligne 11 dans ... tweet.tpl.php).
    Notice : Trying to get property of non-object dans include() (ligne 12 dans ... tweet.tpl.php).
    Notice : Trying to get property of non-object dans include() (ligne 21 dans ... tweet.tpl.php).
    Notice : Trying to get property of non-object dans include() (ligne 21 dans ... tweet.tpl.php).
    Notice : Trying to get property of non-object dans include() (ligne 25 dans ... tweet.tpl.php).
    Notice : Trying to get property of non-object dans include() (ligne 25 dans ... tweet.tpl.php).
    Notice : Undefined property: stdClass::$twitter_id dans include() (ligne 35 dans ... tweet.tpl.php).
    Notice : Undefined property: stdClass::$twitter_id dans include() (ligne 38 dans ... tweet.tpl.php).
    Notice : Undefined property: stdClass::$twitter_id dans include() (ligne 41 dans ... tweet.tpl.php).

Is it working for you now ?

mrfreshly’s picture

mrfreshly CreditAttribution: mrfreshly commented

Yes. It works fine now.

The problem is that randomly a twitter ID isn't being matched and that results in a NULL value getting passed back which is a "non-object". The warning is actually also telling you that you're also missing some data.

In MySQL the twitter ID's are being used in queries with single quotes around them and that is causing MySQL to convert them to floating point numbers.

This is because in the MySQL query:

  • 362626066835177474 does not equal '362626066835177474'.
  • The MySQL query turns '362626066835177474' into 3.62626066835178e+17.

A test query you can run to see for yourself (first select works without casting but the second one doesn't):

select  362627144758722561 = '362627144758722561', 
		362627144758722561 = cast('362627144758722561' as decimal(20,0));

select	362626066835177474 = '362626066835177474',
		362626066835177474 = cast('362626066835177474' as decimal(20,0));

You can tell MySQL to convert it to the correct type by saying cast('362626066835177474' as decimal(20,0)) which is what the twitter_id field is defined as in the database.

It was a fun bug to find. I've found 2 spots in the twitter code that need to be changed and it's a simple fix.

See the docs on at mysql : http://dev.mysql.com/doc/refman/5.1/en/type-conversion.html

drupal_jon’s picture

drupal_jon CreditAttribution: drupal_jon commented
FileSize
twitter-2055951-5.patch998 bytes

Here's a patch against 7.x-6.0-alpha2 using the fix from mrfreshly

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented
Version: 7.x-5.8 » 7.x-5.x-dev

Confirmed that this patch resolves the issue on MySQL. Thanks for tracking this down.

Is this MySQL workaround appropriate for other DBs, though? If not then we need to special case it, or find a solution which works for all.

Needs more testing before commit, sorry.

mrfreshly’s picture

mrfreshly CreditAttribution: mrfreshly commented

A little more on this issue:

http://www.php.net/manual/en/pdostatement.execute.php

An array of values with as many elements as there are bound parameters in the SQL statement being executed. All values are treated as PDO::PARAM_STR.

Apparently, using the Drupal (7.x) db_query() is going to treat any value passed to the query in an array as a string and let the database (MySQL/PostgresQL/SQLite) figure out what to do with it.

Any numeric field with a number bigger than 9007199254740992 (2^53) is going to have a problem with the current MySQL/Drupal implementation unless:

  • it is cast (see my previous example)
  • if the query is executed using bindparm's and PDO::PARAM_INT
  • the database field being matched is non-numeric (ie. store twitter_uid as varchar(22) instead of decimal(20,0) unsigned)

I'll have to dig through the core Drupal bug reports and see if anyone else has noticed this. I haven't had a chance to test the cast function on PostgreSQL or SQLite yet with a live Drupal install. Preliminary SQLite and PostgreSQL handled the cast in a select clause without a problem.

BTW: For those who don't know, to see these queries as MySQL is executing them enable general logging in the my.cnf file, restart mysql, watch the logs. You'll notice 99% of the query parameters are single quoted. Remember to turn it off when you're done or the log may consume your entire drive :).

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented

Thanks - would much appreciate you testing on those other platforms. Noticed that this is a dupe of the following issues -

* #1944226: Several notices "Trying to get property of non-object..." when calling standard tweet page
* #1880142: Integrity constraint violation: 1062 Duplicate entry

These patches are dupes too :)

#1944226-2: Several notices "Trying to get property of non-object..." when calling standard tweet page suggests this is MySQL version specific?

mrfreshly’s picture

mrfreshly CreditAttribution: mrfreshly commented

Re-quoting a few things to save you from digging ;)

The site I'm seeing the issue on is MySQL 5.1.69. I don't think it matters with the version because as of the most current MySQL 5.7 the MySQL documentation reports the exact same floating point number conversion assumption for all strings that I saw on MySQL 5.1:
http://dev.mysql.com/doc/refman/5.7/en/type-conversion.html (about half way down):

Comparisons that use floating-point numbers (or values that are converted to floating-point numbers) are approximate because such numbers are inexact. This might lead to results that appear inconsistent:

mysql> SELECT '18015376320243458' = 18015376320243458;
-> 1
mysql> SELECT '18015376320243459' = 18015376320243459;
-> 0
Such results can occur because the values are converted to floating-point numbers, which have only 53 bits of precision and are subject to rounding:

According to PHP:
http://www.php.net/manual/en/pdostatement.execute.php

An array of values with as many elements as there are bound parameters in the SQL statement being executed. All values are treated as PDO::PARAM_STR.

Drupal 7.x uses db_query as the primary database access call. "db_query" (found in includes/database.inc) uses arrays to pass the arguments. All the queries that go through db_query are going to get 'single quoted' values which will get float cast'ed unless you specifically cast it to another type.

This problem isn't limited to the Twitter module. It's a Drupal query issue with large numbers (greater than 53 bit) issue.

I even tried this in includes/database/mysql/database.inc:

PDO::ATTR_EMULATE_PREPARES => FALSE,

PDO query cache is functional in MySQL 5.1.17+ (pretty sure that was the number) so it should be safe to use now from a performance standpoint. Drupal is set to make PHP do the PDO emulation so older MySQL query cache works. (At least that's what I think the logic is)

This is what MySQL says it's doing:

                   13 Prepare   SELECT * FROM twitter WHERE twitter_id = ?
                   13 Execute   SELECT * FROM twitter WHERE twitter_id = '369897853797146624'

We end up with the same issue as the default PDO::ATTR_EMULATE_PREPARES => TRUE that Drupal has set. So PHP or MySQL PDO both behave this way. I was hoping that MySQL would be smart enough to match the types in the conversion since it has intimate knowledge of the field characteristics.

I'm kind of boggled at this issue...I'd think MySQL would look at the compare field to figure out what to cast to rather than just going float for everything. I'd also think PHP would have some way of conveying data types along with the array of PDO parameters.

I haven't had a change to test it on MySQL 5.5/5.6/5.7. According to the MySQL documentation it should behave the same way though. I will point out that this error doesn't show up 100% of the time with the Twitter data because sometimes it's close enough to match.

I hope this is helpful. I'll post my test results when I get a chance to setup a couple new Drupal instances with the other platforms.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented

Great summary, MUCH appreciated.

I'm pretty confident that the db_query/CAST approach will be fine on PgSQL / SQLite, I just don't have a test setup for them handy, and don't want to commit without someone checking that. Less concerned about MySQL versions tbh, I trust your reading of their docs :)

leewillis77’s picture

leewillis77 CreditAttribution: leewillis77 commented
FileSize
switch-twitter_id-to-bigint-2055951.patch1.2 KB

I've just landed here after experiencing a related issue.

IMHO - the real issue is that the module stores the twitter_id as a DECIMAL(20,0) for no good reason that I can see. The twitter ID is an INT (Ref: https://dev.twitter.com/docs/platform-objects/tweets) so we should be storing it as such - it will never need sub-int precision, and using a float value as a primary key will cause issues as identified in this thread.

The attached patch swaps twitter_id to a BIGINT which resolves the issues I had, and I believe should also resolve the issues identified in this thread. Note: While the twitter doc I referenced refers to "signed 64-bit INT", the linked discussion refers to "unsigned" 64-bit int, so I've gone with that since that provides a larger scope.

After this update has been run no changes to the SELECTs should be required, nor any casting.

Exploratus’s picture

Exploratus CreditAttribution: Exploratus commented

#5 worked for me.

leewillis77’s picture

leewillis77 CreditAttribution: leewillis77 commented

#5 may work, but it's really not the best solution. Is there any chance you can try the patch in #11 and confirm that works for you? (You'll need to run update.php after applying the patch - take a DB backup first just in case.)

michaelk’s picture

michaelk CreditAttribution: michaelk commented

#11 solved the issue for me

michaelk’s picture

michaelk CreditAttribution: michaelk commented
Issue summary: View changes

updated solution to a more appropriate one.

mrfreshly’s picture

mrfreshly CreditAttribution: mrfreshly commented

I agree, BIGINT is a more appropriate data type than DEC(20,0). You could also use CHAR(20) which would completely eliminate any type conversion issues at the expense of some extra bytes of storage but it would also cover Twitter switching to an alpha numeric ID if they ever do.

After seeing what Drupal/PHP are doing with the queries I like the combination of the two solutions. I really don't like the way PDO is passing everything as ::PARAM_STR. That's more work for the SQL server and more ambiguity.

I can confirm that on MySQL 5.1 with the test value of 362626066835177474:

  • BIGINT : works select count(1) from testtable where biginttest='362626066835177474'
  • CHAR(20) : works select count(1) from testtable where chartest='362626066835177474'
  • DEC(20,0) : fails select count(1) from testtable where dectest='362626066835177474'

Of course, if you cast the data it works for all:

  • BIGINT: works select count(1) from testtable where biginttest=cast('362626066835177474' as UNSIGNED );
  • CHAR(20): works select count(1) from testtable where chartest=cast('362626066835177474' as CHAR(20) );
  • DEC(20,0) : works select count(1) from testtable where dectest=cast('362626066835177474' as DEC(20,0) );
  • DEC(20,0) : also works select count(1) from testtable where dectest=cast('362626066835177474' as UNSIGNED );
modiphier’s picture

modiphier CreditAttribution: modiphier commented

#11 worked for me with 7.x-5.8 I also had a similar issue with a module called "drifter" and there was a similar patch the corrected error being shown. Tweets page is displaying ok now.

thanks

barryvdh’s picture

barryvdh CreditAttribution: barryvdh commented

After applying #11 and running update.php, my issues where also solved. Thanks!

intrafusion’s picture

intrafusion
Primary language English
Location Edinburgh, UK
CreditAttribution: intrafusion commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

#11 works for me too, can we get this added to the module?

atiba’s picture

atiba CreditAttribution: atiba commented

#11 solved the problem. thanks for the patch!

syawillim’s picture

syawillim CreditAttribution: syawillim commented

#11 solved the problem. No error messages. Thanks!

autopoietic’s picture

autopoietic CreditAttribution: autopoietic commented

#11 worked for me also

Applied to twitter-7.x-5.x(dev), update initially failed due to duplicate update id (twitter_update_7501), but on amending to twitter_update_7503 it worked fine.

also resolved theme issues mentioned here: https://drupal.org/node/1944226

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented
FileSize
twitter-2055951-switch-twitter_id-to-bigint.patch1.15 KB

Re-rolled with fix from #21.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented
FileSize
twitter-2055951-switch-twitter_id-to-bigint-7.x-6.x.patch1.2 KB

And here's a forward-patch to 7.x-6.x, BUT note that if you apply this you need to consider also updates 7501 and 7502 which aren't in this patch and won't get applied.

AFAICT 7.x-6.x is not the current 7.x branch to be on anyway, but a couple of hundred sites report usage of it. I've just opened #2205183: Clarify recommended download(s) for Twitter 7.x to ask this question.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented

Along the way, I think the {twitter}.twitter_id column has been stored as integers, strings, decimal/numeric, and now integer again.

I've committed this fix to 7.x-5.x-dev but it may need to be reconsidered with regard to 32-bit hosting environments. Hopefully that's less of a problem today than it was

Possible duplicates / related issues with big numbers - some wrt JSON & some DB storage. See next comment / issue description for related issues.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented
Parent issue: » #2205277: Twitpocalypse issues redux
xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
CreditAttribution: xurizaemon commented
Parent issue: #2205277: Twitpocalypse issues redux »
dgeo’s picture

dgeo CreditAttribution: dgeo commented

seems it just need a 5.x release now :)

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Reviewed & tested by the community » Needs review

Triggering testbot.

Status: Needs review » Needs work

The last submitted patch, 23: twitter-2055951-switch-twitter_id-to-bigint-7.x-6.x.patch, failed testing.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Parent issue: » #2402307: Plan for Twitter v7.x-5.9 release
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Version: 7.x-5.x-dev » 7.x-6.x-dev
Status: Needs work » Needs review
FileSize
twitter-n1936598-31.patch1.14 KB

Update 7501 comes from #2138341: Search links use V1.0 URL by default, which is no longer relevant, but update 7502 comes from #1936598: Add support for displaying tweet images and does need to be ported to the 7.x-6.x branch.

This patch is a rerolled version of #23 with minor adjustments to the formatting to match what I just changed in the 7.x-5.x branch, plus it has a placeholder for both of the other updates.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented

I'm rather alarmed by the statement in #24

I've committed this fix to 7.x-5.x-dev but it may need to be reconsidered with regard to 32-bit hosting environments. Hopefully that's less of a problem today than it was

I still use 32-bit and have spent quite a while contributing to fix "twitpocalypse" which even gets a mention on the project home page. Please don't publish a release that breaks 32-bit servers. I think there is a significant minority of these servers around.

@xurizaemon I'm willing to help test, but I can't see anywhere in this issue a link to exactly what was committed to the 5.x branch - please can you clarify?

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Version: 7.x-6.x-dev » 7.x-5.x-dev
Priority: Normal » Critical

@AdamPS: The last change on the twitter_id field was in commit 396a4983.

I wonder should we either a) officially drop compatibility for 32bit sites, b) go back to varchar()?

I'm switching this back to a critical issue and it's going to need to be re-done for 7.x-5.x.

Sorry everyone, but an integer field was the wrong way to go.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented

@DamienMcKenna Great thanks for the response

Before concluding this fix creates a problem, I'll test it out on 32-bit later today and let you know how it goes.

I tried to find some statistics on 32-bit vs 64-bit, but didn't find a lot of information. A search on a download site http://www.freewarefiles.com/search.php?query=fedora&B1.x=0&B1.y=0&boolean=exact showed Fedora Linux 3 times as many 32-bit as 64-bit (I picked that particular search as it was a fairly popular distro on the site and fairly recent). I don't claim this reflects the actual percentage, but it at least suggests 32-bit is still relevant. It's also worth bearing in mind that some servers - including mine - were installed several years ago when 32-bit was presumably more common.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Needs review » Needs work

@AdamPS: It's a very simple issue - Twitter changed to using 64bit integers a few years ago, so it's completely unrealistic to expect this should work on 32bit machines.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented

@DamienMcKenna Well there are plenty of techniques to handle 64-bit numbers on 32-bit machines - and indeed the module does currently work on my 32-bit machine.

I suppose that my time spent helping to solve "twitpocalypse" was wasted then. Perhaps you could remove the following text from the module page to save other people making the same mistake?

If you are still hosting on older versions of PHP or a 32-bit server, consider contributing towards ongoing support. Search for "twitpocalypse".

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Version: 7.x-5.x-dev » 7.x-6.x-dev
Priority: Critical » Normal
Status: Needs work » Needs review

Ok, I take it back - after reading through all of the other issues (#333788: Cannot insert/update unsigned integers or numbers bigger than an integer, #499254: BIGINT handling). According to them it should all work correctly on 32bit systems.

I've updated the project page to remove the comment, and I guess I'll need to get some new releases out soon :)

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
FileSize
twitter-n2055951-31.patch1.53 KB

An updated patch for the 7.x-6.x branch.

  • DamienMcKenna committed a157e5b on 7.x-6.x 
    Issue #2055951 by xurizaemon, leewillis77, drupal_jon, DamienMcKenna:...
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Needs review » Fixed

Committed.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Fixed » Needs review
FileSize
twitter-n2055951-41.patch1.35 KB

Rerolling for the 6x-5.x branch..

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Related issues: +#2402311: Plan for Twitter v6.x-5.2 release, +#2402309: Plan for Twitter v7.x-6.0 release

Status: Needs review » Needs work

The last submitted patch, 41: twitter-n2055951-41.patch, failed testing.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Version: 7.x-6.x-dev » 6.x-5.x-dev

Duh.

Status: Needs work » Needs review

DamienMcKenna queued 41: twitter-n2055951-41.patch for re-testing.

The last submitted patch, 11: switch-twitter_id-to-bigint-2055951.patch, failed testing.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Fixed

Committed.

  • DamienMcKenna committed 98344a4 on 6.x-5.x 
    Issue #2055951 by xurizaemon, leewillis77, drupal_jon, DamienMcKenna:...
AdamPS’s picture

AdamPS CreditAttribution: AdamPS commented

@DamienMcKenna Thanks for the reply. I confirm the fix is good for 32-bit.

I noticed that in_reply_to_user_id, in_reply_to_status_id, and twitter_account->twitter_uid are still using floats. It seems that these have the potential to cause other bugs of a similar nature. Should they be fixed too? Would you like me to open another issue?

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

@AdamPS: Thanks for looking into those. I've opened #2531874: Update all Twitter id fields to BIGINT to handle fixing other fields.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.