3

I've this function that inserts a row into a city table without duplicates. It returns the id of the inserted row:

CREATE OR REPLACE FUNCTION public.insert_city(
character varying,
character varying,
character varying,
character varying,
character varying,
character varying)
RETURNS integer AS
$BODY$
DECLARE
name_city1 ALIAS FOR $1;
country1 ALIAS FOR $2;
province1 ALIAS FOR $3;
region1 ALIAS FOR $4;
cap1 ALIAS FOR $5;
nationality1 ALIAS FOR $6;
id_city1 integer;
BEGIN
   INSERT INTO city (name_city, country, province, region, cap, nationality) 
   SELECT name_city1, country1, province1, region1, cap1, nationality1
WHERE NOT EXISTS (SELECT id_city FROM city WHERE name_city = name_city1)
RETURNING id_city INTO id_city1;

-- xxx

END;
$BODY$
LANGUAGE plpgsql VOLATILE;

xxx marks the spot where I need something like this:

IF is_number(id_city1) THEN
    RETURN id_city1;
ELSE
RETURN query select id_city from city where name_city=name_city1;
END IF;

If the first query does not insert a new row and I don't get an id_city from it, I want to execute the second query to select an existing id_city.

How can I do this?

Erwin Brandstetter
  • 479,275
  • 111
  • 893
  • 1,042
Andrea Perdicchia
  • 2,637
  • 1
  • 16
  • 19

3 Answers3

3

Your function can be simplified some more. More importantly, you can fix the built-in race condition:

CREATE OR REPLACE FUNCTION public.insert_city(name_city1   varchar
                                            , country1     varchar
                                            , province1    varchar
                                            , region1      varchar
                                            , cap1         varchar
                                            , nationality1 varchar)
  RETURNS integer AS
$func$
   WITH ins AS (
      INSERT INTO city
            (name_city , country , province , region , cap , nationality ) 
      VALUES(name_city1, country1, province1, region1, cap1, nationality1)
      ON     CONFLICT (name_city) DO UPDATE
      SET    name_city = NULL WHERE FALSE  -- never executed, but locks the row!
      RETURNING id_city
      )
   SELECT id_city FROM ins
   UNION  ALL
   SELECT id_city FROM city WHERE name_city = name_city1  -- only executed if no INSERT
   LIMIT  1;
$func$  LANGUAGE sql;

Major points

  • Assuming you run Postgres 9.5 or later, since you did not declare it.

  • Use the new faster UPSERT solution INSERT .. ON CONFLICT ...
    Detailed explanation:

  • You need a UNIQUE constraint on name_city for this.

  • About UNION ALL ... LIMIT 1:

  • Can be achieved with a single SQL command using a data-modifying CTE. This is least vulnerable to lock contention or other concurrency issues. It's shortest and fastest even without concurrent access.

  • The function can be a simpler SQL function. (But plpgsql isn't wrong or bad either.)

  • Don't abuse ALIAS FOR to attach names to parameters. That's explicitly discouraged in the manual. Use proper parameter names. The manual:

    It's best to use it only for the purpose of overriding predetermined names.

Community
  • 1
  • 1
Erwin Brandstetter
  • 479,275
  • 111
  • 893
  • 1,042
  • ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification SQL state: 42P10 Context: SQL function "insert_city" during startup – Andrea Perdicchia Oct 01 '16 at 17:03
1

Why not alter your function like so?:

Insert the existing id_city into id_city1. If one does not exist, it will be NULL. You can then perform the INSERT if it is NULL and assign the new id_city1. Finally return id_city1.

SELECT id_city INTO id_city1 FROM city WHERE name_city = name_city1;

IF id_city1 IS NULL THEN

    INSERT INTO city (name_city, country, province, region, cap, nationality) 
    VALUES (name_city1, country1, province1, region1, cap1, nationality1)
    RETURNING id_city INTO id_city1;

END IF;

RETURN id_city1;
Nick
  • 5,818
  • 1
  • 16
  • 34
  • There is a race condition. It's possible to run into a unique violation even if the first SELECT does not find a row, when a concurrent transaction commits in the meantime. There are faster solutions not exhibiting this problem. – Erwin Brandstetter Sep 30 '16 at 00:16
0

This is plpgsql version

CREATE OR REPLACE FUNCTION public.insert_city(name_city1   varchar
                                        , country1     varchar
                                        , province1    varchar
                                        , region1      varchar
                                        , zip1         varchar
                                        , nationality1 varchar,
                                        OUT id_city1 int)
  AS
 $func$
 BEGIN
    INSERT INTO city
        (name_city , country , province , region , zip , nationality ) 
    VALUES(name_city1, country1, province1, region1, zip1, nationality1)
    ON CONFLICT (name_city,zip) DO UPDATE
    SET    name_city = NULL WHERE FALSE  -- never executed, but locks the row!
    RETURNING id_city
    INTO id_city1;

    IF NOT FOUND THEN
        SELECT id_city
        FROM city
        WHERE name_city = name_city1
        INTO id_city1;
    END IF;
  END  $func$  LANGUAGE plpgsql;

There is a way that when the row exist don't increase the primary_key number (id_city in this case) ?

Andrea Perdicchia
  • 2,637
  • 1
  • 16
  • 19