Re: Wrong result when enable_partitionwise_join is on if collation of PartitionKey and Column is different.
Amit Langote <amitlangote09@gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
To: jian he <jian.universality@gmail.com>
Cc: Junwang Zhao <zhjwpku@gmail.com>, Tender Wang <tndrwang@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: 2024-11-05T07:55:25Z
Lists: pgsql-hackers
Attachments
- v6-0002-Disallow-partitionwise-join-when-collation-doesn-.patch (application/octet-stream) patch v6-0002
- v6-0001-Disallow-partitionwise-grouping-when-collation-do.patch (application/octet-stream) patch v6-0001
Hi,
On Mon, Nov 4, 2024 at 12:28 PM jian he <jian.universality@gmail.com> wrote:
>
> hi.
> about v5.
> if (exprs_known_equal(root, expr1, expr2, btree_opfamily))
> {
> /*
> * Ensure that the collation of the expression matches
> * that of the partition key. Checking just one collation
> * (partcoll1 and exprcoll1) suffices because partcoll1
> * and partcoll2, as well as exprcoll1 and exprcoll2,
> * should be identical. This holds because both rel1 and
> * rel2 use the same PartitionScheme and expr1 and expr2
> * are equal.
> */
> if (partcoll1 == exprcoll1)
> {
> Oid partcoll2 PG_USED_FOR_ASSERTS_ONLY =
> rel1->part_scheme->partcollation[ipk];
> Oid exprcoll2 PG_USED_FOR_ASSERTS_ONLY =
> exprCollation(expr2);
> Assert(partcoll2 == exprcoll2);
> pk_known_equal[ipk] = true;
> if (OidIsValid(exprcoll1))
> elog(INFO, "this path called %s:%d",
> __FILE_NAME__, __LINE__);
> break;
> }
> }
>
> tests still passed, which means that we didn't have text data type as
> partition key related tests for partition-wise join.
> Do we need to add one?
>
> +-- Another case where the partition keys are matched via equivalence class,
> +-- not a join restriction clause.
> +
> +-- OK when the join clause uses the same collation as the partition key
> +EXPLAIN (COSTS OFF)
> +SELECT t1.c, count(t2.c) FROM pagg_tab3 t1 JOIN pagg_tab4 t2 ON t1.c
> = t2.c AND t1.c = t2.b COLLATE "C" GROUP BY 1 ORDER BY 1;
>
> i suppose, you comments is saying that in have_partkey_equi_join
> the above query will return true via
> `if (exprs_known_equal(root, expr1, expr2, btree_opfamily))`
> But " t1.c = t2.b COLLATE "C" already in "restrictlist".
> In have_partkey_equi_join loop through "restrictlist" would return
> true for above query, won't reach exprs_known_equal.
>
> Other than the comments that confused me, the test and the results
> look fine to me.
Thanks, yes, a test case that exercises the partcoll1 == exprcoll1
code was missing.
> some column collation is case_insensitive, ORDER BY that column would
> render the output not deterministic.
> like 'A' before 'a' and 'a' before 'A' are both correct.
> it may cause regress tests to fail.
> So I did some minor refactoring to make the "ORDER BY" deterministic.
Thanks, merged.
--
Thanks, Amit Langote
Commits
-
Disallow partitionwise join when collations don't match
- 075acdd93388 18.0 landed
- a0cdfc889367 17.1 landed
- f734b6b4d0cc 16.5 landed
- 33040b1715c7 15.9 landed
- 62df5484f976 14.14 landed
- 054701a2b77a 13.17 landed
- 9c4757491683 12.21 landed
-
Disallow partitionwise grouping when collations don't match
- 90fe6251c816 18.0 landed
- b6484ca9535e 17.1 landed
- dd2f8ebee221 16.5 landed
- 0a620659c549 15.9 landed
- 96f9b29a3e1e 14.14 landed
- ff65f695c0d3 13.17 landed
- 46d9be5efb1a 12.21 landed