Richard Guo <[email protected]> wrote:
> Thanks. Fixed in v4. v4 also drops the costsize.c changes from 0003,
> as the costing issue has been fixed on master.
A few more comments:
* populate_joinrel_uniquekeys()
Shouldn't we care about the number of unique keys created by the combination
technique? If most / all relations do have the unique keys, the list of keys
can grow exponentially as we keep adding relations to the join.
* uniquekeys_match_join_clauses()
/*
* Nullable keys are fine here: rows with NULL key values cannot match
any
* outer row through these strict clauses.
*/
I'm not able to find where the callers check the strictness of the clauses.
Also, "Nullable" => "null-unaware" ?
* strengthen_uniquekeys_for_join()
In the header comment:
"The caller must have established that every clause holds of every output" =>
"...holds for ..." ?
I also tried to adjust one comment regarding "strengthening" of the unique
keys - see the attached diff. The original wording was hard form me to
understand, so I elaborated it more in detail. (Of course it's possible that I
missed the point altogether.)
Comments on regression tests:
v4-0001
-------
* The following
+-- a key column equated to a constant drops out of the key
+explain (costs off) select distinct b from uk_pk2 where a = 5;
+ QUERY PLAN
+----------------------------------------
+ Bitmap Heap Scan on uk_pk2
+ Recheck Cond: (a = 5)
+ -> Bitmap Index Scan on uk_pk2_pkey
+ Index Cond: (a = 5)
+(4 rows)
does not seem prove what the comment claims because explicit grouping is (for
different reason, probably due to the unique key not being a subset of the
DISTINCT clause) needed even without the WHERE clause:
explain (costs off) select distinct b from uk_pk2;
QUERY PLAN
--------------------------
HashAggregate
Group Key: b
-> Seq Scan on uk_pk2
(3 rows)
* To test the translation of the unique keys into the outer query,
+--
+-- Subquery-in-FROM keys
+--
+-- a DISTINCT inside is translated into the outer query
+explain (costs off) select distinct a, b from (select distinct a, b from
uk_pk2) s;
+ QUERY PLAN
+--------------------
+ Seq Scan on uk_pk2
+(1 row)
+
+-- likewise a GROUP BY
+explain (costs off) select distinct a, b from (select a, b from uk_pk2 group
by a, b) s;
+ QUERY PLAN
+--------------------
+ Seq Scan on uk_pk2
+(1 row)
I think neither DISTINCT nor GROUP BY is needed in the subquery. The subquery
should have the appropriate unique keys anyway:
explain (costs off) select distinct a, b from (select a, b from uk_pk2) s;
QUERY PLAN
--------------------
Seq Scan on uk_pk2
(1 row)
* What's the point of the OFFSET clause here?
+-- a key deduced inside the subquery's own join search
+explain (costs off)
+select distinct x from
+ (select uk_p.id as x from uk_p join uk_q on uk_p.id = uk_q.id offset 0) s;
+ QUERY PLAN
+----------------------------------
+ Hash Join
+ Hash Cond: (uk_p.id = uk_q.id)
+ -> Seq Scan on uk_p
+ -> Hash
+ -> Seq Scan on uk_q
+(5 rows)
I'm getting the same plan w/o that.
* I was wondering what's specific about CROSS JOIN (v03-0001)
+-- one side's key alone is not a key of a cross join
+explain (costs off) select distinct uk_p.id from uk_p cross join uk_q;
+ QUERY PLAN
+-----------------------------------------------------
+ Unique
+ -> Nested Loop
+ -> Index Only Scan using uk_p_pkey on uk_p
+ -> Materialize
+ -> Seq Scan on uk_q
+(5 rows)
but debugger session revealed that populate_plain_rel_uniquekeys() does not
find the ECs for uk_q. I think it's because the table is not mentioned in the
DISTINCT clause. Once I added it, the appropriate unique key seems to exist,
even when the join is CROSS JOIN:
explain (costs off) select distinct uk_p.id, uk_q.id from uk_p cross join uk_q;
QUERY PLAN
------------------------------
Nested Loop
-> Seq Scan on uk_p
-> Materialize
-> Seq Scan on uk_q
(4 rows)
* Regarding FULL JOIN
+-- a full join null-extends both sides, so nothing survives
+explain (costs off)
+select distinct uk_p.id from uk_p full join uk_q on uk_p.id = uk_q.id;
don't we also need a query whith "DISTINCT uk_q.id" clause?
* I'm not sure the unique keys are tested here
+-- the RHS of this semijoin is already distinct, so it is not unique-ified
+explain (costs off)
+select * from uk_p, uk_q where (uk_p.id, uk_q.id) in (select a, b from uk_pk2);
+ QUERY PLAN
+-----------------------------------------
+ Hash Join
+ Hash Cond: (uk_pk2.b = uk_q.id)
+ -> Hash Join
+ Hash Cond: (uk_pk2.a = uk_p.id)
+ -> Seq Scan on uk_pk2
+ -> Hash
+ -> Seq Scan on uk_p
+ -> Hash
+ -> Seq Scan on uk_q
+(9 rows)
I'm getting the same plan even if I comment out the fast path (i.e. the use of
the unique keys) in rel_is_distinct_for().
* Is this comment
+-- across a left join the RHS key covers only the matched rows
+explain (costs off) select distinct uk_d1.u from uk_d2 left join uk_d1 on
uk_d2.id = uk_d1.u;
saying that the RHS "loses its NULL-awareness", as tested earlier in the
script, or is it a different problem?
v4-0002
-------
* Maybe
"A partitioned table's unique index must include ..."
=>
"A partitioned table's unique index key must include ..."
* " ... an inheritance parent has none"
Does that mean " ... an inheritance parent has no unique indexes", because
inheritance parent is treated as if it had no indexes at all, per
get_relation_info()?
/*
* Make list of indexes. Ignore indexes on system catalogs if told to.
* Don't bother with indexes from traditional inheritance parents. For
* partitioned tables, we need a list of at least unique indexes as
these
* serve as unique proofs for certain planner optimizations. However,
* let's not discriminate here and just record all partitioned indexes
* whether they're unique indexes or not.
*/
v4-0003
-------
* The patch seems to handle grouped child relation, so it'd make sense to have
a test for that.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
>From 148ad41d7fc531ad55fbc7e79a7e5c15a587377d Mon Sep 17 00:00:00 2001
From: Antonin Houska <[email protected]>
Date: Fri, 21 Aug 2026 10:21:20 +0200
Subject: [PATCH] Attempt to reword a comment.
In particular, the phrase "so every output row satisfies them" seems
problematic because an output row of a semijoin is NULL-extended, so the
output row usually does not satisfy the join clauses.
---
src/backend/optimizer/path/uniquekeys.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/path/uniquekeys.c b/src/backend/optimizer/path/uniquekeys.c
index f9d9c37f447..34b2507d6b7 100644
--- a/src/backend/optimizer/path/uniquekeys.c
+++ b/src/backend/optimizer/path/uniquekeys.c
@@ -538,9 +538,14 @@ populate_joinrel_uniquekeys(PlannerInfo *root, RelOptInfo *joinrel,
if (jointype == JOIN_SEMI || jointype == JOIN_ANTI)
{
/*
- * A semijoin emits an LHS row only when some RHS row satisfied the
- * join clauses, so every output row satisfies them. An antijoin
- * emits exactly the rows that no RHS row matched.
+ * A semijoin emits a row only when the LHS satisfied the join clauses
+ * (for at least one RHS row). Given that all the current unique keys
+ * originate from the LHS, it's possible that rows having NULL in any
+ * unique key attribute are filtered out by the join clauses.
+ *
+ * (An antijoin emits exactly the rows that do not satisfy the join
+ * clauses, so rows having NULL in the unique key are not filtered
+ * out.)
*/
if (jointype == JOIN_SEMI)
strengthen_uniquekeys_for_join(root, joinrel, restrictlist);
--
2.52.0
