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

On Mon, Sep 14, 2026 at 5:24 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> On Sat, Sep 12, 2026 at 10:57 PM Alexander Korotkov
> <aekorotkov(at)gmail(dot)com> wrote:
> > Any objections if I push (and backpatch) this?

> I'll have a look ASAP.

I looked at v6. The fix itself looks right to me. I have some
comments on the rest of the patch, though.

* Overall, the new comments in this patch are overly verbose. I think
we should keep them in line with the style of the existing comments.
Also, the new comment in compute_semijoin_info() talks only about the
collation, but the call also keeps the expression's original type,
which I think the comment should mention as well.

* In compute_semijoin_info(), the new comment and the relabeling call
are inserted between the two appends, right after

/* so far so good, keep building lists */

That comment is about building both lists, so it reads oddly with the
appends split apart. I'd rather relabel right_expr as a separate step
before that comment, and then keep the two appends together under it.

* The tests alter tables that other test cases in this file rely on,
which is really not a good practice.

INSERT INTO test2cs VALUES ('abc');
ALTER TABLE test2cs ALTER COLUMN x SET (n_distinct = 1);
ANALYZE test1ci, test2cs;
...
ANALYZE test1cs;

The INSERT is only there so that test2cs holds both 'abc' and 'ABC',
but test3cs already holds exactly that, and it also has the unique
index under case_sensitive, so it covers the unique-index case as
well. We don't need the n_distinct hack or the ANALYZEs either:
disabling seqscan and material is enough to make the planner
unique-ify test3cs, since the semijoin's inner index-only scan on
test3cs can't use the case_insensitive join clause as an index
condition and would have to be rescanned for every outer row.

* The same query

SELECT count(*) FROM test1ci
WHERE x COLLATE case_insensitive IN
(SELECT x FROM test2cs WHERE x IN ('abc', 'ABC'));

is run three times, once under the default settings without an EXPLAIN
and once under each forced strategy. The first run adds nothing, as
its plan is one of the other two and we can't even tell which.

* The last test, the one with "ANALYZE test1cs", pins a Hash Right
Semi Join, which is a plain semijoin that never unique-ifies, so on
master it doesn't exercise this patch at all. What it does exercise
is reduce_unique_semijoins(), which was fixed in 5a55ea507 and is
already covered by the "Ensure that semijoin is not reduced to
innerjoin" tests earlier in this file. The case the comment describes
only exists in the back branches, where create_unique_path() still
consults relation_has_unique_index_for() with semi_rhs_exprs, and
using test3cs as the RHS covers that with the same query.

* One more thing for the commit message. In the back branches
create_unique_path() also has the DISTINCT-subquery shortcut, which
goes through the collation-blind two-list query_is_distinct_for()
wrapper. This patch happens to close that too, because
translate_sub_tlist() punts on the RelabelType, so it's worth a
sentence there.

Attached is v7 with these changes.

- Richard