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

I don't think this is a bug.

You can decide how conflicts are resolved with the pgplsql parameter
variable_conflict:

CREATE OR REPLACE FUNCTION log_tst(id int, info text)
RETURNS void AS
$$
#variable_conflict use_column
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;

See the documentation:
https://www.postgresql.org/docs/current/plpgsql-implementation.html

Pantelis Theodosio

On Fri, Nov 30, 2018 at 8:18 PM PG Bug reporting form <
noreply@postgresql.org> wrote:

In my opinion this is a bug and what you are suggesting is a work around
for this particular case. what if we have a function where somewhere else
we need the variable first?

My point is that ON CONFLICT (<variable_name>) DO UPDATE is not a valid
construct an as such PostgreSQL should know that and try to use only the
column.

And if both can be used than it should be possible to use the full name
like log_tst.id or tst.id.

Lulzim

On Sat, Dec 1, 2018 at 12:05 AM Pantelis Theodosiou <ypercube@gmail.com>
wrote:

Hi

so 1. 12. 2018 v 12:02 odesílatel Lulzim Bilali <lulzimbilali@gmail.com>
napsal:

In my opinion this is a bug and what you are suggesting is a work around
for this particular case. what if we have a function where somewhere else
we need the variable first?

every time any possible conflict between SQL and plpgsql identifier is
terrible bad issue and it is a source of very hidden errors.

You can prefer SQL before plpgsql identifiers (like Oracle), or plpgsql
before SQL (like old Postgres) or raise error on conflict (current
Postgres).

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

You can use a) alias b) prefixes

so very safe is using

DECLARE _id int;
BEGIN
..
WHERE id = _id

or

<<blocklabel>>
DECLARE id int;
BEGIN
SELECT * FROM tab WHERE tab.id = blocklabel.id

or

CREATE OR REPLACE FUNCTION fname(id int)
...

BEGIN
SELECT * FROM tab WHERE tab.id = fname.id

So current behave is different than Oracle or old Postgres, but it is SAFE!
It doesn't block any necessary functionality, just it show any possible
issue.

Regards

Pavel

"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)

so 1. 12. 2018 v 12:32 odesílatel Andrew Gierth <andrew@tao11.riddles.org.uk>
napsal:

"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).

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

"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)