From ace3c7cc0518269733c37b37b7aebb9adec2454e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 16 May 2018 13:46:09 -0400 Subject: [PATCH] Fix misprocessing of equivalence classes involving record_eq(). canonicalize_ec_expression() is supposed to agree with coerce_type() as to whether a RelabelType should be inserted to make a subexpression be valid input for the operators of a given opclass. However, it did the wrong thing with named-composite-type inputs to record_eq(): it put in a RelabelType to RECORDOID, which the parser doesn't. In some cases this was harmless because all code paths involving a particular equivalence class did the same thing, but in other cases this would result in failing to recognize a composite-type expression as being a member of an equivalence class that it actually is a member of. The most obvious bad effect was to fail to recognize that an index on a composite column could provide the sort order needed for a mergejoin on that column, as reported by Teodor Sigaev. I think there might be other, subtler, cases that result in misoptimization. It also seems possible that an unwanted RelabelType would sometimes get into an emitted plan --- but because record_eq and friends don't examine the declared type of their input expressions, that would not create any visible problems. To fix, just treat RECORDOID as if it were a polymorphic type, which in some sense it is. We might want to consider formalizing that a bit more someday, but for the moment this seems to be the only place where an IsPolymorphicType() test ought to include RECORDOID as well. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/a6b22369-e3bf-4d49-f59d-0c41d3551e81@sigaev.ru --- src/backend/optimizer/path/equivclass.c | 3 ++- src/test/regress/expected/join.out | 30 +++++++++++++++++++++++++ src/test/regress/sql/join.sql | 20 +++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 0e50ad5f34..503246f96d 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -424,8 +424,9 @@ canonicalize_ec_expression(Expr *expr, Oid req_type, Oid req_collation) /* * For a polymorphic-input-type opclass, just keep the same exposed type. + * RECORD opclasses work like polymorphic-type ones for this purpose. */ - if (IsPolymorphicType(req_type)) + if (IsPolymorphicType(req_type) || req_type == RECORDOID) req_type = expr_type; /* diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index ae760e6142..4f016b6c05 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2650,6 +2650,36 @@ select * from a left join b on i = x and i = y and x = i; ---+---+--- (0 rows) +rollback; +-- +-- test handling of merge clauses using record_ops +-- +begin; +create type mycomptype as (id int, v bigint); +create temp table tidv (idv mycomptype); +create index on tidv (idv); +explain (costs off) +select a.idv, b.idv from tidv a, tidv b where a.idv = b.idv; + QUERY PLAN +---------------------------------------------------------- + Merge Join + Merge Cond: (a.idv = b.idv) + -> Index Only Scan using tidv_idv_idx on tidv a + -> Materialize + -> Index Only Scan using tidv_idv_idx on tidv b +(5 rows) + +set enable_mergejoin = 0; +explain (costs off) +select a.idv, b.idv from tidv a, tidv b where a.idv = b.idv; + QUERY PLAN +---------------------------------------------------- + Nested Loop + -> Seq Scan on tidv a + -> Index Only Scan using tidv_idv_idx on tidv b + Index Cond: (idv = a.idv) +(4 rows) + rollback; -- -- test NULL behavior of whole-row Vars, per bug #5025 diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 174eec0ba4..8f1698648c 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -664,6 +664,26 @@ select * from a left join b on i = x and i = y and x = i; rollback; +-- +-- test handling of merge clauses using record_ops +-- +begin; + +create type mycomptype as (id int, v bigint); + +create temp table tidv (idv mycomptype); +create index on tidv (idv); + +explain (costs off) +select a.idv, b.idv from tidv a, tidv b where a.idv = b.idv; + +set enable_mergejoin = 0; + +explain (costs off) +select a.idv, b.idv from tidv a, tidv b where a.idv = b.idv; + +rollback; + -- -- test NULL behavior of whole-row Vars, per bug #5025 --