Better way to write the PostgreSQl function

Question

This is a stored procedure I have converted from MS SQLServer to PostgreSQL. Is there any better way to write it in Postgres. Also what should be the best practices? Any help will be appreciated.

create or replace function GenSerialNo_SP1(tname character varying) returns AS $$

--declare @PK bigint
declare totalRec bigint;
declare getRec bigint;
Begin

    --set @PK = 1
    set totalRec=-1;

    for getRec inselect coalesce(primary_key,0) from STD_SERIAL_NOS 
   where table_name = tname
    loop
    open  getRec;
    fetch next from getRec into totalRec;
    close getRec;
    deallocate getRec;
    end loop;
    if totalRec = -1 then

        insert into STD_SERIAL_NOS (TABLE_NAME, PRIMARY_KEY) values (tname, 1);

    else

        update STD_SERIAL_NOS set primary_key = primary_key +1 
           where table_name = tname;

    end if;

end;
$$ language plpgsql;

What are you trying to achieve? Why aren't you just using a `SEQUENCE`? Are you looking for a gapless sequence? — Craig Ringer, Dec 05 '12 at 12:16

score 1 · Answer 1 · answered Dec 05 '12 at 09:58

1

Your approach is fundamentally flawed, in that the read consistency mechanism of the database could cause multiple simultaneous processes to read the same values from STD_SERIAL_NOS and try to insert/update those values.

The correct way to generate key values in Postgres (or Oracle) is to use a sequence object, and maybe a trigger to autopopulate the primary key column.

Come to think of it, the method you are using is probably flawed in any database system.

answered Dec 05 '12 at 09:58

David Aldridge

51,479
8
68
96

It was fundamentally flawed whatever RDBMS is was first implemented in. – David Aldridge Dec 05 '12 at 13:03
@DavidAldridge: not necessarily flawed in SQL Server. In a default installation readers and writers will block each other. So as soon as the select from the cursor is executed any update to that data will be blocked until the transaction is committed. – Dec 05 '12 at 15:49
@a_horse_with_no_name ah well in that case I'd suggest that the default installation of SQL Server is fundamentally flawed – David Aldridge Dec 05 '12 at 15:50

score 1 · Accepted Answer · edited May 23 '17 at 11:48

It looks like an UPSERT, but you don't need any cursor achieve that.

Assuming there is a unique index on STD_SERIAL_NOS.table_name, a (hopefully) correct version that includes dealing with the race condition on insert would be:

create or replace function GenSerialNo_SP1(tname character varying) returns void
as $$
begin
  loop
   begin
     update STD_SERIAL_NOS set primary_key = primary_key +1 
        where table_name = tname;
     exit when found; -- exit loop if the row exists, otherwise insert it
     insert into STD_SERIAL_NOS (TABLE_NAME, PRIMARY_KEY)
        values(tname,1);
   exception  WHEN unique_violation THEN
     -- the insert conflicts, let's go back to the update in the next loop iteration
   end;
  end loop;

end;
$$ language plpgsql;

Better way to write the PostgreSQl function

2 Answers2