BUG #17233: Incorrect behavior of DELETE command with bad subquery in WHERE clause

On Saturday, October 16, 2021, PG Bug reporting form <noreply@postgresql.org>
wrote:

The following bug has been logged on the website:

Bug reference: 17233
Logged by: Alexander Korolev
Email address: lxndrkrlv@gmail.com
PostgreSQL version: 14.0
Operating system: Windows
Description:

This SELECT command fails as expected:
SELECT CTID FROM tmp1 INNER JOIN tmp2 ON tmp1.id = tmp2.id FOR UPDATE;
-- ERROR: column "ctid" does not exist.

But if I use same SELECT in WHERE clause of DELETE command
DELETE FROM tmp1 WHERE CTID in (
SELECT CTID FROM tmp1 INNER JOIN tmp2 ON tmp1.id = tmp2.id FOR
UPDATE);
this command is executed without errors.

This is not a bug:

https://wiki.postgresql.org/wiki/FAQ#Why_doesn.27t_PostgreSQL_report_a_column_not_found_error_when_using_the_wrong_name_in_a_subquery.3F

The virtual join table doesn’t have a ctid, only physical tables do, and
the ctid of physical tables apparently aren’t propogated when they are
joined.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

The virtual join table doesn’t have a ctid, only physical tables do, and
the ctid of physical tables apparently aren’t propogated when they are
joined.

We leave the system columns out of the join because otherwise they'd
surely conflict between the two sides of the join. However, you could
still reference either one with "tmp1.ctid" or "tmp2.ctid".

There might be an opportunity here to improve the error message's hint:

regression=# SELECT CTID
regression-# FROM tmp1
regression-# INNER JOIN tmp2 ON tmp1.id = tmp2.id;
ERROR: column "ctid" does not exist
LINE 1: SELECT CTID
^
HINT: There is a column named "ctid" in table "tmp1", but it cannot be referenced from this part of the query.

"cannot be referenced" is probably a shade misleading, given the
availability of the qualified-name alternative.

regards, tom lane

On Sunday, October 17, 2021, Tom Lane <tgl@sss.pgh.pa.us> wrote:

There might be an opportunity here to improve the error message's hint:

regression=# SELECT CTID
regression-# FROM tmp1
regression-# INNER JOIN tmp2 ON tmp1.id = tmp2.id;
ERROR: column "ctid" does not exist
LINE 1: SELECT CTID
^
HINT: There is a column named "ctid" in table "tmp1", but it cannot be
referenced from this part of the query.

"cannot be referenced" is probably a shade misleading, given the
availability of the qualified-name alternative.

I was actually wondering why the error wasn’t an ambiguous column name
error. For a pure select query we already allow duplicate column names in
the result.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

I was actually wondering why the error wasn’t an ambiguous column name
error. For a pure select query we already allow duplicate column names in
the result.

To get an "ambiguous column name" error, there'd need to be multiple
*accessible* names, not multiple inaccessible ones. Although I concede
your point that maybe we could adjust the "does not exist" phraseology
too. Maybe something like "column foo is not available"?

regards, tom lane

This is not a bug:

https://wiki.postgresql.org/wiki/FAQ#Why_doesn.27t_PostgreSQL_report_a_column_not_found_error_when_using_the_wrong_name_in_a_subquery.3F

The virtual join table doesn’t have a ctid, only physical tables do, and
the ctid of physical tables apparently aren’t propogated when they are
joined.

Possibly this is not a bug, but this behavior is strange.
Also, this subquery has different behavior in SELECT and DELETE:

--------------------------------------------------------------------------------------------------
DROP TABLE IF EXISTS pg_temp.tmp1;
DROP TABLE IF EXISTS pg_temp.tmp2;

CREATE TEMPORARY TABLE tmp1 (id int NOT NULL, name text);
CREATE TEMPORARY TABLE tmp2 (id int NOT NULL, name text);

INSERT INTO tmp1 (id, name) VALUES (1, 'aaa'), (2, 'bbb'), (3, 'ccc');
INSERT INTO tmp2 (id, name) VALUES (1, 'aaa');

-- select outputs all rows from tmp1
SELECT * FROM tmp1
WHERE CTID in (
SELECT CTID
FROM tmp1
INNER JOIN tmp2 ON tmp1.id = tmp2.id FOR UPDATE
);

--delete affects only first row from tmp1
DELETE FROM tmp1
WHERE CTID in (
SELECT CTID
FROM tmp1
INNER JOIN tmp2 ON tmp1.id = tmp2.id FOR UPDATE
) RETURNING *;
--------------------------------------------------------------------------------------------------

On Sunday, October 17, 2021, Александр Королев <lxndrkrlv@gmail.com> wrote:

This is not a bug:

https://wiki.postgresql.org/wiki/FAQ#Why_doesn.27t_
PostgreSQL_report_a_column_not_found_error_when_using_
the_wrong_name_in_a_subquery.3F

The virtual join table doesn’t have a ctid, only physical tables do, and
the ctid of physical tables apparently aren’t propogated when they are
joined.

Possibly this is not a bug, but this behavior is strange.
Also, this subquery has different behavior in SELECT and DELETE:

-- select outputs all rows from tmp1
SELECT * FROM tmp1
WHERE CTID in (
SELECT CTID
FROM tmp1
INNER JOIN tmp2 ON tmp1.id = tmp2.id FOR UPDATE
);

As long as the subquery returns at least one row every row in the table
will be returned.

--delete affects only first row from tmp1
DELETE FROM tmp1
WHERE CTID in (
SELECT CTID
FROM tmp1
INNER JOIN tmp2 ON tmp1.id = tmp2.id FOR UPDATE
) RETURNING *;

Here, as soon as you delete the single row that the subquery returns no
additional rows will be deleted. It seems indeterminate as to how many,
and which, rows actually get removed. At least one, but possibly all. The
is more procedural an execution plan than I would expect from SQL but it’s
all that seems to fit the described behavior.

In short, your subquery is basically bogus and so, yes, you will see
strange behavior if you use it.

The server cannot always inform you that you’ve written something bogus
(i.e., error) because the same general query form can be used to write
something useful. Correlated subqueries are one of those cases.

David J.

On Sun, Oct 17, 2021 at 10:34:18AM -0400, Tom Lane wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

The virtual join table doesn’t have a ctid, only physical tables do, and
the ctid of physical tables apparently aren’t propogated when they are
joined.

We leave the system columns out of the join because otherwise they'd
surely conflict between the two sides of the join. However, you could
still reference either one with "tmp1.ctid" or "tmp2.ctid".

There might be an opportunity here to improve the error message's hint:

regression=# SELECT CTID
regression-# FROM tmp1
regression-# INNER JOIN tmp2 ON tmp1.id = tmp2.id;
ERROR: column "ctid" does not exist
LINE 1: SELECT CTID
^
HINT: There is a column named "ctid" in table "tmp1", but it cannot be referenced from this part of the query.

"cannot be referenced" is probably a shade misleading, given the
availability of the qualified-name alternative.

I looked into this by modifying the error message with the attached
patch and running the regression tests. I saw the following regression
failures due to the message change. While the email posted query was
fixed by table-qualifying the column, the first few queries of the
regression tests were fixed by adding LATERAL, but I couldn't get the
UPDATE/DELETE queries to work.

I am feeling there isn't much we can add to this message except to say
maybe:

There is a column named "f1" in table "a", but it cannot be referenced
from this part of the query as structured.
-------------

It suggests you might be able to get it working by restructuring the
query, e.g., table-qualified or LATERAL.

Feedback?

---------------------------------------------------------------------------

 -- test some error cases where LATERAL should have been used but wasn't
 select f1,g from int4_tbl a, (select f1 as g) ss;
 ERROR:  column "f1" does not exist
 LINE 1: select f1,g from int4_tbl a, (select f1 as g) ss;
                                              ^
-HINT:  There is a column named "f1" in table "a", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "f1" in table "a" and another table so a table-qualified column reference is required.

select f1,g from int4_tbl a, (select a.f1 as g) ss;
ERROR: invalid reference to FROM-clause entry for table "a"
LINE 1: select f1,g from int4_tbl a, (select a.f1 as g) ss;
^
HINT: There is an entry for table "a", but it cannot be referenced from this part of the query.

 select f1,g from int4_tbl a cross join (select f1 as g) ss;
 ERROR:  column "f1" does not exist
 LINE 1: select f1,g from int4_tbl a cross join (select f1 as g) ss;
                                                        ^
-HINT:  There is a column named "f1" in table "a", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "f1" in table "a" and another table so a table-qualified column reference is required.

select f1,g from int4_tbl a cross join (select a.f1 as g) ss;
ERROR: invalid reference to FROM-clause entry for table "a"
LINE 1: select f1,g from int4_tbl a cross join (select a.f1 as g) ss...
^
HINT: There is an entry for table "a", but it cannot be referenced from this part of the query.

-- check behavior of LATERAL in UPDATE/DELETE
create temp table xx1 as select f1 as x1, -f1 as x2 from int4_tbl;
-- error, can't do this:

 update xx1 set x2 = f1 from (select * from int4_tbl where f1 = x1) ss;
 ERROR:  column "x1" does not exist
 LINE 1: ... set x2 = f1 from (select * from int4_tbl where f1 = x1) ss;
                                                                 ^
-HINT:  There is a column named "x1" in table "xx1", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "x1" in table "xx1" and another table so a table-qualified column reference is required.

update xx1 set x2 = f1 from (select * from int4_tbl where f1 = xx1.x1) ss;
ERROR: invalid reference to FROM-clause entry for table "xx1"
LINE 1: ...t x2 = f1 from (select * from int4_tbl where f1 = xx1.x1) ss...
^
HINT: There is an entry for table "xx1", but it cannot be referenced from this part of the query.
-- can't do it even with LATERAL:

update xx1 set x2 = f1 from lateral (select * from int4_tbl where f1 = x1) ss;
ERROR: invalid reference to FROM-clause entry for table "xx1"
LINE 1: ...= f1 from lateral (select * from int4_tbl where f1 = x1) ss;
^
HINT: There is an entry for table "xx1", but it cannot be referenced from this part of the query.
-- we might in future allow something like this, but for now it's an error:

update xx1 set x2 = f1 from xx1, lateral (select * from int4_tbl where f1 = x1) ss;
ERROR: table name "xx1" specified more than once
-- also errors:

 delete from xx1 using (select * from int4_tbl where f1 = x1) ss;
 ERROR:  column "x1" does not exist
 LINE 1: ...te from xx1 using (select * from int4_tbl where f1 = x1) ss;
                                                                 ^
-HINT:  There is a column named "x1" in table "xx1", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "x1" in table "xx1" and another table so a table-qualified column reference is required.

delete from xx1 using (select * from int4_tbl where f1 = xx1.x1) ss;
ERROR: invalid reference to FROM-clause entry for table "xx1"
LINE 1: ...from xx1 using (select * from int4_tbl where f1 = xx1.x1) ss...
^
HINT: There is an entry for table "xx1", but it cannot be referenced from this part of the query.

delete from xx1 using lateral (select * from int4_tbl where f1 = x1) ss;
ERROR: invalid reference to FROM-clause entry for table "xx1"
LINE 1: ...xx1 using lateral (select * from int4_tbl where f1 = x1) ss;
^
-- this should fail because f1 is not exposed for unqualified reference:

 create rule rules_foorule as on insert to rules_foo where f1 < 100
 do instead insert into rules_foo2 values (f1);
 ERROR:  column "f1" does not exist
 LINE 2: do instead insert into rules_foo2 values (f1);
                                                   ^
-HINT:  There is a column named "f1" in table "old", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "f1" in table "old" and another table so a table-qualified column reference is required.

-- This should fail, because q2 isn't a name of an EXCEPT output column

 SELECT q1 FROM int8_tbl EXCEPT SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1;
 ERROR:  column "q2" does not exist
 LINE 1: ... int8_tbl EXCEPT SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1...
                                                              ^
-HINT:  There is a column named "q2" in table "*SELECT* 2", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "q2" in table "*SELECT* 2" and another table so a table-qualified column reference is required.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Indecision is a decision. Inaction is an action. Mark Batterson

Bruce Momjian <bruce@momjian.us> writes:

Feedback?

-- test some error cases where LATERAL should have been used but wasn't
select f1,g from int4_tbl a, (select f1 as g) ss;
ERROR:  column "f1" does not exist
LINE 1: select f1,g from int4_tbl a, (select f1 as g) ss;
^
-HINT:  There is a column named "f1" in table "a", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "f1" in table "a" and another table so a table-qualified column reference is required.

That one has gone from accurate to completely wrong. First, it's not
the case that there's more than one possible referent, and second,
table-qualifying the reference wouldn't help. What *would* help here
is adding LATERAL, but I'm not sure if we can easily tell whether that
is the case.

I think perhaps the existing message is mislabeled: it's not a hint
as written, but errdetail, because it's entirely factual. For this
particular example, what would be on-point is

DETAIL: There is a column named "f1" in table "a", but it cannot be referenced from this part of the query.
HINT: To reference that column, you must mark this subquery with LATERAL.

While we don't insist that hints be 100% accurate, it's not good
if they're wildly unhelpful. So I'm not sure if we can determine
whether or not it's likely to be on-point.

I didn't look too closely at your other examples.

regards, tom lane

On Thu, Aug 18, 2022 at 05:52:08PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Feedback?

-- test some error cases where LATERAL should have been used but wasn't
select f1,g from int4_tbl a, (select f1 as g) ss;
ERROR:  column "f1" does not exist
LINE 1: select f1,g from int4_tbl a, (select f1 as g) ss;
^
-HINT:  There is a column named "f1" in table "a", but it cannot be referenced from this part of the query.
+HINT:  There is a column named "f1" in table "a" and another table so a table-qualified column reference is required.

That one has gone from accurate to completely wrong. First, it's not
the case that there's more than one possible referent, and second,
table-qualifying the reference wouldn't help. What *would* help here
is adding LATERAL, but I'm not sure if we can easily tell whether that
is the case.

My error text was written based on the email report and was just a test
to see what queries trigger it.

I think perhaps the existing message is mislabeled: it's not a hint
as written, but errdetail, because it's entirely factual. For this
particular example, what would be on-point is

DETAIL: There is a column named "f1" in table "a", but it cannot be referenced from this part of the query.
HINT: To reference that column, you must mark this subquery with LATERAL.

Yes, the problem is that I don't think we want to just be making a
suggestion when we have other cases where LATERAL would not help.

While we don't insist that hints be 100% accurate, it's not good
if they're wildly unhelpful. So I'm not sure if we can determine
whether or not it's likely to be on-point.

I didn't look too closely at your other examples.

Yeah, I think someone will have to have a new idea to improve this.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Indecision is a decision. Inaction is an action. Mark Batterson

On Thu, Aug 18, 2022 at 10:07:02PM -0400, Bruce Momjian wrote:

While we don't insist that hints be 100% accurate, it's not good
if they're wildly unhelpful. So I'm not sure if we can determine
whether or not it's likely to be on-point.

I didn't look too closely at your other examples.

Yeah, I think someone will have to have a new idea to improve this.

Thinking some more, my point is that this error message is being
generated for three cases I know of:

1. email reporters case of CTID column, which is fixed by table
qualification

2. adding LATERAL

3. UPDATE/DELETE where adding LATERAL doesn't fix the query

We can't simply improve the error message because there are unfixable
cases, and we know of two possible fixes.

To improve things, it would be good if we could determine if LATERAL
will really fix the error, or at least detect one of the cases above we
have a clearer way to suggest a fix.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Indecision is a decision. Inaction is an action. Mark Batterson

Bruce Momjian <bruce@momjian.us> writes:

To improve things, it would be good if we could determine if LATERAL
will really fix the error, or at least detect one of the cases above we
have a clearer way to suggest a fix.

Here's a proposed patch that tries to determine this by looking at
ParseNamespaceItem flags. I'm not sure it's totally bulletproof,
but it's likely good enough for a HINT.

I felt that the conditional-expression nests in the existing ereport
calls were nearly unintelligible already, so I rearranged the logic
to duplicate portions of the ereports instead. That could be debated
perhaps. Also, as written some paths through errorMissingColumn
will invoke the findNSItemForRTE search twice. I'm not too fussed
about that: it's a pretty cheap search and anyway nobody should be
bothering to shave microseconds off an error reporting path.

regards, tom lane

On Sun, Aug 21, 2022 at 01:50:35PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

To improve things, it would be good if we could determine if LATERAL
will really fix the error, or at least detect one of the cases above we
have a clearer way to suggest a fix.

Here's a proposed patch that tries to determine this by looking at
ParseNamespaceItem flags. I'm not sure it's totally bulletproof,
but it's likely good enough for a HINT.

Wow, this is great. I can see how people would be confused by the need
for LATERAL, and this is really a great boost for them.

I felt that the conditional-expression nests in the existing ereport
calls were nearly unintelligible already, so I rearranged the logic

Wow, I am glad you mentioned this. I never saw a ? b ? c : d : e used
before and I had to look it up, and I have been around C for a long time:

https://www.geeksforgeeks.org/c-nested-ternary-operator/

to duplicate portions of the ereports instead. That could be debated
perhaps. Also, as written some paths through errorMissingColumn
will invoke the findNSItemForRTE search twice. I'm not too fussed
about that: it's a pretty cheap search and anyway nobody should be
bothering to shave microseconds off an error reporting path.

Exactly.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Indecision is a decision. Inaction is an action. Mark Batterson

I gave this a quick spin and couldn't find any faults.

The bit about getting an incorrect hint when p_rel_visible is false had
me busy for a little while (specifically I ran into it with the
"unnamed_subquery" stuff added by commit bcedd8f5fce0), but maybe that's
a fringe enough case, as the comment in rte_visible_if_lateral says.

I did wonder why errorMissingColumn doesn't consider rte_visible_if_* in
the case when there *is* an rsecond candidate. I understand that the
reason is that if we come across any exact match we already return that
one without looking for a second one. Maybe this deserves a comment (in
errorMissingColumn I mean) but I also wonder if we shouldn't scan the
whole RT in case there's another exact match that's also not visible.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Learn about compilers. Then everything looks like either a compiler or
a database, and now you have two problems but one of them is fun."
https://twitter.com/thingskatedid/status/1456027786158776329

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

I did wonder why errorMissingColumn doesn't consider rte_visible_if_* in
the case when there *is* an rsecond candidate. I understand that the
reason is that if we come across any exact match we already return that
one without looking for a second one. Maybe this deserves a comment (in
errorMissingColumn I mean) but I also wonder if we shouldn't scan the
whole RT in case there's another exact match that's also not visible.

Um. I'd not wanted to touch the fuzzy-search stuff because it seemed
like a mess of incomprehensible (if not actually buggy) code. But you
have a point --- I'd already noticed that the code was encouraging
people to qualify with a name that might be the wrong table altogether.

So here's a revision that tries to clean that up a little. 0001 is the
same patch as before, and then 0002 revises the fuzzy-search logic enough
that I can make sense of it. I split them mainly so that you can see the
behavioral difference in the changed test outputs.

regards, tom lane

I wrote:

So here's a revision that tries to clean that up a little. 0001 is the
same patch as before, and then 0002 revises the fuzzy-search logic enough
that I can make sense of it. I split them mainly so that you can see the
behavioral difference in the changed test outputs.

Hearing no further comments, pushed.

regards, tom lane