Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expression: don't cast collation for in expression is the new collation is disabled #52812

Merged
merged 5 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/planner/core/expression_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1951,6 +1951,9 @@ func (er *expressionRewriter) castCollationForIn(colLen int, elemCnt int, stkLen
if colLen != 1 {
return
}
if !collate.NewCollationEnabled() {
Copy link
Member

@YangKeao YangKeao Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment here to tell why castCollationForIn is disabled when new collation is not enabled? It's not intuitive. IIUC, it works because it didn't set coercibility to 0, so that the following panic (mix collation with 0 coercibility) will not happen.

I personally prefer to also enable the collation check in ColumnSubstituteImpl for old collation, but it may cause big performance impact and need manually workaround (rewriting SQL), so it's not good.
This solution is also fine for me, as I cannot think up any counter example in old collation.

return
}
for i := stkLen - elemCnt; i < stkLen; i++ {
// todo: consider refining the code and reusing expression.BuildCollationFunction here
if er.ctxStack[i].GetType().EvalType() == types.ETString {
Expand Down
13 changes: 13 additions & 0 deletions tests/integrationtest/r/collation_misc_disabled.result
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,16 @@ insert into t values ("1", "a ", "a");
select /*+USE_INDEX(t, idx)*/ * from t;
id a b
1 a a
drop table if exists t1;
drop table if exists t2;
create table t1(code varchar(32)) CHARSET=utf8 COLLATE=utf8_general_ci;
create table t2(code varchar(32)) CHARSET=utf8 COLLATE=utf8_bin;
desc format=brief select * from t1 join t2 on t1.code=t2.code and t1.code in ('1') and t2.code in ('1');
id estRows task access object operator info
HashJoin 12.50 root inner join, equal:[eq(cd_test_utf8mb4_0900_bin.t1.code, cd_test_utf8mb4_0900_bin.t2.code)]
├─TableReader(Build) 10.00 root data:Selection
│ └─Selection 10.00 cop[tikv] eq(cd_test_utf8mb4_0900_bin.t2.code, "1"), not(isnull(cd_test_utf8mb4_0900_bin.t2.code))
│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo
└─TableReader(Probe) 10.00 root data:Selection
└─Selection 10.00 cop[tikv] eq(cd_test_utf8mb4_0900_bin.t1.code, "1"), not(isnull(cd_test_utf8mb4_0900_bin.t1.code))
└─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo
14 changes: 14 additions & 0 deletions tests/integrationtest/r/collation_misc_enabled.result
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,17 @@ insert into t values ("1", "a ", "a");
select /*+USE_INDEX(t, idx)*/ * from t;
id a b
1 a a
drop table if exists t1;
drop table if exists t2;
create table t1(code varchar(32)) CHARSET=utf8 COLLATE=utf8_general_ci;
create table t2(code varchar(32)) CHARSET=utf8 COLLATE=utf8_bin;
desc format=brief select * from t1 join t2 on t1.code=t2.code and t1.code in ('1') and t2.code in ('1');
id estRows task access object operator info
Projection 80000.00 root cd_test_utf8mb4_0900_bin.t1.code, cd_test_utf8mb4_0900_bin.t2.code
└─HashJoin 80000.00 root CARTESIAN inner join
├─TableReader(Build) 10.00 root data:Selection
│ └─Selection 10.00 cop[tikv] eq(cd_test_utf8mb4_0900_bin.t2.code, "1")
│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo
└─TableReader(Probe) 8000.00 root data:Selection
└─Selection 8000.00 cop[tikv] eq(cd_test_utf8mb4_0900_bin.t1.code, "1")
└─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo
7 changes: 7 additions & 0 deletions tests/integrationtest/t/collation_misc.test
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,10 @@ use cd_test_utf8mb4_0900_bin;
create table t (id varchar(255) primary key clustered, a varchar(255) collate utf8mb4_0900_bin, b varchar(255) collate utf8mb4_bin, key idx(a, b));
insert into t values ("1", "a ", "a");
select /*+USE_INDEX(t, idx)*/ * from t;

# issue 52772
drop table if exists t1;
drop table if exists t2;
create table t1(code varchar(32)) CHARSET=utf8 COLLATE=utf8_general_ci;
create table t2(code varchar(32)) CHARSET=utf8 COLLATE=utf8_bin;
desc format=brief select * from t1 join t2 on t1.code=t2.code and t1.code in ('1') and t2.code in ('1');