From a106c57e76b6db33cab680664e2fb4823d1f5692 Mon Sep 17 00:00:00 2001 From: Logan Saso Date: Thu, 16 Jul 2026 13:04:38 -0700 Subject: [PATCH v2] Ignore duplicate IS [NOT] NULL clauses in clauselist_selectivity() clauselist_selectivity() assumes the clauses of an implicitly-ANDed list have independent selectivities and multiplies them together. For exact duplicates of the same clause that's maximally wrong: the duplicate filters out no additional rows, but the estimate shrinks exponentially with each repetition. With a 50%-NULL column, four copies of "x IS NULL" (as machine-generated queries and view-plus-WHERE stacking readily produce) underestimate the matching rows by 8x, which can then cascade into poor join plans. Other duplicate-clause shapes are already handled: equivalence-class processing removes duplicate equalities, prepqual.c pulls up duplicated OR arms, and the RangeQueryClause pairing in this file recognizes redundant inequalities. Duplicate NullTests were the most common remaining gap. Detecting duplicates of arbitrary clauses would require equal() matching on every pair of clauses, which benchmarks show is too slow. Instead, follow the RangeQueryClause precedent: track IS [NOT] NULL tests on plain Vars in a side list and count each distinct (Var, nulltesttype) combination only once. Comparing Var identity is three scalar comparisons plus a bms_equal() of the nulling relids, so clause lists without plain-Var NullTests see no overhead beyond one nodetag check per clause. --- src/backend/optimizer/path/clausesel.c | 93 ++++++++++++++++++ src/test/regress/expected/planner_est.out | 112 ++++++++++++++++++++++ src/test/regress/sql/planner_est.sql | 65 +++++++++++++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 271 insertions(+) diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c index 25c4d17..e35d6d3 100644 --- a/src/backend/optimizer/path/clausesel.c +++ b/src/backend/optimizer/path/clausesel.c @@ -38,8 +38,20 @@ typedef struct RangeQueryClause Selectivity hibound; /* Selectivity of a var < something clause */ } RangeQueryClause; +/* + * Data structure for detecting duplicate IS [NOT] NULL clauses in + * clauselist_selectivity. + */ +typedef struct NullTestClause +{ + struct NullTestClause *next; /* next in linked list */ + Var *var; /* the tested Var */ + NullTestType nulltesttype; /* IS NULL or IS NOT NULL */ +} NullTestClause; + static void addRangeClause(RangeQueryClause **rqlist, Node *clause, bool varonleft, bool isLTsel, Selectivity s2); +static bool duplicateNullTest(NullTestClause **ntlist, NullTest *ntest); static RelOptInfo *find_single_rel_for_clauses(PlannerInfo *root, List *clauses); static Selectivity clauselist_selectivity_or(PlannerInfo *root, @@ -95,6 +107,18 @@ static Selectivity clauselist_selectivity_or(PlannerInfo *root, * * Of course this is all very dependent on the behavior of the inequality * selectivity functions; perhaps some day we can generalize the approach. + * + * In a similar spirit, we count duplicate occurrences of an IS [NOT] NULL + * test on a plain Var only once. Applying the same test again filters out + * no additional rows, so multiplying its selectivity in again would just + * underestimate, and the error grows exponentially with the number of + * duplicates. Duplicates of other clause types survive to this point too, + * but detecting them in general would require expensive equal() matching, + * whereas comparing (varno, varattno, nulltesttype) triples is cheap. + * (Duplicate equality clauses are already removed by equivalence-class + * processing, duplicated OR arms by prepqual.c, and duplicate inequalities + * by the range-pairing logic above; NullTests are the most common + * remaining case, typically arising in machine-generated queries.) */ Selectivity clauselist_selectivity(PlannerInfo *root, @@ -125,6 +149,7 @@ clauselist_selectivity_ext(PlannerInfo *root, RelOptInfo *rel; Bitmapset *estimatedclauses = NULL; RangeQueryClause *rqlist = NULL; + NullTestClause *ntlist = NULL; ListCell *l; int listidx; @@ -168,6 +193,7 @@ clauselist_selectivity_ext(PlannerInfo *root, { Node *clause = (Node *) lfirst(l); RestrictInfo *rinfo; + Node *plainclause; Selectivity s2; listidx++; @@ -179,6 +205,26 @@ clauselist_selectivity_ext(PlannerInfo *root, if (bms_is_member(listidx, estimatedclauses)) continue; + /* + * Skip this clause if it's an IS [NOT] NULL test on a plain Var that + * duplicates one we've already counted (per the comments for + * clauselist_selectivity). We check the bare clause, so that a + * duplicate is recognized whether or not both occurrences are wrapped + * in RestrictInfos. + */ + plainclause = IsA(clause, RestrictInfo) ? + (Node *) ((RestrictInfo *) clause)->clause : clause; + if (IsA(plainclause, NullTest)) + { + NullTest *ntest = (NullTest *) plainclause; + + if (!ntest->argisrow && + IsA(ntest->arg, Var) && + ((Var *) ntest->arg)->varlevelsup == 0 && + duplicateNullTest(&ntlist, ntest)) + continue; + } + /* Compute the selectivity of this clause in isolation */ s2 = clause_selectivity_ext(root, clause, varRelid, jointype, sjinfo, use_extended_stats); @@ -338,6 +384,15 @@ clauselist_selectivity_ext(PlannerInfo *root, rqlist = rqnext; } + /* release the NullTest list, too */ + while (ntlist != NULL) + { + NullTestClause *ntnext = ntlist->next; + + pfree(ntlist); + ntlist = ntnext; + } + return s1; } @@ -513,6 +568,44 @@ addRangeClause(RangeQueryClause **rqlist, Node *clause, *rqlist = rqelem; } +/* + * duplicateNullTest --- detect duplicate IS [NOT] NULL clauses + * + * Returns true if an equivalent NullTest was seen earlier in the clause + * list, in which case the caller should ignore this one. Otherwise, + * remember the clause and return false. The caller has already verified + * that the argument is a plain Var of the current query level, so we + * need only compare the Var's identity and the test type, which is much + * cheaper than a full equal() check. Note that two Vars with the same + * varno/varattno necessarily match on type too, but we must still check + * varnullingrels and varreturningtype, which affect the Var's value. + */ +static bool +duplicateNullTest(NullTestClause **ntlist, NullTest *ntest) +{ + NullTestClause *ntelem; + Var *var = (Var *) ntest->arg; + + for (ntelem = *ntlist; ntelem; ntelem = ntelem->next) + { + if (ntelem->var->varno == var->varno && + ntelem->var->varattno == var->varattno && + ntelem->var->varreturningtype == var->varreturningtype && + bms_equal(ntelem->var->varnullingrels, var->varnullingrels) && + ntelem->nulltesttype == ntest->nulltesttype) + return true; + } + + /* Not a duplicate, so remember it */ + ntelem = palloc_object(NullTestClause); + ntelem->var = var; + ntelem->nulltesttype = ntest->nulltesttype; + ntelem->next = *ntlist; + *ntlist = ntelem; + + return false; +} + /* * find_single_rel_for_clauses * Examine each clause in 'clauses' and determine if all clauses diff --git a/src/test/regress/expected/planner_est.out b/src/test/regress/expected/planner_est.out index 236cb27..82fde3e 100644 --- a/src/test/regress/expected/planner_est.out +++ b/src/test/regress/expected/planner_est.out @@ -221,4 +221,116 @@ EXPLAIN (COSTS OFF) SELECT * FROM char_table_1 WHERE c < 'Q'; Filter: (c < 'Q'::"char") (2 rows) +-- +-- Test that clauselist_selectivity() counts duplicate IS [NOT] NULL +-- clauses only once rather than compounding their selectivities. +-- +-- exactly half the rows have a NULL "a" +CREATE TEMP TABLE dup_clause_tab AS + SELECT CASE WHEN i % 2 = 0 THEN i END AS a, i % 10 AS b + FROM generate_series(1, 1000) i; +ANALYZE dup_clause_tab; +-- expect the same 500-row estimate as a single IS NULL clause +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NULL AND a IS NULL;$$, +true, true, false, true); + explain_mask_costs +--------------------------------------------------------------------------------------- + Seq Scan on dup_clause_tab (cost=N..N rows=500 width=N) (actual rows=500.00 loops=1) + Filter: ((a IS NULL) AND (a IS NULL)) + Rows Removed by Filter: 500 +(3 rows) + +-- likewise for IS NOT NULL +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NOT NULL AND a IS NOT NULL;$$, +true, true, false, true); + explain_mask_costs +--------------------------------------------------------------------------------------- + Seq Scan on dup_clause_tab (cost=N..N rows=500 width=N) (actual rows=500.00 loops=1) + Filter: ((a IS NOT NULL) AND (a IS NOT NULL)) + Rows Removed by Filter: 500 +(3 rows) + +-- distinct clauses must still be multiplied: expect 1000 * 0.5 * 0.1 = 50 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NULL AND b = 1 AND a IS NULL;$$, +true, true, false, true); + explain_mask_costs +-------------------------------------------------------------------------------------- + Seq Scan on dup_clause_tab (cost=N..N rows=50 width=N) (actual rows=100.00 loops=1) + Filter: ((a IS NULL) AND (a IS NULL) AND (b = 1)) + Rows Removed by Filter: 900 +(3 rows) + +-- IS NULL and IS NOT NULL on the same column are not duplicates of each +-- other: expect 1000 * 0.5 * 0.5 = 250 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NULL AND a IS NOT NULL;$$, +false, true, false, true); + explain_mask_costs +---------------------------------------------------------- + Seq Scan on dup_clause_tab (cost=N..N rows=250 width=N) + Filter: ((a IS NULL) AND (a IS NOT NULL)) +(2 rows) + +-- Duplicate detection must compare the whole Var, not just the column +-- position: the same test on a same-named column of another relation is not +-- a duplicate. A FULL JOIN keeps the NullTests on both sides in the join +-- clause list (an inner join would push them down to the scans), so this +-- exercises both relations' Vars within a single clauselist_selectivity() +-- call. +CREATE TEMP TABLE dup_clause_tab2 AS + SELECT CASE WHEN i % 2 = 0 THEN i END AS a, i % 10 AS b + FROM generate_series(1, 1000) i; +ANALYZE dup_clause_tab2; +-- both selectivities must be applied: expect 1000 * 1000 * 0.1 * 0.5 * 0.5 +-- = 25000 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab t1 FULL JOIN dup_clause_tab2 t2 + ON t1.b = t2.b AND t1.a IS NULL AND t2.a IS NULL;$$, +false, true, false, true); + explain_mask_costs +--------------------------------------------------------------------------- + Hash Full Join (cost=N..N rows=25000 width=N) + Hash Cond: (t1.b = t2.b) + Join Filter: ((t1.a IS NULL) AND (t2.a IS NULL)) + -> Seq Scan on dup_clause_tab t1 (cost=N..N rows=1000 width=N) + -> Hash (cost=N..N rows=1000 width=N) + -> Seq Scan on dup_clause_tab2 t2 (cost=N..N rows=1000 width=N) +(6 rows) + +-- with each side's test duplicated, expect the same 25000-row estimate +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab t1 FULL JOIN dup_clause_tab2 t2 + ON t1.b = t2.b AND t1.a IS NULL AND t2.a IS NULL + AND t1.a IS NULL AND t2.a IS NULL;$$, +false, true, false, true); + explain_mask_costs +------------------------------------------------------------------------------------------ + Hash Full Join (cost=N..N rows=25000 width=N) + Hash Cond: (t1.b = t2.b) + Join Filter: ((t1.a IS NULL) AND (t2.a IS NULL) AND (t1.a IS NULL) AND (t2.a IS NULL)) + -> Seq Scan on dup_clause_tab t1 (cost=N..N rows=1000 width=N) + -> Hash (cost=N..N rows=1000 width=N) + -> Seq Scan on dup_clause_tab2 t2 (cost=N..N rows=1000 width=N) +(6 rows) + +-- control: with only one side tested the estimate must be higher, showing +-- the second relation's test above was not discarded as a duplicate: +-- expect 1000 * 1000 * 0.1 * 0.5 = 50000 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab t1 FULL JOIN dup_clause_tab2 t2 + ON t1.b = t2.b AND t1.a IS NULL;$$, +false, true, false, true); + explain_mask_costs +--------------------------------------------------------------------------- + Hash Full Join (cost=N..N rows=50000 width=N) + Hash Cond: (t1.b = t2.b) + Join Filter: (t1.a IS NULL) + -> Seq Scan on dup_clause_tab t1 (cost=N..N rows=1000 width=N) + -> Hash (cost=N..N rows=1000 width=N) + -> Seq Scan on dup_clause_tab2 t2 (cost=N..N rows=1000 width=N) +(6 rows) + DROP FUNCTION explain_mask_costs(text, bool, bool, bool, bool); diff --git a/src/test/regress/sql/planner_est.sql b/src/test/regress/sql/planner_est.sql index 2b696a4..07736f5 100644 --- a/src/test/regress/sql/planner_est.sql +++ b/src/test/regress/sql/planner_est.sql @@ -153,4 +153,69 @@ CREATE TEMP TABLE char_table_1 AS ANALYZE char_table_1; EXPLAIN (COSTS OFF) SELECT * FROM char_table_1 WHERE c < 'Q'; +-- +-- Test that clauselist_selectivity() counts duplicate IS [NOT] NULL +-- clauses only once rather than compounding their selectivities. +-- + +-- exactly half the rows have a NULL "a" +CREATE TEMP TABLE dup_clause_tab AS + SELECT CASE WHEN i % 2 = 0 THEN i END AS a, i % 10 AS b + FROM generate_series(1, 1000) i; +ANALYZE dup_clause_tab; + +-- expect the same 500-row estimate as a single IS NULL clause +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NULL AND a IS NULL;$$, +true, true, false, true); + +-- likewise for IS NOT NULL +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NOT NULL AND a IS NOT NULL;$$, +true, true, false, true); + +-- distinct clauses must still be multiplied: expect 1000 * 0.5 * 0.1 = 50 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NULL AND b = 1 AND a IS NULL;$$, +true, true, false, true); + +-- IS NULL and IS NOT NULL on the same column are not duplicates of each +-- other: expect 1000 * 0.5 * 0.5 = 250 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab WHERE a IS NULL AND a IS NOT NULL;$$, +false, true, false, true); + +-- Duplicate detection must compare the whole Var, not just the column +-- position: the same test on a same-named column of another relation is not +-- a duplicate. A FULL JOIN keeps the NullTests on both sides in the join +-- clause list (an inner join would push them down to the scans), so this +-- exercises both relations' Vars within a single clauselist_selectivity() +-- call. +CREATE TEMP TABLE dup_clause_tab2 AS + SELECT CASE WHEN i % 2 = 0 THEN i END AS a, i % 10 AS b + FROM generate_series(1, 1000) i; +ANALYZE dup_clause_tab2; + +-- both selectivities must be applied: expect 1000 * 1000 * 0.1 * 0.5 * 0.5 +-- = 25000 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab t1 FULL JOIN dup_clause_tab2 t2 + ON t1.b = t2.b AND t1.a IS NULL AND t2.a IS NULL;$$, +false, true, false, true); + +-- with each side's test duplicated, expect the same 25000-row estimate +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab t1 FULL JOIN dup_clause_tab2 t2 + ON t1.b = t2.b AND t1.a IS NULL AND t2.a IS NULL + AND t1.a IS NULL AND t2.a IS NULL;$$, +false, true, false, true); + +-- control: with only one side tested the estimate must be higher, showing +-- the second relation's test above was not discarded as a duplicate: +-- expect 1000 * 1000 * 0.1 * 0.5 = 50000 +SELECT explain_mask_costs($$ +SELECT * FROM dup_clause_tab t1 FULL JOIN dup_clause_tab2 t2 + ON t1.b = t2.b AND t1.a IS NULL;$$, +false, true, false, true); + DROP FUNCTION explain_mask_costs(text, bool, bool, bool, bool); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 56c1f99..65b2046 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1861,6 +1861,7 @@ NtDllRoutine NtFlushBuffersFileEx_t NullIfExpr NullTest +NullTestClause NullTestType NullableDatum NullingRelsMatch -- 2.54.0