1

I have a SQL function that is supposed to return 0 or 1 depending on whether a user activation was successful or not. I have the following two tables I need to interact with:

users {user_id, unique email, ...}
user_activation {activation_hash, unique email, ...}

The function is supposed to evaluate:

  1. Does the incoming hash match a row in user_activation?
  2. And does the corresponding email not already exist in the users table?
  3. Then insert a new user into users and delete the activation row and return 1, else return 0

Here is my function:

delimiter #
create function activate_user
(
    p_activation_hash char(32)
)
returns int
deterministic
begin

    if not exists (
        select 1 from users u
        inner join (
            select email from user_activation where activation_hash = p_activation_hash
        ) ua
        on u.email = ua.email
    )
  then

    -- hash exists but email doesnt so add
    insert into users (email, password_hash, first_name, last_name, company_name)
      select email, password_hash, first_name, last_name, company_name
      from user_activation
      where activation_hash = p_activation_hash
      and expiry_date > now();

    -- delete the activation row(s)
    delete low_priority from user_activation where activation_hash = p_activation_hash;

    return 1;

  end if;

  return 0;

end #
delimiter ;

My problem is that the conditional always evaluates to true (although only 1 row is ever inserted into the user table even without the unique keyword).

Thanks.

Dominic
  • 62,658
  • 20
  • 139
  • 163

2 Answers2

1

Try changing the definition from DETERMINISTIC to NOT DETERMINISTIC (or remove it since NOT is default) since the result of that function is not the same each time for a given input.

Once a (valid) hash is used once, the function returns a different value for that hash. You are probably seeing the same result over and over because the first time you called that function it returned 1 and now each time you call it it is returning that same value even though the activation record no longer exists. The input for a hash may be invalid one moment, and then valid the next (unlikely, however).

See CREATE PROCEDURE Syntax for more details.

drew010
  • 68,777
  • 11
  • 134
  • 162
  • Thanks a lot for the answer, I tried not deterministic but unfortunately it still didn't work. I read up a little on this and realised functions mostly don't modify sql and declaring not deterministic meant I had to set log_bin_trust_function_creators to 1. So to circumvent this I converted it to a stored procedure and tested the functionality to be working but sadly the conditional still is always evaluating to true. – Dominic Sep 11 '12 at 23:49
0

I revisited this problem today and found a very handy NOT IN query which did the trick:

delimiter #
create procedure activate_user
(
    in p_activation_hash char(32),
    inout status int
)
proc_main:begin

    set status = 0;

    if exists (
        select 1 from user_activation
            where activation_hash = p_activation_hash
            and email not in (select email from users)
    )
    then

    -- hash exists but email doesnt so add
    insert into users (email, password_hash, first_name, last_name, company_name)
      select email, password_hash, first_name, last_name, company_name
      from user_activation
      where activation_hash = p_activation_hash
      and expiry_date > now();

    -- delete the activation row(s)
    delete low_priority from user_activation where activation_hash = p_activation_hash;

    set status = 1;

  end if;

end proc_main #
delimiter ;
Dominic
  • 62,658
  • 20
  • 139
  • 163