0

I'm new to working with PL/pgSQL, and I'm attempting to create a function that will either find the ID of an existing row, or will insert a new row if it is not found, and return the new ID.

The query contained in the function below works fine on its own, and the function gets created fine. However, when I try to run it, I get an error stating "ERROR: column reference "id" is ambiguous". Can anybody identify my problem, or suggest a more appropriate way to do this?

create or replace function sp_get_insert_company(
    in company_name varchar(100)
)
returns table (id int)
as $$
begin
    with s as (
        select 
            id
        from 
            companies
        where name = company_name
    ), 
    i as (
        insert into companies (name)
        select company_name 
        where not exists (select 1 from s)
        returning id
    )
    select id
    from i
    union all
    select id
    from s;
end;
$$ language plpgsql;

This is how I call the function:

select sp_get_insert_company('TEST')

And this is the error that I get:

SQL Error [42702]: ERROR: column reference "id" is ambiguous
Detail: It could refer to either a PL/pgSQL variable or a table column.
Where: PL/pgSQL function sp_get_insert_company(character varying) line 3 at SQL statement

RToyo
  • 2,792
  • 1
  • 12
  • 21
  • Hmm, I think PL SQL is asking you which `id` is being returned? `s.id`? `i.id`? – Jacob H May 21 '18 at 18:32
  • @JacobH I just tried the answer from sticky bit, to define which `id` was meant (`s.id` or `i.id`), and it didn't fix the problem. I don't think it should make a difference, since I'm `union`ing two separate queries. – RToyo May 21 '18 at 18:35
  • 1
    Try aliasing everything, including the CTE's (ex: `select c.id from companies c where c.name = company_name`) and see if that gets you any closer. – Jacob H May 21 '18 at 18:41

2 Answers2

1

As the messages says, id is in there twice. Once in the queries, once in the table definition of the return type. Somehow this clashes.

Try qualifying the column expressions, everywhere.

...
with s as (
    select 
        companies.id
    from 
        companies
    where name = company_name
), 
i as (
    insert into companies (name)
    select company_name 
    where not exists (select 1 from s)
    returning companies.id
)
select i.id
from i
union all
select s.id
from s;
...

By qualifying the column expression the DBMS does no longer confuse id of a table with the id in the return type definition.

The next problem will be, that your SELECT has no target. It will tell you to do a PERFORM instead. But I assume you want to return the results. Change the body to

...
RETURN QUERY (
with s as (
    select 
        companies.id
    from 
        companies
    where name = company_name
), 
i as (
    insert into companies (name)
    select company_name 
    where not exists (select 1 from s)
    returning companies.id
)
select i.id
from i
union all
select s.id
from s);
...

to do so.

sticky bit
  • 31,711
  • 12
  • 26
  • 38
  • I just tried that, and it didn't make a difference. I would be surprised if it did, since `union` takes two separate queries and unions the results together. There should be no ambiguity there. – RToyo May 21 '18 at 18:34
  • @RToyo. Hmm, you're right. Let me think again. Sorry. – sticky bit May 21 '18 at 18:35
  • @RToyo: The reason is an other then I initially and mistakenly assumed. But the solution is the same. – sticky bit May 21 '18 at 18:46
  • Thank you, that did it! And thank you for pointing out that I needed a `return query`. – RToyo May 21 '18 at 20:12
1

In the function you display there is no need for returns table (id int). It's supposed to always return exactly one integer ID. Simplify to RETURNS int. This also makes ERROR: column reference "id" is ambiguous go away, since we implicitly removed the OUT parameter id (visible in the whole function block).

There is also no need for LANGUAGE plpgsql. Could simply be LANGUAGE sql, then you wouldn't need to add RETURNS QUERY, either. So:

CREATE OR REPLACE FUNCTION sp_get_insert_company(_company_name text)
  RETURNS int AS
$func$
   WITH s as (
     select c.id  -- still good style to table-qualify all columns
     from   companies c
     where  c.name = _company_name
   ), 
   i as (
     insert into companies (name)
     select _company_name
     where  not exists (select 1 from s)
     returning id
   )
   select s.id from s 
   union all
   select i.id from i
   LIMIT 1;  -- to optimize performance
$func$  LANGUAGE sql;

Except that it still suffers from concurrency issues. Find a proper solution for your undisclosed version of Postgres in this closely related answer:

Erwin Brandstetter
  • 479,275
  • 111
  • 893
  • 1,042
  • Thank you for the extended explanation Erwin. I initially wrote the function as a simple TSQL proc, and started adding tossing in extra definitions to try get it to work. And thank you for pointing out the problem with `select or insert`; I'll read up on that once I'm home, and start making some updates to my code. – RToyo May 22 '18 at 13:57