Thread: BUG #15533: error on upsert when used in a fuction and a functionparameter has the same name as the column

The following bug has been logged on the website:

Bug reference:      15533
Logged by:          Lulzim Bilali
Email address:      lulzimbilali@gmail.com
PostgreSQL version: 11.1
Operating system:   Ubuntu 18.04
Description:

Can't use `ON CONFLICT DO UPDATE` in a function which has a parameter with
the same name as the column where the unique key is.

Here is the error I get.

Query execution failed

       Reason:
       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 log_tst(integer,text) line 4 at SQL
statement

the test code I'm using:


    --DROP TABLE IF EXISTS tst;
    CREATE TABLE tst (
     id int UNIQUE,
     info text
    );
    
    --DROP FUNCTION IF EXISTS log_tst;
    CREATE OR REPLACE FUNCTION log_tst(id int, info text) RETURNS void AS
    $$
    BEGIN
        
        INSERT INTO tst (id, info)
        VALUES (log_tst.id, log_tst.info)
        --ON CONFLICT DO NOTHING
        ON CONFLICT (id) DO UPDATE
        SET info = log_tst.info
    ;
    END $$
    LANGUAGE plpgsql;
    
    SELECT log_tst(1, 'changed');

I would expect it to work since we can't use a parameter to check the
uniqueness even if we want (or can we!?), so PostgreSQL should know to use
the column instead.

Lulzim

>>>>> "Pavel" == Pavel Stehule <pavel.stehule@gmail.com> writes:

 Pavel> I am strongly sure, so current default is best and any change of
 Pavel> this behave (it is simply - just use #option) is strongly wrong.

I don't buy it; I call this a bug.

Here's why: in an ON CONFLICT (col) clause, the (col) is not a list of
expressions or even really a list of columns, what it is is an index
definition (i.e. the same thing that would appear in CREATE INDEX). One
consequence of this is that _qualified_ column names, which are a usual
solution to variable name vs column conflicts, are not allowed here.
There is already special processing done on the clause for this reason
(the hiding of other tables that might be visible at this point in the
query), and I would say that this simply doesn't go far enough and that
parameters should be hidden too (by suppressing the columnref hooks
while the arbiter clause is being analyzed).

-- 
Andrew (irc:RhodiumToad)

>>>>> "Pavel" == Pavel Stehule <pavel.stehule@gmail.com> writes:

 Pavel> I am strongly sure, so current default is best and any change of
 Pavel> this behave (it is simply - just use #option) is strongly wrong.

 >> I don't buy it; I call this a bug.
 >> 
 >> Here's why: in an ON CONFLICT (col) clause, the (col) is not a list of
 >> expressions or even really a list of columns, what it is is an index
 >> definition (i.e. the same thing that would appear in CREATE INDEX). One
 >> consequence of this is that _qualified_ column names, which are a usual
 >> solution to variable name vs column conflicts, are not allowed here.
 >> There is already special processing done on the clause for this reason
 >> (the hiding of other tables that might be visible at this point in the
 >> query), and I would say that this simply doesn't go far enough and that
 >> parameters should be hidden too (by suppressing the columnref hooks
 >> while the arbiter clause is being analyzed).
 >> 

 Pavel> If there a expressions are not accepted there, then I can accept
 Pavel> your argumentation. Second hand is a implementation.

Expressions are allowed there on the same basis that they are allowed in
CREATE INDEX (i.e. with extra parens) - but they can still only refer to
columns of the specific table for which we're trying to identify an
arbiter index, not to anything whatsoever from the ordinary context of
the query.

If I were writing a patch to change this, I would expand on this comment:

        /*
         * While we process the arbiter expressions, accept only non-qualified
         * references to the target table. Hide any other relations.
         */

to say something like:

        /*
         * While we process the arbiter expressions, accept only non-qualified
         * references to the target table. Hide any other relations, and also
         * suppress the columnref hooks. This is justified by the fact that
         * these expressions and column references are not part of query
         * execution, but rather are a way to identify a constraint. As such,
         * the parser environment has no business trying to change the
         * interpretation of column names here (just as it would have no
         * business changing a column name list in the INSERT proper).
         */

-- 
Andrew (irc:RhodiumToad)