1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-28 23:42:10 +03:00

Be more careful about the shape of hashable subplan clauses.

nodeSubplan.c expects that the testexpr for a hashable ANY SubPlan
has the form of one or more OpExprs whose LHS is an expression of the
outer query's, while the RHS is an expression over Params representing
output columns of the subquery.  However, the planner only went as far
as verifying that the clauses were all binary OpExprs.  This works
99.99% of the time, because the clauses have the right shape when
emitted by the parser --- but it's possible for function inlining to
break that, as reported by PegoraroF10.  To fix, teach the planner
to check that the LHS and RHS contain the right things, or more
accurately don't contain the wrong things.  Given that this has been
broken for years without anyone noticing, it seems sufficient to just
give up hashing when it happens, rather than go to the trouble of
commuting the clauses back again (which wouldn't necessarily work
anyway).

While poking at that, I also noticed that nodeSubplan.c had a baked-in
assumption that the number of hash clauses is identical to the number
of subquery output columns.  Again, that's fine as far as parser output
goes, but it's not hard to break it via function inlining.  There seems
little reason for that assumption though --- AFAICS, the only thing
it's buying us is not having to store the number of hash clauses
explicitly.  Adding code to the planner to reject such cases would take
more code than getting nodeSubplan.c to cope, so I fixed it that way.

This has been broken for as long as we've had hashable SubPlans,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/1549209182255-0.post@n3.nabble.com
This commit is contained in:
Tom Lane
2020-08-14 22:14:03 -04:00
parent 3dadcb2b31
commit 912fb290c5
7 changed files with 219 additions and 30 deletions

View File

@ -757,6 +757,7 @@ insert into outer_text values ('a', null);
insert into outer_text values ('b', null);
create temp table inner_text (c1 text, c2 text);
insert into inner_text values ('a', null);
insert into inner_text values ('123', '456');
select * from outer_text where (f1, f2) not in (select * from inner_text);
f1 | f2
----+----
@ -797,6 +798,82 @@ select '1'::text in (select '1'::name union all select '1'::name);
t
(1 row)
--
-- Test that we don't try to use a hashed subplan if the simplified
-- testexpr isn't of the right shape
--
-- this fails by default, of course
select * from int8_tbl where q1 in (select c1 from inner_text);
ERROR: operator does not exist: bigint = text
LINE 1: select * from int8_tbl where q1 in (select c1 from inner_tex...
^
HINT: No operator matches the given name and argument types. You might need to add explicit type casts.
begin;
-- make an operator to allow it to succeed
create function bogus_int8_text_eq(int8, text) returns boolean
language sql as 'select $1::text = $2';
create operator = (procedure=bogus_int8_text_eq, leftarg=int8, rightarg=text);
explain (costs off)
select * from int8_tbl where q1 in (select c1 from inner_text);
QUERY PLAN
--------------------------------
Seq Scan on int8_tbl
Filter: (hashed SubPlan 1)
SubPlan 1
-> Seq Scan on inner_text
(4 rows)
select * from int8_tbl where q1 in (select c1 from inner_text);
q1 | q2
-----+------------------
123 | 456
123 | 4567890123456789
(2 rows)
-- inlining of this function results in unusual number of hash clauses,
-- which we can still cope with
create or replace function bogus_int8_text_eq(int8, text) returns boolean
language sql as 'select $1::text = $2 and $1::text = $2';
explain (costs off)
select * from int8_tbl where q1 in (select c1 from inner_text);
QUERY PLAN
--------------------------------
Seq Scan on int8_tbl
Filter: (hashed SubPlan 1)
SubPlan 1
-> Seq Scan on inner_text
(4 rows)
select * from int8_tbl where q1 in (select c1 from inner_text);
q1 | q2
-----+------------------
123 | 456
123 | 4567890123456789
(2 rows)
-- inlining of this function causes LHS and RHS to be switched,
-- which we can't cope with, so hashing should be abandoned
create or replace function bogus_int8_text_eq(int8, text) returns boolean
language sql as 'select $2 = $1::text';
explain (costs off)
select * from int8_tbl where q1 in (select c1 from inner_text);
QUERY PLAN
--------------------------------------
Seq Scan on int8_tbl
Filter: (SubPlan 1)
SubPlan 1
-> Materialize
-> Seq Scan on inner_text
(5 rows)
select * from int8_tbl where q1 in (select c1 from inner_text);
q1 | q2
-----+------------------
123 | 456
123 | 4567890123456789
(2 rows)
rollback; -- to get rid of the bogus operator
--
-- Test case for planner bug with nested EXISTS handling
--

View File

@ -449,6 +449,7 @@ insert into outer_text values ('b', null);
create temp table inner_text (c1 text, c2 text);
insert into inner_text values ('a', null);
insert into inner_text values ('123', '456');
select * from outer_text where (f1, f2) not in (select * from inner_text);
@ -468,6 +469,46 @@ select 'foo'::text in (select 'bar'::name union all select 'bar'::name);
select '1'::text in (select '1'::name union all select '1'::name);
--
-- Test that we don't try to use a hashed subplan if the simplified
-- testexpr isn't of the right shape
--
-- this fails by default, of course
select * from int8_tbl where q1 in (select c1 from inner_text);
begin;
-- make an operator to allow it to succeed
create function bogus_int8_text_eq(int8, text) returns boolean
language sql as 'select $1::text = $2';
create operator = (procedure=bogus_int8_text_eq, leftarg=int8, rightarg=text);
explain (costs off)
select * from int8_tbl where q1 in (select c1 from inner_text);
select * from int8_tbl where q1 in (select c1 from inner_text);
-- inlining of this function results in unusual number of hash clauses,
-- which we can still cope with
create or replace function bogus_int8_text_eq(int8, text) returns boolean
language sql as 'select $1::text = $2 and $1::text = $2';
explain (costs off)
select * from int8_tbl where q1 in (select c1 from inner_text);
select * from int8_tbl where q1 in (select c1 from inner_text);
-- inlining of this function causes LHS and RHS to be switched,
-- which we can't cope with, so hashing should be abandoned
create or replace function bogus_int8_text_eq(int8, text) returns boolean
language sql as 'select $2 = $1::text';
explain (costs off)
select * from int8_tbl where q1 in (select c1 from inner_text);
select * from int8_tbl where q1 in (select c1 from inner_text);
rollback; -- to get rid of the bogus operator
--
-- Test case for planner bug with nested EXISTS handling
--