Hi,

The column shorturl_link.lid should not be a serial (but an int).

The new values of 'serial' types are handled by the database, not by the programmer (so they should not be set to specific values, by code).

The INSERT statement in the stable version of short_url_shorten() ignores that and just overwrites lid anyway. But drupal_write_record() in the -dev version plays nice and adheres to the schema definition. Effect: even though you pass $link->lid = 3845, the shorturl_link.lid column will be 1. (And the next one 2, etc. Your $last_id values are totally ignored.)

And no recorded short URL will ever resolve, because they were recorded wrong.

Solution: simple.
Make shorturl_link.lid an int; that's what it is. (There's no requirement for primary keys to be serials.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

roderik’s picture

FileSize
1.51 KB

Actually (I should have checked this earlier...)

While you're busy, can you use the below patch instead of the one in my first message?

The length definition for your number columns does not do anything - except it makes schema.module complain about a "mismatch" between the definition and the actual database.

seanburlington’s picture

Yep - this bug completely breaks new installs of this module.

If you have upgraded from a previous version so that max(lid) is already greater than SHORTURL_START_FROM it will probably work

But otherwise all your links will be broken.

seanburlington’s picture

FileSize
828 bytes

Confirmed that with this change (making the lid column int not serial) a new install now works

I've re-rolled the patch as the above didn't work for me

However this may be down to my code as I've already incorporated the patch at #520588: Implementing tracking and metrics

roderik’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.37 KB

Maybe it's because my diffs contained some weird lines? In that case, apologies.

Otherwise the patch in #3 is the same as the one in the original message. Please use this one instead -- for the reasons outlined in #1.

irakli’s picture

Fixed in the commit: http://drupal.org/cvs?commit=380576

Many thanks, roderik.

irakli’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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