pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18261: Inconsistent results of SELECT affected by joined subqueries
14+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2024-01-06 5ef34a8fc3 Fix the issue that SJE mistakenly omits qual clauses
* BUG #18261: Inconsistent results of SELECT affected by joined subqueries
@ 2023-12-27 10:47 PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: PG Bug reporting form @ 2023-12-27 10:47 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: zuming.jiang@inf.ethz.ch
The following bug has been logged on the website:
Bug reference: 18261
Logged by: Zuming Jiang
Email address: zuming.jiang@inf.ethz.ch
PostgreSQL version: 16.1
Operating system: Ubuntu 20.04
Description:
My fuzzer finds a logic bug in Postgres, which makes Postgres return
inconsistent results.
--- Set up database ---
create table t0 (pkey int4, c0 numeric, primary key(c0));
create table t1 (c3 int4);
create table t5 (vkey int4, pkey int4);
create view t6 as
select
1 as c_7
from
((select
0.0 as c_0,
-4 as c_1
from
t0 as ref_0
) as subq_0
right outer join ((select
ref_3.pkey as c_6,
ref_3.c0 as c_7
from
t0 as ref_3
) as subq_1
right outer join t0 as ref_4
on (subq_1.c_7 = ref_4.c0 ))
on (subq_0.c_0 = subq_1.c_7 ))
where 'xxx' ~~* (ltrim('xxx',
(overlay('xxx', 'xxx', subq_0.c_1, subq_1.c_6))));
insert into t5 values (68, 78000);
---
The fuzzer generates Test case 1:
--- Test case 1 ---
select 1 as c_0
from
((select
ref_2.vkey as c_3,
ref_2.pkey as c_4,
ref_2.pkey as c_5
from
t5 as ref_2
) as subq_0
full outer join (select
ref_4.c_7 as c_0
from
t6 as ref_4
where null::bool) as subq_1
on (subq_0.c_3 = subq_1.c_0))
where subq_0.c_4 < (case when ((exists (
select
ref_16.c3 as c_9
from
t1 as ref_16
where null::bool))) then (subq_1.c_0 # null::int4) else
(subq_1.c_0 # null::int4) end);
Because the then branch and else branch of the CASE WHEN expression '(case
when ((exists (select ref_16.c3 as c_9 from t1 as ref_16 where null::bool)))
then (subq_1.c_0 # null::int4) else (subq_1.c_0 # null::int4) end)' are the
same (both are subq_1.c_0 # null::int4), I simplify this CASE WHEN
expression by replacing it with (subq_1.c_0 # null::int4), and get Test case
2:
--- Test case 2 ---
select 1 as c_0
from
((select
ref_2.vkey as c_3,
ref_2.pkey as c_4,
ref_2.pkey as c_5
from
t5 as ref_2
) as subq_0
full outer join (select
ref_4.c_7 as c_0
from
t6 as ref_4
where null::bool) as subq_1
on (subq_0.c_3 = subq_1.c_0 ))
where subq_0.c_4 < (subq_1.c_0 # null::int4);
--- Expected behavior ---
Test case 1 and Test case 2 return the same results.
--- Actual behavior ---
Test case 1 returns 0 rows, while Test case 2 returns 1 row.
Output of Test case 1:
c_0
-----
(0 rows)
Output of Test case 2:
c_0
-----
1
(1 row)
--- Postgres version ---
Github commit: 0eac3c798c2d223d6557a5440d7534317dbd4fa0
Version: PostgreSQL 17devel on x86_64-pc-linux-gnu, compiled by gcc
(Ubuntu
9.4.0-1ubuntu1~20.04.1) 9.4.0, 64-bit
--- Platform information ---
Platform: Ubuntu 20.04
Kernel: Linux 5.4.0-147-generic
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
@ 2023-12-28 11:01 ` Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guo @ 2023-12-28 11:01 UTC (permalink / raw)
To: zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Wed, Dec 27, 2023 at 7:36 PM PG Bug reporting form <
noreply@postgresql.org> wrote:
> My fuzzer finds a logic bug in Postgres, which makes Postgres return
> inconsistent results.
Thank you for the report. This is surely a wrong result issue. Great
catch!
I've looked into it a bit. The problem lies in how the SJE code handles
the transfer of qual clauses from the removed relation to the remaining
one. The code replaces the Vars of the removed relation with the Vars
of the remaining relation for each clause. It then reintegrates these
clauses into the appropriate restriction or join clause lists, while
attempting to avoid duplicates. So far so good.
However, the code compares RestrictInfo->clause to determine if two
clauses are duplicates. This is just flat wrong. Two RestrictInfos
with the same clause can have different required_relids,
incompatible_relids, is_pushed_down and so on.
Here is a simple example to illustrate this issue.
create table t (a int primary key, b int);
insert into t select 1,1;
-- wrong plan. the full join should be dummy
explain (costs off)
select 1 from t full join
(select * from t t1 join
t t2 join t t3 on t2.a = t3.a
on true
where false) s on true
where false;
QUERY PLAN
--------------------------------------
Merge Full Join
-> Seq Scan on t
-> Materialize
-> Result
One-Time Filter: false
(5 rows)
-- wrong result
select 1 from t full join
(select * from t t1 join
t t2 join t t3 on t2.a = t3.a
on true
where false) s on true
where false;
?column?
----------
1
(1 row)
In this query there are two RestrictInfos whose clause are both
const-false: one is supposed to be evaluated at join t1/t3, and the
other is supposed to be evaluated above the full join. But the SJE
code mistakenly thinks that they are duplicates, so the one above the
full join is just abandoned.
Do we really need to avoid duplicates here? We do not do that before.
For instance,
explain (costs off)
select * from t where b > 1 and b > 1;
QUERY PLAN
---------------------------------
Seq Scan on t
Filter: ((b > 1) AND (b > 1))
(2 rows)
So I will not be surprised if I see redundant 'b > 1' after the join
removal in the following plan.
explain (costs off)
select * from t t1 join t t2 on t1.a = t2.a where t1.b > 1 and t2.b > 1;
QUERY PLAN
-----------------------------------------
Seq Scan on t t2
Filter: ((a IS NOT NULL) AND (b > 1))
(2 rows)
If we determine that avoiding duplicates is necessary, I think at least
we should compare the entire RestrictInfos not just their clauses. One
challenge with this approach is that the 'rinfo_serial' usually differs,
making direct comparison problematic. I'm wondering if we can make
'rinfo_serial' equal_ignore. Not too sure about that.
Attached is a patch to show my thoughts.
Thanks
Richard
Attachments:
v1-0001-Fix-the-issue-that-SJE-mistakenly-omits-qual-clauses.patch (application/octet-stream, 5.5K) download
From 11601360a4471cd6fafffb91e64b6c34d2b99c1c Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 28 Dec 2023 18:52:11 +0800
Subject: [PATCH v1] Fix the issue that SJE mistakenly omits qual clauses
When the SJE code handles the transfer of qual clauses from the removed
relation to the remaining one, it replaces the Vars of the removed
relation with the Vars of the remaining relation for each clause, and
then reintegrates these clauses into the appropriate restriction or join
clause lists, while attempting to avoid duplicates.
However, the code compares RestrictInfo->clause to determine if two
clauses are duplicates. This is just flat wrong. Two RestrictInfos
with the same clause can have different required_relids,
incompatible_relids, is_pushed_down and so on. This can cause qual
clauses to be mistakenly omited, leading to wrong result.
This patch fixes it by comparing the entire RestrictInfos not just their
clauses. To make it work, this patch makes 'rinfo_serial' equal_ignore.
---
src/backend/optimizer/plan/analyzejoins.c | 4 ++--
src/include/nodes/pathnodes.h | 2 +-
src/test/regress/expected/join.out | 27 ++++++++++++++++++++++-
src/test/regress/sql/join.sql | 18 ++++++++++++++-
4 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 80739451b7..1f035f2743 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1760,7 +1760,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
if (src == rinfo ||
(rinfo->parent_ec != NULL
&& src->parent_ec == rinfo->parent_ec)
- || equal(rinfo->clause, src->clause))
+ || equal(rinfo, src))
{
is_redundant = true;
break;
@@ -1788,7 +1788,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
if (src == rinfo ||
(rinfo->parent_ec != NULL
&& src->parent_ec == rinfo->parent_ec)
- || equal(rinfo->clause, src->clause))
+ || equal(rinfo, src))
{
is_redundant = true;
break;
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index ed85dc7414..c791ca0c3f 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2595,7 +2595,7 @@ typedef struct RestrictInfo
* of redundant pushed-down equality clauses.
*----------
*/
- int rinfo_serial;
+ int rinfo_serial pg_node_attr(equal_ignore);
/*
* Generating EquivalenceClass. This field is NULL unless clause is
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 1557e17299..45259e08e2 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -6742,7 +6742,7 @@ explain (costs off) select 1 from
reset join_collapse_limit;
reset enable_seqscan;
-- Check that clauses from the join filter list is not lost on the self-join removal
-CREATE TABLE emp1 ( id SERIAL PRIMARY KEY NOT NULL, code int);
+CREATE TABLE emp1 (id SERIAL PRIMARY KEY NOT NULL, code int);
explain (verbose, costs off)
SELECT * FROM emp1 e1, emp1 e2 WHERE e1.id = e2.id AND e2.code <> e1.code;
QUERY PLAN
@@ -6868,6 +6868,31 @@ select * from emp1 t1
-> Seq Scan on emp1 t3
(6 rows)
+-- Check that SJE does not mistakenly omit qual clauses
+insert into emp1 values (1, 1);
+explain (costs off)
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+ ?column?
+----------
+(0 rows)
+
-- We can remove the join even if we find the join can't duplicate rows and
-- the base quals of each side are different. In the following case we end up
-- moving quals over to s1 to make it so it can't match any rows.
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index fed9e83e31..b48eb701c8 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2568,7 +2568,7 @@ reset join_collapse_limit;
reset enable_seqscan;
-- Check that clauses from the join filter list is not lost on the self-join removal
-CREATE TABLE emp1 ( id SERIAL PRIMARY KEY NOT NULL, code int);
+CREATE TABLE emp1 (id SERIAL PRIMARY KEY NOT NULL, code int);
explain (verbose, costs off)
SELECT * FROM emp1 e1, emp1 e2 WHERE e1.id = e2.id AND e2.code <> e1.code;
@@ -2616,6 +2616,22 @@ select * from emp1 t1
inner join emp1 t2 on t1.id = t2.id
left join emp1 t3 on t1.id > 1 and t1.id < 2;
+-- Check that SJE does not mistakenly omit qual clauses
+insert into emp1 values (1, 1);
+explain (costs off)
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+
-- We can remove the join even if we find the join can't duplicate rows and
-- the base quals of each side are different. In the following case we end up
-- moving quals over to s1 to make it so it can't match any rows.
--
2.31.0
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
@ 2023-12-28 16:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 01:43 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Tom Lane @ 2023-12-28 16:32 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: zuming.jiang@inf.ethz.ch, Alexander Korotkov <aekorotkov@gmail.com>; pgsql-bugs@lists.postgresql.org
Richard Guo <guofenglinux@gmail.com> writes:
> I've looked into it a bit. The problem lies in how the SJE code handles
> the transfer of qual clauses from the removed relation to the remaining
> one.
I am definitely starting to think that the SJE patch was not ready
for prime time. We keep finding not only minor but major problems
in it --- I'd call this one a "major" one. Is it time to revert
and rethink it from scratch?
> If we determine that avoiding duplicates is necessary, I think at least
> we should compare the entire RestrictInfos not just their clauses. One
> challenge with this approach is that the 'rinfo_serial' usually differs,
> making direct comparison problematic. I'm wondering if we can make
> 'rinfo_serial' equal_ignore. Not too sure about that.
I'd say that that will break the cases rinfo_serial was introduced for.
Now, I certainly don't love rinfo_serial and would be happier if we
could do without it, but getting rid of it is another research project.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-01-02 01:43 ` Richard Guo <guofenglinux@gmail.com>
2024-01-02 08:08 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-03 05:56 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
1 sibling, 2 replies; 14+ messages in thread
From: Richard Guo @ 2024-01-02 01:43 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: zuming.jiang@inf.ethz.ch, Alexander Korotkov <aekorotkov@gmail.com>; pgsql-bugs@lists.postgresql.org
On Fri, Dec 29, 2023 at 12:32 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Richard Guo <guofenglinux@gmail.com> writes:
> > I've looked into it a bit. The problem lies in how the SJE code handles
> > the transfer of qual clauses from the removed relation to the remaining
> > one.
>
> I am definitely starting to think that the SJE patch was not ready
> for prime time. We keep finding not only minor but major problems
> in it --- I'd call this one a "major" one. Is it time to revert
> and rethink it from scratch?
I feel the same way. It seems to me that the SJE code still needs some
refactoring to get to a committable state.
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 01:43 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
@ 2024-01-02 08:08 ` Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 0 replies; 14+ messages in thread
From: Alexander Korotkov @ 2024-01-02 08:08 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Tue, Jan 2, 2024 at 3:43 AM Richard Guo <guofenglinux@gmail.com> wrote:
> On Fri, Dec 29, 2023 at 12:32 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Richard Guo <guofenglinux@gmail.com> writes:
>> > I've looked into it a bit. The problem lies in how the SJE code handles
>> > the transfer of qual clauses from the removed relation to the remaining
>> > one.
>>
>> I am definitely starting to think that the SJE patch was not ready
>> for prime time. We keep finding not only minor but major problems
>> in it --- I'd call this one a "major" one. Is it time to revert
>> and rethink it from scratch?
>
> I feel the same way. It seems to me that the SJE code still needs some
> refactoring to get to a committable state.
Thank you for your feedback. Do you have something particular to be
done, which requires reverting the feature from the source tree?
My concern is that we've already fixed quite a set of bugs after
committing to this feature. If we push the feature back into the
mailing list thread for possibly a long time, then more conflicting
features could be committed (like nullable vars in the past). Thus,
we may end up committing it again with a shape not better than it is
now. On the other hand, if there is a plan to redesign the feature in
a way that greatly reduces the number of potential conflicts, that's
another story.
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 01:43 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
@ 2024-01-03 05:56 ` Andrei Lepikhov <a.lepikhov@postgrespro.ru>
1 sibling, 0 replies; 14+ messages in thread
From: Andrei Lepikhov @ 2024-01-03 05:56 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: zuming.jiang@inf.ethz.ch, Alexander Korotkov <aekorotkov@gmail.com>; pgsql-bugs@lists.postgresql.org
On 2/1/2024 08:43, Richard Guo wrote:
>
> On Fri, Dec 29, 2023 at 12:32 AM Tom Lane <tgl@sss.pgh.pa.us
> <mailto:tgl@sss.pgh.pa.us>> wrote:
>
> Richard Guo <guofenglinux@gmail.com <mailto:guofenglinux@gmail.com>>
> writes:
> > I've looked into it a bit. The problem lies in how the SJE code
> handles
> > the transfer of qual clauses from the removed relation to the
> remaining
> > one.
>
> I am definitely starting to think that the SJE patch was not ready
> for prime time. We keep finding not only minor but major problems
> in it --- I'd call this one a "major" one. Is it time to revert
> and rethink it from scratch?
>
>
> I feel the same way. It seems to me that the SJE code still needs some
> refactoring to get to a committable state.
Being an author of the feature, I couldn't be all objective, of course.
However, let me write some words on this code.
About profitability. According to current database development trends,
the feature looks helpful, where we see many auto-generated and
syntactically redundant queries. So, IMO, some tools for simplifying the
parse tree should be in the pocket of the DBMS. We do have numerous
planner hooks that give way to well-known extensions like Citus or
TimescaleDB, but tweaking the parse tree is gradually cheaper in many cases.
By reducing the number of RangeTblEntries, clauses, and equivalence
classes, we can profit significantly from this feature, especially with
partitioned tables.
Hence, this code is not only about self-join elimination. It also
introduces machinery for redundant parse tree reduction not only for the
case of dead tails, like reduce_outer_joins. It may be utilized by
extensions in other ways, too.
About weak points. We tried different ways to implement that feature
during the development, even in the optimization stage. The current
place and logic are the most effective. The self-join detection stage
looks stable and polished enough. Only one part of the code is debatable
- replacing one baserel (removing), involved in many relations with
other DB objects, with another one (keeping).
About development. Having it working in a PG fork and as a patch at the
commitfest for a long time, we hadn't had so much food for thought as
last three months when it was committed to the master. It raised new
questions on weaknesses of setrefs, links from outer query blocks,
integrity control of PlannerInfo, etc.
Bugs we have been finding look at most not about architectural issues
but about leaks in planner understanding, which is primarily correct
about me personally.
I think, having close scrutiny of the PlannerInfo fields and clause
replacement strategy, we can achieve stable behaviour of this feature
before the release and I already see progress in stability of the
feature. As I see dynamics of discussions in the pgsql-bugs list, we
already have a community team of people who detect SJE problems, develop
the code and provide a review. Maybe it is also a positive outcome?
Having written all the words above, I think we must formulate objective
reasons if we want to revert this feature. Another way it can live on
the mailing list for another five years without any progress. I,
personally, vote for leaving this feature in the master.
--
regards,
Andrei Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-01-02 08:02 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
1 sibling, 1 reply; 14+ messages in thread
From: Alexander Korotkov @ 2024-01-02 08:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Thu, Dec 28, 2023 at 6:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Richard Guo <guofenglinux@gmail.com> writes:
> > I've looked into it a bit. The problem lies in how the SJE code handles
> > the transfer of qual clauses from the removed relation to the remaining
> > one.
>
> I am definitely starting to think that the SJE patch was not ready
> for prime time. We keep finding not only minor but major problems
> in it --- I'd call this one a "major" one. Is it time to revert
> and rethink it from scratch?
First, I'd like to admit that the SJE patch contained a set of
oversights, some of them were very unnecessary ones (like putting
non-node into RelOptInfo.unique_for_rels).
The distinction between minor and major issues is often a matter of
perspective. In the case of the SJE patch, I believe the issues yet
spotted are localized and do not necessitate a complete redesign of
the feature. This localized nature of the problems suggests that
targeted fixes could be sufficient to address the current concerns.
In terms of effort, the bitmapset issue was the toughest for me yet.
But I would have to say that this issue persisted in the code before.
As a result of the committed fixes, now we have the instrumentation to
detect problems like this in the future.
The integration of the SJE feature with the optimizer's data structure
and algorithms indeed presents a high risk of conflicts with other
patches. The experience with nullable vars is a testament to this
challenge. It's a delicate balance to maintain, and I recognize the
difficulties in predicting and managing such conflicts.
I am not advocating for the SJE feature unconditionally. I am fully
prepared to reconsider its implementation if a fundamental redesign
becomes necessary. However, at this stage, I believe that retaining
and refining the feature is more beneficial than a complete rollback.
Despite its imperfections, the SJE patch holds significant potential.
> > If we determine that avoiding duplicates is necessary, I think at least
> > we should compare the entire RestrictInfos not just their clauses. One
> > challenge with this approach is that the 'rinfo_serial' usually differs,
> > making direct comparison problematic. I'm wondering if we can make
> > 'rinfo_serial' equal_ignore. Not too sure about that.
>
> I'd say that that will break the cases rinfo_serial was introduced for.
> Now, I certainly don't love rinfo_serial and would be happier if we
> could do without it, but getting rid of it is another research project.
It's a pity that no regression tests detect that. The attached patch
implements the special comparison function, which ignores the
'rinfo_serial' field. This avoids marking 'rinfo_serial' as
pg_node_attr(equal_ignore).
Links.
1. https://www.postgresql.org/message-id/CAMbWs4_wJthNtYBL%2BSsebpgF-5L2r5zFFk6xYbS0A78GKOTFHw%40mail.g...
------
Regards,
Alexander Korotkov
Attachments:
0001-Fix-the-issue-that-SJE-mistakenly-omits-qual-clau-v2.patch (application/octet-stream, 6.2K) download
From eb9081a0fe916514b0bedafce05cf646c3e2146e Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sat, 30 Dec 2023 01:17:17 +0200
Subject: [PATCH] Fix the issue that SJE mistakenly omits qual clauses
When the SJE code handles the transfer of qual clauses from the removed
relation to the remaining one, it replaces the Vars of the removed
relation with the Vars of the remaining relation for each clause, and
then reintegrates these clauses into the appropriate restriction or join
clause lists, while attempting to avoid duplicates.
However, the code compares RestrictInfo->clause to determine if two
clauses are duplicates. This is just flat wrong. Two RestrictInfos
with the same clause can have different required_relids,
incompatible_relids, is_pushed_down, and so on. This can cause qual
clauses to be mistakenly omitted, leading to wrong results.
This patch fixes it by comparing the entire RestrictInfos not just their
clauses ignoring 'rinfo_serial' field (otherwise almost all RestrictInfos will
be unique). Making 'rinfo_serial' equal_ignore would break other code. This
is why we implement our own comparison function for checking the equality of
RestrictInfos.
Reported-by: Zuming Jiang
Bug: #18261
Discussion: https://postgr.es/m/18261-2a75d748c928609b%40postgresql.org
Author: Richard Guo
---
src/backend/optimizer/plan/analyzejoins.c | 24 ++++++++++++++++++--
src/test/regress/expected/join.out | 27 ++++++++++++++++++++++-
src/test/regress/sql/join.sql | 18 ++++++++++++++-
3 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 1296b9da41e..bc31d57ae47 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1657,6 +1657,26 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
ec->ec_relids = replace_relid(ec->ec_relids, from, to);
}
+/*
+ * "Logically" compares two RestrictInfo's ignoring the 'rinfo_serial' field,
+ * which makes almost every RestrictInfo unique.
+ *
+ * XXX: In the future, we might remove the 'rinfo_serial' field completely and get
+ * rid of this function.
+ */
+static bool
+restrict_infos_logically_equal(RestrictInfo *a, RestrictInfo *b)
+{
+ int saved_rinfo_serial = a->rinfo_serial;
+ bool result;
+
+ a->rinfo_serial = b->rinfo_serial;
+ result = equal(a, b);
+ a->rinfo_serial = saved_rinfo_serial;
+
+ return result;
+}
+
/*
* Remove a relation after we have proven that it participates only in an
* unneeded unique self join.
@@ -1760,7 +1780,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
if (src == rinfo ||
(rinfo->parent_ec != NULL
&& src->parent_ec == rinfo->parent_ec)
- || equal(rinfo->clause, src->clause))
+ || restrict_infos_logically_equal(rinfo, src))
{
is_redundant = true;
break;
@@ -1788,7 +1808,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
if (src == rinfo ||
(rinfo->parent_ec != NULL
&& src->parent_ec == rinfo->parent_ec)
- || equal(rinfo->clause, src->clause))
+ || restrict_infos_logically_equal(rinfo, src))
{
is_redundant = true;
break;
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 7c6f3cb15c9..789b96ffbb3 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -6742,7 +6742,7 @@ explain (costs off) select 1 from
reset join_collapse_limit;
reset enable_seqscan;
-- Check that clauses from the join filter list is not lost on the self-join removal
-CREATE TABLE emp1 ( id SERIAL PRIMARY KEY NOT NULL, code int);
+CREATE TABLE emp1 (id SERIAL PRIMARY KEY NOT NULL, code int);
explain (verbose, costs off)
SELECT * FROM emp1 e1, emp1 e2 WHERE e1.id = e2.id AND e2.code <> e1.code;
QUERY PLAN
@@ -6880,6 +6880,31 @@ WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code;
Filter: (id IS NOT NULL)
(3 rows)
+-- Check that SJE does not mistakenly omit qual clauses (bug #18187)
+insert into emp1 values (1, 1);
+explain (costs off)
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+ ?column?
+----------
+(0 rows)
+
-- We can remove the join even if we find the join can't duplicate rows and
-- the base quals of each side are different. In the following case we end up
-- moving quals over to s1 to make it so it can't match any rows.
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 6d368562646..d5658fe118c 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2568,7 +2568,7 @@ reset join_collapse_limit;
reset enable_seqscan;
-- Check that clauses from the join filter list is not lost on the self-join removal
-CREATE TABLE emp1 ( id SERIAL PRIMARY KEY NOT NULL, code int);
+CREATE TABLE emp1 (id SERIAL PRIMARY KEY NOT NULL, code int);
explain (verbose, costs off)
SELECT * FROM emp1 e1, emp1 e2 WHERE e1.id = e2.id AND e2.code <> e1.code;
@@ -2622,6 +2622,22 @@ WITH t1 AS (SELECT * FROM emp1)
UPDATE emp1 SET code = t1.code + 1 FROM t1
WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code;
+-- Check that SJE does not mistakenly omit qual clauses (bug #18187)
+insert into emp1 values (1, 1);
+explain (costs off)
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+select 1 from emp1 full join
+ (select * from emp1 t1 join
+ emp1 t2 join emp1 t3 on t2.id = t3.id
+ on true
+ where false) s on true
+where false;
+
-- We can remove the join even if we find the join can't duplicate rows and
-- the base quals of each side are different. In the following case we end up
-- moving quals over to s1 to make it so it can't match any rows.
--
2.39.3 (Apple Git-145)
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
@ 2024-01-05 03:32 ` Andrei Lepikhov <a.lepikhov@postgrespro.ru>
2024-01-06 12:44 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-08 11:50 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Andrei Lepikhov @ 2024-01-05 03:32 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On 2/1/2024 15:02, Alexander Korotkov wrote:
> On Thu, Dec 28, 2023 at 6:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'd say that that will break the cases rinfo_serial was introduced for.
>> Now, I certainly don't love rinfo_serial and would be happier if we
>> could do without it, but getting rid of it is another research project.
>
> It's a pity that no regression tests detect that. The attached patch
> implements the special comparison function, which ignores the
> 'rinfo_serial' field. This avoids marking 'rinfo_serial' as
> pg_node_attr(equal_ignore).
I've reviewed this patch, and it seems okay. It was a real blunder in
our understanding of clauses; thanks to all for pointing this out and
fixing it.
The regression test shows where two fully equal join clauses, applied on
different join levels, differ by the only required_relids. It covers the
problem and can be helpful by itself. IMO, one line in this test is
redundant (see attachment).
The comment on the patch looks okay. But assuming required_relids and
clause fields of two RestrinctInfos are equal, I can't imagine a
situation to have different is_pushed_down and incompatible_relids. Is
it really possible?
--
regards,
Andrei Lepikhov
Postgres Professional
Attachments:
tests_arrangement.diff (text/plain, 966B) download
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index faad882a03..063396ab52 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -6881,7 +6881,6 @@ WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code;
(3 rows)
-- Check that SJE does not mistakenly omit qual clauses (bug #18187)
-insert into emp1 values (1, 1);
explain (costs off)
select 1 from emp1 full join
(select * from emp1 t1 join
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 4f3c51b6eb..5c420a7c6d 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2623,7 +2623,6 @@ UPDATE emp1 SET code = t1.code + 1 FROM t1
WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code;
-- Check that SJE does not mistakenly omit qual clauses (bug #18187)
-insert into emp1 values (1, 1);
explain (costs off)
select 1 from emp1 full join
(select * from emp1 t1 join
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
@ 2024-01-06 12:44 ` Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 0 replies; 14+ messages in thread
From: Alexander Korotkov @ 2024-01-06 12:44 UTC (permalink / raw)
To: Andrei Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Fri, Jan 5, 2024 at 5:32 AM Andrei Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 2/1/2024 15:02, Alexander Korotkov wrote:
> > On Thu, Dec 28, 2023 at 6:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I'd say that that will break the cases rinfo_serial was introduced for.
> >> Now, I certainly don't love rinfo_serial and would be happier if we
> >> could do without it, but getting rid of it is another research project.
> >
> > It's a pity that no regression tests detect that. The attached patch
> > implements the special comparison function, which ignores the
> > 'rinfo_serial' field. This avoids marking 'rinfo_serial' as
> > pg_node_attr(equal_ignore).
> I've reviewed this patch, and it seems okay. It was a real blunder in
> our understanding of clauses; thanks to all for pointing this out and
> fixing it.
> The regression test shows where two fully equal join clauses, applied on
> different join levels, differ by the only required_relids. It covers the
> problem and can be helpful by itself. IMO, one line in this test is
> redundant (see attachment).
> The comment on the patch looks okay. But assuming required_relids and
> clause fields of two RestrinctInfos are equal, I can't imagine a
> situation to have different is_pushed_down and incompatible_relids. Is
> it really possible?
Andrei, thank you for your review. I've pushed it with your
suggestions and my further minor editings to comments and the commit
message. Sorry, I forgot to mention you as the reviewer.
One thing that worries me is that the patch version by Richard Guo [1]
marking 'rinfo_serial' as pg_node_attr(equal_ignore) didn't cause our
test suite to fail. Do you think you can write a test, which would
fail in that case?
Links
1. https://www.postgresql.org/message-id/CAMbWs48_EAPrc_C5qvK2WGvydHvht1JUnXwer9FVeU7t_zA%2BMQ%40mail.g...
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
@ 2024-01-08 11:50 ` Richard Guo <guofenglinux@gmail.com>
2024-01-08 12:49 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-09 00:27 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
1 sibling, 2 replies; 14+ messages in thread
From: Richard Guo @ 2024-01-08 11:50 UTC (permalink / raw)
To: Andrei Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Fri, Jan 5, 2024 at 11:32 AM Andrei Lepikhov <a.lepikhov@postgrespro.ru>
wrote:
> The regression test shows where two fully equal join clauses, applied on
> different join levels, differ by the only required_relids. It covers the
> problem and can be helpful by itself. IMO, one line in this test is
> redundant (see attachment).
Hmm, I don't think the insert statement in the test case is redundant.
It's needed to verify that the query in the test case gives the correct
result. Without the insert statement, the wrong plan would give the
same result as the correct plan, i.e., an empty set in this case.
IMO, if we make some code changes and add a test case for that, we need
to ensure the test can give a different (and correct of course) result
than what came before.
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
2024-01-08 11:50 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
@ 2024-01-08 12:49 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-08 13:10 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Alexander Korotkov @ 2024-01-08 12:49 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Andrei Lepikhov <a.lepikhov@postgrespro.ru>; Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Mon, Jan 8, 2024, 13:51 Richard Guo <guofenglinux@gmail.com> wrote:
>
> On Fri, Jan 5, 2024 at 11:32 AM Andrei Lepikhov <a.lepikhov@postgrespro.ru>
> wrote:
>
>> The regression test shows where two fully equal join clauses, applied on
>> different join levels, differ by the only required_relids. It covers the
>> problem and can be helpful by itself. IMO, one line in this test is
>> redundant (see attachment).
>
>
> Hmm, I don't think the insert statement in the test case is redundant.
> It's needed to verify that the query in the test case gives the correct
> result. Without the insert statement, the wrong plan would give the
> same result as the correct plan, i.e., an empty set in this case.
>
> IMO, if we make some code changes and add a test case for that, we need
> to ensure the test can give a different (and correct of course) result
> than what came before.
>
Thank you for pointing this. Yes, veryfying query result not just plan is
strengthening the test. I'll recheck this and push the fix later today.
------
Regards,
Alexander Korotkov
>
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
2024-01-08 11:50 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2024-01-08 12:49 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
@ 2024-01-08 13:10 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-08 15:40 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Alexander Korotkov @ 2024-01-08 13:10 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Andrei Lepikhov <a.lepikhov@postgrespro.ru>; Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Mon, Jan 8, 2024 at 2:49 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Mon, Jan 8, 2024, 13:51 Richard Guo <guofenglinux@gmail.com> wrote:
>> On Fri, Jan 5, 2024 at 11:32 AM Andrei Lepikhov <a.lepikhov@postgrespro.ru> wrote:
>>>
>>> The regression test shows where two fully equal join clauses, applied on
>>> different join levels, differ by the only required_relids. It covers the
>>> problem and can be helpful by itself. IMO, one line in this test is
>>> redundant (see attachment).
>>
>>
>> Hmm, I don't think the insert statement in the test case is redundant.
>> It's needed to verify that the query in the test case gives the correct
>> result. Without the insert statement, the wrong plan would give the
>> same result as the correct plan, i.e., an empty set in this case.
>>
>> IMO, if we make some code changes and add a test case for that, we need
>> to ensure the test can give a different (and correct of course) result
>> than what came before.
>
> Thank you for pointing this. Yes, veryfying query result not just plan is strengthening the test. I'll recheck this and push the fix later today.
Pushed. Thanks again, Richard.
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
2024-01-08 11:50 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2024-01-08 12:49 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-08 13:10 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
@ 2024-01-08 15:40 ` Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 0 replies; 14+ messages in thread
From: Alexander Korotkov @ 2024-01-08 15:40 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Andrei Lepikhov <a.lepikhov@postgrespro.ru>; Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Mon, Jan 8, 2024 at 3:10 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Mon, Jan 8, 2024 at 2:49 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Mon, Jan 8, 2024, 13:51 Richard Guo <guofenglinux@gmail.com> wrote:
> >> On Fri, Jan 5, 2024 at 11:32 AM Andrei Lepikhov <a.lepikhov@postgrespro.ru> wrote:
> >>>
> >>> The regression test shows where two fully equal join clauses, applied on
> >>> different join levels, differ by the only required_relids. It covers the
> >>> problem and can be helpful by itself. IMO, one line in this test is
> >>> redundant (see attachment).
> >>
> >>
> >> Hmm, I don't think the insert statement in the test case is redundant.
> >> It's needed to verify that the query in the test case gives the correct
> >> result. Without the insert statement, the wrong plan would give the
> >> same result as the correct plan, i.e., an empty set in this case.
> >>
> >> IMO, if we make some code changes and add a test case for that, we need
> >> to ensure the test can give a different (and correct of course) result
> >> than what came before.
> >
> > Thank you for pointing this. Yes, veryfying query result not just plan is strengthening the test. I'll recheck this and push the fix later today.
>
> Pushed. Thanks again, Richard.
I also noticed my indent oversight. Will fix that later today.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=koel&dt=2024-01-08%2014%3A19%3A03
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 08:02 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Andrei Lepikhov <a.lepikhov@postgrespro.ru>
2024-01-08 11:50 ` Re: BUG #18261: Inconsistent results of SELECT affected by joined subqueries Richard Guo <guofenglinux@gmail.com>
@ 2024-01-09 00:27 ` Andrei Lepikhov <a.lepikhov@postgrespro.ru>
1 sibling, 0 replies; 14+ messages in thread
From: Andrei Lepikhov @ 2024-01-09 00:27 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On 8/1/2024 18:50, Richard Guo wrote:
>
> On Fri, Jan 5, 2024 at 11:32 AM Andrei Lepikhov
> <a.lepikhov@postgrespro.ru <mailto:a.lepikhov@postgrespro.ru>> wrote:
>
> The regression test shows where two fully equal join clauses,
> applied on
> different join levels, differ by the only required_relids. It covers
> the
> problem and can be helpful by itself. IMO, one line in this test is
> redundant (see attachment).
>
>
> Hmm, I don't think the insert statement in the test case is redundant.
> It's needed to verify that the query in the test case gives the correct
> result. Without the insert statement, the wrong plan would give the
> same result as the correct plan, i.e., an empty set in this case.
>
> IMO, if we make some code changes and add a test case for that, we need
> to ensure the test can give a different (and correct of course) result
> than what came before.
Maybe. I personally support explain by an execution result if the code
is related to executor and couldn't be detected by the form of query
plan. But why not? anyway, Alexander already committed that.
--
regards,
Andrei Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2024-01-09 00:27 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-12-27 10:47 BUG #18261: Inconsistent results of SELECT affected by joined subqueries PG Bug reporting form <noreply@postgresql.org>
2023-12-28 11:01 ` Richard Guo <guofenglinux@gmail.com>
2023-12-28 16:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-01-02 01:43 ` Richard Guo <guofenglinux@gmail.com>
2024-01-02 08:08 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-03 05:56 ` Andrei Lepikhov <a.lepikhov@postgrespro.ru>
2024-01-02 08:02 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-05 03:32 ` Andrei Lepikhov <a.lepikhov@postgrespro.ru>
2024-01-06 12:44 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-08 11:50 ` Richard Guo <guofenglinux@gmail.com>
2024-01-08 12:49 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-08 13:10 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-08 15:40 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-01-09 00:27 ` Andrei Lepikhov <a.lepikhov@postgrespro.ru>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox