From dbf9093c642a42d9831cad0d8914bb096750c625 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Tue, 8 Oct 2024 18:59:03 +0900 Subject: [PATCH v1] Fix inconsistent RestrictInfo serial numbers When we generate multiple clones of the same qual condition to cope with outer join identity 3, we need to ensure that all the clones get the same serial number. To achieve this, we reset the root->last_rinfo_serial counter each time we produce RestrictInfo(s) from the qual (see deconstruct_distribute_oj_quals). This approach works only if we ensure that we are not changing the qual list in any way that'd affect the number of RestrictInfos built from it. However, with b262ad440, an IS NULL qual on a NOT NULL column might result in an additional constant-FALSE RestrictInfo. This can unexpectedly increase root->last_rinfo_serial, causing inconsistent RestrictInfo serial numbers across multiple clones of the same qual, which can confuse users of these serial numbers, such as rebuild_joinclause_attr_needed, and lead to planner errors. To fix, reset the root->last_rinfo_serial counter after generating the additional constant-FALSE RestrictInfo. --- src/backend/optimizer/plan/initsplan.c | 10 ++++++++- src/backend/optimizer/util/joininfo.c | 12 +++++++++-- src/test/regress/expected/predicate.out | 28 +++++++++++++++++++++++++ src/test/regress/sql/predicate.sql | 15 +++++++++++++ 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index c5bc0f51e9..903c397d40 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -2767,11 +2767,18 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid, /* * Substitute the origin qual with constant-FALSE if it is provably - * always false. Note that we keep the same rinfo_serial. + * always false. + * + * Note that we need to keep the same rinfo_serial, since it is in + * practice the same condition. We also need to reset the + * last_rinfo_serial counter, which is essential to ensure that the + * RestrictInfos for the "same" qual condition get identical serial + * numbers (see deconstruct_distribute_oj_quals). */ if (restriction_is_always_false(root, restrictinfo)) { int save_rinfo_serial = restrictinfo->rinfo_serial; + int save_last_rinfo_serial = root->last_rinfo_serial; restrictinfo = make_restrictinfo(root, (Expr *) makeBoolConst(false, false), @@ -2784,6 +2791,7 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid, restrictinfo->incompatible_relids, restrictinfo->outer_relids); restrictinfo->rinfo_serial = save_rinfo_serial; + root->last_rinfo_serial = save_last_rinfo_serial; } } diff --git a/src/backend/optimizer/util/joininfo.c b/src/backend/optimizer/util/joininfo.c index 5fb0c17630..65993bd659 100644 --- a/src/backend/optimizer/util/joininfo.c +++ b/src/backend/optimizer/util/joininfo.c @@ -106,12 +106,19 @@ add_join_clause_to_rels(PlannerInfo *root, return; /* - * Substitute constant-FALSE for the origin qual if it is always false. - * Note that we keep the same rinfo_serial. + * Substitute the origin qual with constant-FALSE if it is provably always + * false. + * + * Note that we need to keep the same rinfo_serial, since it is in + * practice the same condition. We also need to reset the + * last_rinfo_serial counter, which is essential to ensure that the + * RestrictInfos for the "same" qual condition get identical serial + * numbers (see deconstruct_distribute_oj_quals). */ if (restriction_is_always_false(root, restrictinfo)) { int save_rinfo_serial = restrictinfo->rinfo_serial; + int save_last_rinfo_serial = root->last_rinfo_serial; restrictinfo = make_restrictinfo(root, (Expr *) makeBoolConst(false, false), @@ -124,6 +131,7 @@ add_join_clause_to_rels(PlannerInfo *root, restrictinfo->incompatible_relids, restrictinfo->outer_relids); restrictinfo->rinfo_serial = save_rinfo_serial; + root->last_rinfo_serial = save_last_rinfo_serial; } cur_relid = -1; diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out index 6f1cc0d54c..965a3a7616 100644 --- a/src/test/regress/expected/predicate.out +++ b/src/test/regress/expected/predicate.out @@ -290,3 +290,31 @@ SELECT * FROM pred_parent WHERE a IS NULL; (2 rows) DROP TABLE pred_parent, pred_child; +-- Validate the additional constant-FALSE qual does not cause inconsistent +-- RestrictInfo serial numbers +CREATE TABLE pred_tab (a int PRIMARY KEY, b int); +INSERT INTO pred_tab SELECT i, i FROM generate_series(1, 10)i; +ANALYZE pred_tab; +EXPLAIN (COSTS OFF) +SELECT 1 FROM pred_tab t1 + LEFT JOIN + (pred_tab t2 LEFT JOIN pred_tab t3 ON t2.a = t3.a) ON TRUE + LEFT JOIN pred_tab t4 ON t1.a IS NULL AND t1.b = 1 + RIGHT JOIN pred_tab t5 ON t1.b = t5.b; + QUERY PLAN +--------------------------------------------------- + Hash Right Join + Hash Cond: (t1.b = t5.b) + -> Nested Loop Left Join + -> Nested Loop Left Join + Join Filter: (false AND (t1.b = 1)) + -> Seq Scan on pred_tab t1 + -> Result + One-Time Filter: false + -> Materialize + -> Seq Scan on pred_tab t2 + -> Hash + -> Seq Scan on pred_tab t5 +(12 rows) + +DROP TABLE pred_tab; diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql index 63f6a7786f..661013ff7e 100644 --- a/src/test/regress/sql/predicate.sql +++ b/src/test/regress/sql/predicate.sql @@ -147,3 +147,18 @@ EXPLAIN (COSTS OFF) SELECT * FROM pred_parent WHERE a IS NULL; DROP TABLE pred_parent, pred_child; + +-- Validate the additional constant-FALSE qual does not cause inconsistent +-- RestrictInfo serial numbers +CREATE TABLE pred_tab (a int PRIMARY KEY, b int); +INSERT INTO pred_tab SELECT i, i FROM generate_series(1, 10)i; +ANALYZE pred_tab; + +EXPLAIN (COSTS OFF) +SELECT 1 FROM pred_tab t1 + LEFT JOIN + (pred_tab t2 LEFT JOIN pred_tab t3 ON t2.a = t3.a) ON TRUE + LEFT JOIN pred_tab t4 ON t1.a IS NULL AND t1.b = 1 + RIGHT JOIN pred_tab t5 ON t1.b = t5.b; + +DROP TABLE pred_tab; -- 2.43.0