On Thu, Jun 30, 2022 at 1:43 PM Robert Haas <robertmh...@gmail.com> wrote:

> On Thu, Jun 9, 2022 at 11:40 AM David G. Johnston
> <david.g.johns...@gmail.com> wrote:
> >     As one cannot place excluded in a FROM clause (subquery) in the
> >     ON CONFLICT clause referring to it as a table, ...
>
> Well, it would be nice if you had included a test case rather than
> leaving it to the reviewer or committer to construct one.  In general,
> dropping subtle patches with minimal commentary isn't really very
> helpful.
>

Fair point.

>
> But I decided to dig in and see what I could figure out. I constructed
> this test case first, which does work:
>
> rhaas=# create table foo (a int primary key, b text);
> CREATE TABLE
> rhaas=# insert into foo values (1, 'blarg');
> INSERT 0 1
> rhaas=# insert into foo values (1, 'frob') on conflict (a) do update
> set b = (select excluded.b || 'nitz');
> INSERT 0 1
> rhaas=# select * from foo;
>  a |    b
> ---+----------
>  1 | frobnitz
> (1 row)
>
> Initially I thought that was the case you were talking about, but
> after staring at your email for another 20 minutes, I figured out that
> you're probably talking about something more like this, which doesn't
> work:
>
> rhaas=# insert into foo values (1, 'frob') on conflict (a) do update
> set b = (select b || 'nitz' from excluded);
> ERROR:  relation "excluded" does not exist
> LINE 1: ...ct (a) do update set b = (select b || 'nitz' from excluded);
>

Right, the word "excluded" appearing immediately after the word FROM is
what I meant by:

"As one cannot place excluded in a FROM clause (subquery) in the
    ON CONFLICT clause"

It is clear that, at the level of the code,
> "excluded" behaves like a pseudo-table,


And people in the code are capable of understanding this without difficulty
no matter how we write it.  They are not the target audience.


> but I
> think that the threshold to commit a patch like this is that the
> change has to be a clear improvement, and I don't think it is.
>

I'm hoping for "more clear and accurate without making things worse"...

The fact that it does not and cannot use FROM and that it never refers to
more than a single row (which is what motivated the change in the first
place) for me make using the word table here more trouble than it is worth.


>
> I think it might be fruitful to consider whether some of the error
> messages here could be improved


Possibly...


> or even whether some of the
> non-working cases could be made to work,


That would, IMO, make things worse.  "excluded" isn't a table in that
sense, anymore than "NEW" and "OLD" in the context of triggers.

but I'm just not really
> seeing the value of tinkering with documentation which is, in my view,
> not wrong.
>
>
Current:
"The SET and WHERE clauses in ON CONFLICT DO UPDATE have access to the
existing row using the table's name (or an alias), and to [rows] proposed
for insertion using the special excluded table."

The word table in that sentence is wrong and not a useful way to think of
the thing which we've named excluded.  It is a single value of a composite
type having the structure of the named table.

I'll agree that most people will mentally paper over the difference and go
merrily on their way.  At least one person recently did not do that, which
prompted an email to the community, which prompted a response and this
suggestion to avoid that in the future while, IMO, not making understanding
of the concept any less clear.

David J.

Reply via email to