2

I want to convert this code in Postgres to something shorter that will do the same. I read about upsert but I couldn't understand a good way to implement that on my code. What I wrote works fine, but I want to find a more elegant way to write it. Hope someone here can help me! This is the query:

CREATE OR REPLACE FUNCTION insert_table(
    in_guid    character varying,
    in_x_value character varying,
    in_y_value character varying
)
RETURNS TABLE(response boolean) LANGUAGE 'plpgsql'

DECLARE _id integer;

BEGIN
    -- guid exists and it's been 10 minutes from created_date:
    IF ((SELECT COUNT (*) FROM public.tbl_client_location WHERE guid = in_guid AND created_date < NOW() - INTERVAL '10 MINUTE') > 0) THEN
        RETURN QUERY (SELECT FALSE);
    
    -- guid exists but 10 minutes hasen't passed yet:
    ELSEIF ((SELECT COUNT (*) FROM public.tbl_client_location WHERE guid = in_guid) > 0) THEN
    
        UPDATE
            public.tbl_client_location
        SET
            x_value = in_x_value,
            y_value = in_y_value,
            updated_date = now()
        WHERE
            guid = in_guid;

        RETURN QUERY (SELECT TRUE);
    
    -- guid not exist:
    ELSE
       
        INSERT INTO public.tbl_client_location
            ( guid   , x_value   , y_value    )
        VALUES
            ( in_guid, in_x_value, in_y_value )
        RETURNING id INTO _id;  

        RETURN QUERY (SELECT TRUE);

    END IF;
END
Dai
  • 141,631
  • 28
  • 261
  • 374
Ofir Sasson
  • 673
  • 4
  • 16
  • 39
  • For Upsert statement, You can check this link -https://www.postgresqltutorial.com/postgresql-tutorial/postgresql-upsert/ – Ankit Bajpai Aug 21 '22 at 11:00
  • It doesn't really help because it's not showing how to use multiple conditions and conditions that are depends on one another (I need to check first if there is already guid in the table and if so check the time so it's a condition inside a condition and I coulcn't write coditions inside on conflict – Ofir Sasson Aug 21 '22 at 11:04
  • `guid` being the primary key of table `tbl`? Please show the core table definition (`CREATE TABLE` script). Data types and constraints on involved columns are relevant. And always declare your version of Postgres. – Erwin Brandstetter Aug 21 '22 at 13:27
  • @OfirSasson Postgres' `INSERT INTO ... ON CONFLICT ...` is very limited in its expressiveness compared to ISO SQL's `MERGE` - from what I can tell you cannot elide your `SELECT COUNT(*)...` query, but you **do need** to wrap everything in a `TRANSACTION`, otherwise concurrent users could invalidate your `public.tbl_client_location` table between the `SELECT COUNT(*)` and the INSERT/UPDATE statement. – Dai Aug 21 '22 at 13:59
  • @Dai: Postgres 15 (currently beta) adds standard-conforming SQL `MERGE`. See: https://www.postgresql.org/docs/15/sql-merge.html But it's not needed for this case. Also, a function is wrapped into a transaction automatically, and we certainly don't need `count(*)` at all. – Erwin Brandstetter Aug 21 '22 at 14:38

2 Answers2

5

This can indeed be a lot simpler:

CREATE OR REPLACE FUNCTION insert_table(in_guid text
                                      , in_x_value text
                                      , in_y_value text
                                      , OUT response bool)  -- ④
  -- RETURNS record  -- optional noise  -- ④
  LANGUAGE plpgsql AS  -- ①
$func$  -- ②
-- DECLARE
   -- _id integer;  -- what for?
BEGIN
   INSERT INTO tbl AS t
          (   guid,    x_value,    y_value)
   VALUES (in_guid, in_x_value, in_y_value)
   ON CONFLICT (guid) DO UPDATE  -- guid exists
   SET    (         x_value,          y_value, updated_date)
        = (EXCLUDED.x_value, EXCLUDED.y_value, now())  -- ⑤
   WHERE  t.created_date >= now() - interval '10 minutes'  -- ③ have not passed yet
   -- RETURNING id INTO _id  -- what for?
   ;
   response := FOUND;  -- ⑥
END
$func$;

Assuming guid is defined UNIQUE or PRIMARY KEY, and created_date is defined NOT NULL DEFAULT now().

① Language name is an identifier - better without quotes.

② Quotes around function body were missing (invalid command). See:

UPDATE only if 10 min have not passed yet. Keep in mind that timestamps are those from the beginning of the respective transactions. So keep transactions short and simple. See:

④ A function with OUT parameter(s) and no RETURNS clause returns a single row (record) automatically. Your original was declared as set-returning function (0-n returned rows), which didn't make sense. See:

⑤ It's generally better to use the special EXCLUDED row than to spell out values again. See:

⑤ Also using short syntax for updating multiple columns. See:

⑥ To see whether a row was written use the special variable FOUND. Subtle difference: different from your original, you get true or false after the fact, saying that a row has actually been written (or not). In your original, the INSERT or UPDATE might still be skipped (without raising an exception) by a trigger or rule, and the function result would be misleading in this case. See:

Further reading:


You might just run the single SQL statement instead, providing your values once:

INSERT INTO tbl AS t(guid, x_value,y_value)
VALUES ($in_guid, $in_x_value, $in_y_value)  -- your values here, once
ON CONFLICT (guid) DO UPDATE
SET    (x_value,y_value, updated_date)
     = (EXCLUDED.x_value, EXCLUDED.y_value, now())
WHERE  t.created_date >= now() - interval '10 minutes';
Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
-1

I finally solved it. I made another function that'll be called and checked if it's already exists and the time and then I can do upsert without any problems. That's what I did at the end:

CREATE OR REPLACE FUNCTION fnc_check_table(
    in_guid character varying)
    RETURNS TABLE(response boolean) 
    LANGUAGE 'plpgsql'

    COST 100
    VOLATILE 
    ROWS 1000
AS $BODY$
BEGIN
IF EXISTS (SELECT FROM tbl WHERE guid = in_guid AND created_date < NOW() - INTERVAL '10 MINUTE' ) THEN
RETURN QUERY (SELECT FALSE);
ELSEIF EXISTS (SELECT FROM tbl WHERE guid = in_guid AND created_date > NOW() - INTERVAL '10 MINUTE') THEN
RETURN QUERY (SELECT TRUE);
ELSE
RETURN QUERY (SELECT TRUE);
END IF;

END
$BODY$;



CREATE OR REPLACE FUNCTION fnc_insert_table(
    in_guid character varying,
    in_x_value character varying,
    in_y_value character varying)
    RETURNS TABLE(response boolean) 
    LANGUAGE 'plpgsql'

    COST 100
    VOLATILE 
    ROWS 1000
AS $BODY$
BEGIN
IF (fnc_check_table(in_guid)) THEN
    INSERT INTO tbl (guid, x_value, y_value)
    VALUES (in_guid,in_x_value,in_y_value)
    ON CONFLICT (guid)
    DO UPDATE SET x_value=in_x_value, y_value=in_y_value, updated_date=now();
    RETURN QUERY (SELECT TRUE);
ELSE
    RETURN QUERY (SELECT FALSE);
END IF;
END
$BODY$;
Ofir Sasson
  • 673
  • 4
  • 16
  • 39