BUG #19633: Unexpected results of IN (subquery) with a non-deterministic collation

чт, 20 авг. 2026 г. в 23:41, PG Bug reporting form <noreply@postgresql.org>:

The following bug has been logged on the website:

Bug reference: 19633
Logged by: Suyang Zhong
Email address: syzhong16@gmail.com
PostgreSQL version: 19beta3
Operating system: Ubuntu 22.04
Description:

Hi,

Consider the following test case:

```
CREATE COLLATION ci (provider = icu, locale = 'und-u-ks-level1',
deterministic = false);

CREATE TABLE t_lhs(c1 text COLLATE ci);
CREATE TABLE t_rhs(c0 text);
INSERT INTO t_lhs VALUES ('a'), ('x'), ('y');
INSERT INTO t_rhs VALUES ('a'), ('a');
ANALYZE t_lhs;
ANALYZE t_rhs;
INSERT INTO t_rhs VALUES ('A');

SELECT c1, c1 IN (SELECT c0 FROM t_rhs) AS p FROM t_lhs;
-- a | t
-- x | f
-- y | f

SELECT count(*) FROM t_lhs WHERE c1 IN (SELECT c0 FROM t_rhs);
-- Expected: 1, Actual: 2
```

Hi, Suyang!

Thanks for the report.

With enable_hashagg off that looks like:

Aggregate
-> Nested Loop
Join Filter: (t_semi_ci.c1 = ((t_semi_cs.c0)::text))
-> Unique
-> Sort
Sort Key: t_semi_cs.c0
-> Seq Scan on t_semi_cs
-> Seq Scan on t_semi_ci

Under gdb that path is create_unique_paths from the join search:

#0 create_unique_paths at planner.c:8673
#1 populate_joinrel_with_paths at joinrels.c:1189
#2 make_join_rel at joinrels.c:774
#3 make_rels_by_clause_joins at joinrels.c:300
#4 join_search_one_level at joinrels.c:123
#5 standard_join_search at allpaths.c:3987

(gdb) pgprint sjinfo
SpecialJoinInfo [jointype=JOIN_SEMI semi_can_btree=true
semi_can_hash=false]
[semi_operators] OidList: [98]
[semi_rhs_exprs]
Var [varno=3 varattno=1 vartype=25 varcollid=100]

Unique/HashAgg take the collation from the RHS expression. Here that
is the default collation of t_rhs.c0 (varcollid 100), not the join's
input collation (ci). So Sort+Unique keeps both 'a' and 'A'. Under
ci those values are equal, and the inner join emits the outer 'a'
twice (count is 2).

The attached patch labels each semi_rhs_expr with the join operator's
inputcollid via canonicalize_ec_expression (RelabelType when needed),
so unique-ification uses the same equality as the join. Sort then
shows

Sort Key: t_semi_cs.c0 COLLATE case_insensitive

and the count is 1. A regress case is included in collate.icu.utf8.

I am still getting familiar with this part. I am not sure this is the right
place or the right approach.

Thoughts?

--
Regards,
Rachitskiy Andrey

Hi Andrey,

Andrey Rachitskiy <pl0h0yp1@gmail.com> 于2026年8月21日周五 04:35写道：

чт, 20 авг. 2026 г. в 23:41, PG Bug reporting form <noreply@postgresql.org>:
Under gdb that path is create_unique_paths from the join search:

#0 create_unique_paths at planner.c:8673
#1 populate_joinrel_with_paths at joinrels.c:1189
#2 make_join_rel at joinrels.c:774
#3 make_rels_by_clause_joins at joinrels.c:300
#4 join_search_one_level at joinrels.c:123
#5 standard_join_search at allpaths.c:3987

(gdb) pgprint sjinfo
SpecialJoinInfo [jointype=JOIN_SEMI semi_can_btree=true
semi_can_hash=false]
[semi_operators] OidList: [98]
[semi_rhs_exprs]
Var [varno=3 varattno=1 vartype=25 varcollid=100]

Unique/HashAgg take the collation from the RHS expression. Here that
is the default collation of t_rhs.c0 (varcollid 100), not the join's
input collation (ci). So Sort+Unique keeps both 'a' and 'A'. Under
ci those values are equal, and the inner join emits the outer 'a'
twice (count is 2).

The attached patch labels each semi_rhs_expr with the join operator's
inputcollid via canonicalize_ec_expression (RelabelType when needed),
so unique-ification uses the same equality as the join. Sort then
shows

Sort Key: t_semi_cs.c0 COLLATE case_insensitive

and the count is 1. A regress case is included in collate.icu.utf8.

I am still getting familiar with this part. I am not sure this is the right place or the right approach.

Thoughts?

The approach looks good to me. I'd suggest adjusting the comment as follows:
...
/* so far so good, keep building lists */
semi_operators = lappend_oid(semi_operators, opno);

/*
* Ensure that the RHS expression exposes the join operator's input
* collation. The expression will later be used as a grouping key when
* unique-ifying the RHS, so its collation must agree with the semijoin
* equality semantics.
*/
semi_rhs_exprs =
lappend(semi_rhs_exprs,
canonicalize_ec_expression((Expr *) copyObject(right_expr),
exprType(right_expr),
op->inputcollid));
...

--
Thanks,
Tender Wang

пт, 21 авг. 2026 г. в 06:54, Tender Wang <tndrwang@gmail.com>:

The approach looks good to me. I'd suggest adjusting the comment as
follows:
...
/* so far so good, keep building lists */
semi_operators = lappend_oid(semi_operators, opno);

/*
* Ensure that the RHS expression exposes the join operator's input
* collation. The expression will later be used as a grouping key when
* unique-ifying the RHS, so its collation must agree with the semijoin
* equality semantics.
*/
semi_rhs_exprs =
lappend(semi_rhs_exprs,
canonicalize_ec_expression((Expr *) copyObject(right_expr),
exprType(right_expr),
op->inputcollid));

Dear Tender,

Thanks for the review.
Made the changes - v2 attached.

--
Regards,
Rachitskiy Andrey

Hi!

On Fri, Aug 21, 2026 at 8:02 AM Andrey Rachitskiy <pl0h0yp1@gmail.com> wrote:

пт, 21 авг. 2026 г. в 06:54, Tender Wang <tndrwang@gmail.com>:

The approach looks good to me. I'd suggest adjusting the comment as follows:
...
/* so far so good, keep building lists */
semi_operators = lappend_oid(semi_operators, opno);

/*
* Ensure that the RHS expression exposes the join operator's input
* collation. The expression will later be used as a grouping key when
* unique-ifying the RHS, so its collation must agree with the semijoin
* equality semantics.
*/
semi_rhs_exprs =
lappend(semi_rhs_exprs,
canonicalize_ec_expression((Expr *) copyObject(right_expr),
exprType(right_expr),
op->inputcollid));

Dear Tender,

Thanks for the review.
Made the changes - v2 attached.

I think this approach is right. Everybody who consumes
SpecialJoinInfo.semi_rhs_exprs takes the collation from the expression
itself, because SortGroupClause does not carry one and neither does
the pathkey machinery. So labelling the expression once, where it is
recorded, fixes every consumer at the same time; fixing it in the
consumers would mean doing the same thing in three places and getting
it right in each. The call is also exactly what process_equivalence()
already does for equivalence class members, with the same two
arguments and for the same reason, so this is not a new trick.

I've revised the patch. The v3 changes are:
1. The test now covers the hash path as well. v2 turned
enable_hashagg off, which is precisely the plan shape that was not
broken in isolation; both strategies are now exercised, each with its
plan and its result.
2. Test also checks results rows, not only the count. The bug is the
disagreement between the two: the predicate is true for one row while
filtering on it returned two.
3. Added a test for unique-ification being skipped because of a unique
index. Getting real coverage from it needs a little care: with only
'a' and 'A' in the RHS the planner keeps an ordinary semijoin and
answers correctly, so the test passes with or without the fix. Two
filler rows are enough to make unique-ification look worthwhile and
put the decision on the table. The plan is checked as well, so that a
future costing change cannot quietly turn this into a test of nothing.
4. Added a comment saying why the INSERT of 'A' comes after ANALYZE.
5. Reworded the code comment to say why rather than what:
unique-ification groups on this expression and takes the collation
from it, since SortGroupClause carries none.
6. Commit message now mentions HashAggregate, which v2 did not.

Now the part that is missing from the thread entirely. This is not a v19
regression. The line

semi_rhs_exprs = lappend(semi_rhs_exprs, copyObject(right_expr));

is identical in every supported branch back to 14, and the consumer
has the same shape there: REL_18's create_unique_path() builds a
SortGroupClause with no collation and takes the key from uniqexpr. So
this is a live wrong-results bug in all of them (I've checked it), and
I intend to backpatch it through 14.

------
Regards,
Alexander Korotkov
Supabase

On Thu, Sep 10, 2026 at 7:40 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:

I've revised the patch. The v3 changes are:
1. The test now covers the hash path as well. v2 turned
enable_hashagg off, which is precisely the plan shape that was not
broken in isolation; both strategies are now exercised, each with its
plan and its result.

I think it'd better to resue existing tables rather than create
new ones for the tests. Maybe we can reuse test1cs/test1ci,
test2cs/test2ci, test3cs/test3ci to reproduce this issue?

Also on master we have this new GUC enable_groupagg. Maybe it'd
better to use that instead of enable_sort?

- Richard