From d3654f95d09d39b09975b65fd5bfb4479c556cd8 Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Thu, 21 Dec 2023 11:27:57 +0800 Subject: [PATCH 1/8] init --- pkg/executor/aggfuncs/aggfunc_test.go | 10 -------- pkg/executor/aggfuncs/builder.go | 24 +++++++++++++++---- pkg/executor/aggfuncs/func_percentile_test.go | 8 +++++++ 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/pkg/executor/aggfuncs/aggfunc_test.go b/pkg/executor/aggfuncs/aggfunc_test.go index 4157adf9212ed..4b132ecc9b596 100644 --- a/pkg/executor/aggfuncs/aggfunc_test.go +++ b/pkg/executor/aggfuncs/aggfunc_test.go @@ -586,16 +586,6 @@ func testAggFunc(t *testing.T, p aggTest) { result, err = dt.Compare(ctx.GetSessionVars().StmtCtx.TypeCtx(), &p.results[1], ctor) require.NoError(t, err) require.Equalf(t, 0, result, "%v != %v", dt.String(), p.results[1]) - - // test the empty input - resultChk.Reset() - finalFunc.ResetPartialResult(finalPr) - err = finalFunc.AppendFinalResult2Chunk(ctx, finalPr, resultChk) - require.NoError(t, err) - dt = resultChk.GetRow(0).GetDatum(0, desc.RetTp) - result, err = dt.Compare(ctx.GetSessionVars().StmtCtx.TypeCtx(), &p.results[0], ctor) - require.NoError(t, err) - require.Equalf(t, 0, result, "%v != %v", dt.String(), p.results[0]) } func testAggFuncWithoutDistinct(t *testing.T, p aggTest) { diff --git a/pkg/executor/aggfuncs/builder.go b/pkg/executor/aggfuncs/builder.go index 9965976eda5b2..94dcb6e47617d 100644 --- a/pkg/executor/aggfuncs/builder.go +++ b/pkg/executor/aggfuncs/builder.go @@ -144,6 +144,25 @@ func buildApproxCountDistinct(aggFuncDesc *aggregation.AggFuncDesc, ordinal int) return nil } +func getEvalTypeForApproxPercentile(aggFuncDesc *aggregation.AggFuncDesc) types.EvalType { + arg0FieldType := aggFuncDesc.Args[0].GetType() + hasEnumSetAsIntFlag := (arg0FieldType.GetFlag() & mysql.EnumSetAsIntFlag) > 0 + if hasEnumSetAsIntFlag { + arg0FieldType.DelFlag(mysql.EnumSetAsIntFlag) + } + + evalType := arg0FieldType.EvalType() + if aggFuncDesc.Args[0].GetType().GetType() == mysql.TypeBit { + evalType = types.ETString // same as other aggregate function + } + + if hasEnumSetAsIntFlag { + arg0FieldType.SetFlag(mysql.EnumSetAsIntFlag) + } + + return evalType +} + func buildApproxPercentile(sctx sessionctx.Context, aggFuncDesc *aggregation.AggFuncDesc, ordinal int) AggFunc { if aggFuncDesc.Mode == aggregation.DedupMode { return nil @@ -159,10 +178,7 @@ func buildApproxPercentile(sctx sessionctx.Context, aggFuncDesc *aggregation.Agg base := basePercentile{percent: int(percent), baseAggFunc: baseAggFunc{args: aggFuncDesc.Args, ordinal: ordinal}} - evalType := aggFuncDesc.Args[0].GetType().EvalType() - if aggFuncDesc.Args[0].GetType().GetType() == mysql.TypeBit { - evalType = types.ETString // same as other aggregate function - } + evalType := getEvalTypeForApproxPercentile(aggFuncDesc) switch aggFuncDesc.Mode { case aggregation.CompleteMode, aggregation.Partial1Mode, aggregation.FinalMode: switch evalType { diff --git a/pkg/executor/aggfuncs/func_percentile_test.go b/pkg/executor/aggfuncs/func_percentile_test.go index 65f22466fd235..848e384eae336 100644 --- a/pkg/executor/aggfuncs/func_percentile_test.go +++ b/pkg/executor/aggfuncs/func_percentile_test.go @@ -46,7 +46,9 @@ func TestPercentile(t *testing.T) { testAggFunc(t, test) }) } +} +func TestFix26807(t *testing.T) { data := testSlice{} want := 28 for i := 1; i <= want; i++ { @@ -57,3 +59,9 @@ func TestPercentile(t *testing.T) { require.Equal(t, want, data[index]) } } + +func TestFix40463(t *testing.T) { + test := buildAggTester(ast.AggFuncApproxPercentile, mysql.TypeEnum, 5, nil, nil) + test.dataType.AddFlag(mysql.EnumSetAsIntFlag) + testAggFunc(t, test) +} From 98716d46694f61b7b8e64f7fd5d4d270becc5c36 Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Thu, 21 Dec 2023 14:17:04 +0800 Subject: [PATCH 2/8] tweaking --- pkg/executor/aggfuncs/builder.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/pkg/executor/aggfuncs/builder.go b/pkg/executor/aggfuncs/builder.go index 94dcb6e47617d..1e590d0fccd08 100644 --- a/pkg/executor/aggfuncs/builder.go +++ b/pkg/executor/aggfuncs/builder.go @@ -145,19 +145,10 @@ func buildApproxCountDistinct(aggFuncDesc *aggregation.AggFuncDesc, ordinal int) } func getEvalTypeForApproxPercentile(aggFuncDesc *aggregation.AggFuncDesc) types.EvalType { - arg0FieldType := aggFuncDesc.Args[0].GetType() - hasEnumSetAsIntFlag := (arg0FieldType.GetFlag() & mysql.EnumSetAsIntFlag) > 0 - if hasEnumSetAsIntFlag { - arg0FieldType.DelFlag(mysql.EnumSetAsIntFlag) - } - - evalType := arg0FieldType.EvalType() - if aggFuncDesc.Args[0].GetType().GetType() == mysql.TypeBit { - evalType = types.ETString // same as other aggregate function - } - - if hasEnumSetAsIntFlag { - arg0FieldType.SetFlag(mysql.EnumSetAsIntFlag) + evalType := aggFuncDesc.Args[0].GetType().EvalType() + argType := aggFuncDesc.Args[0].GetType().GetType() + if argType == mysql.TypeEnum || argType == mysql.TypeBit { + evalType = types.ETString } return evalType From d66146876bb85b3a5c3bb19b4370d390677a72c6 Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Thu, 21 Dec 2023 14:21:45 +0800 Subject: [PATCH 3/8] add comment --- pkg/executor/aggfuncs/builder.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/executor/aggfuncs/builder.go b/pkg/executor/aggfuncs/builder.go index 1e590d0fccd08..4bab0ef237621 100644 --- a/pkg/executor/aggfuncs/builder.go +++ b/pkg/executor/aggfuncs/builder.go @@ -147,6 +147,9 @@ func buildApproxCountDistinct(aggFuncDesc *aggregation.AggFuncDesc, ordinal int) func getEvalTypeForApproxPercentile(aggFuncDesc *aggregation.AggFuncDesc) types.EvalType { evalType := aggFuncDesc.Args[0].GetType().EvalType() argType := aggFuncDesc.Args[0].GetType().GetType() + + // When argType == mysql.TypeEnum, the field flag may be set with mysql.EnumSetAsIntFlag + // and the evalType will be set to types.ETInt, it's an unexpected behavior. if argType == mysql.TypeEnum || argType == mysql.TypeBit { evalType = types.ETString } From d5e329cd610183c05760a2b8e8a5cca7a836bd2d Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Thu, 21 Dec 2023 14:31:52 +0800 Subject: [PATCH 4/8] tweaking --- pkg/executor/aggfuncs/builder.go | 7 ++++--- pkg/executor/aggfuncs/func_percentile_test.go | 9 ++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/executor/aggfuncs/builder.go b/pkg/executor/aggfuncs/builder.go index 4bab0ef237621..788ff06c80ad2 100644 --- a/pkg/executor/aggfuncs/builder.go +++ b/pkg/executor/aggfuncs/builder.go @@ -148,9 +148,10 @@ func getEvalTypeForApproxPercentile(aggFuncDesc *aggregation.AggFuncDesc) types. evalType := aggFuncDesc.Args[0].GetType().EvalType() argType := aggFuncDesc.Args[0].GetType().GetType() - // When argType == mysql.TypeEnum, the field flag may be set with mysql.EnumSetAsIntFlag - // and the evalType will be set to types.ETInt, it's an unexpected behavior. - if argType == mysql.TypeEnum || argType == mysql.TypeBit { + // When argType == mysql.TypeEnum or argType == mysql.TypeSet, the field flag + // may be set with mysql.EnumSetAsIntFlag and the evalType will be set to types.ETInt, + // it's an unexpected behavior. + if argType == mysql.TypeEnum || argType == mysql.TypeSet || argType == mysql.TypeBit { evalType = types.ETString } diff --git a/pkg/executor/aggfuncs/func_percentile_test.go b/pkg/executor/aggfuncs/func_percentile_test.go index 848e384eae336..13f2800f9b9e2 100644 --- a/pkg/executor/aggfuncs/func_percentile_test.go +++ b/pkg/executor/aggfuncs/func_percentile_test.go @@ -61,7 +61,10 @@ func TestFix26807(t *testing.T) { } func TestFix40463(t *testing.T) { - test := buildAggTester(ast.AggFuncApproxPercentile, mysql.TypeEnum, 5, nil, nil) - test.dataType.AddFlag(mysql.EnumSetAsIntFlag) - testAggFunc(t, test) + types := []byte{mysql.TypeEnum, mysql.TypeSet} + for _, tp := range types { + test := buildAggTester(ast.AggFuncApproxPercentile, tp, 5, nil, nil) + test.dataType.AddFlag(mysql.EnumSetAsIntFlag) + testAggFunc(t, test) + } } From 4234ddf45bc21b7a80078afd72c6ed4466ba3005 Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Fri, 22 Dec 2023 09:00:18 +0800 Subject: [PATCH 5/8] tweaking --- pkg/executor/aggfuncs/builder.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/executor/aggfuncs/builder.go b/pkg/executor/aggfuncs/builder.go index 788ff06c80ad2..c283a185f9dca 100644 --- a/pkg/executor/aggfuncs/builder.go +++ b/pkg/executor/aggfuncs/builder.go @@ -148,9 +148,9 @@ func getEvalTypeForApproxPercentile(aggFuncDesc *aggregation.AggFuncDesc) types. evalType := aggFuncDesc.Args[0].GetType().EvalType() argType := aggFuncDesc.Args[0].GetType().GetType() - // When argType == mysql.TypeEnum or argType == mysql.TypeSet, the field flag - // may be set with mysql.EnumSetAsIntFlag and the evalType will be set to types.ETInt, - // it's an unexpected behavior. + // Sometimes `mysql.EnumSetAsIntFlag` may be set to true, such as when join, + // which is unexpected for `buildApproxPercentile` and `mysql.TypeEnum` and `mysql.TypeSet` will return unexpected `ETInt` here, + // so here `evalType` are forcibly set to `ETString`. if argType == mysql.TypeEnum || argType == mysql.TypeSet || argType == mysql.TypeBit { evalType = types.ETString } From 583c638ac34bda69a76d93c0a2da462d9b6ee5e1 Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Fri, 22 Dec 2023 10:10:49 +0800 Subject: [PATCH 6/8] tweaking --- pkg/executor/aggfuncs/builder.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/executor/aggfuncs/builder.go b/pkg/executor/aggfuncs/builder.go index c283a185f9dca..df31ceaa58171 100644 --- a/pkg/executor/aggfuncs/builder.go +++ b/pkg/executor/aggfuncs/builder.go @@ -151,6 +151,7 @@ func getEvalTypeForApproxPercentile(aggFuncDesc *aggregation.AggFuncDesc) types. // Sometimes `mysql.EnumSetAsIntFlag` may be set to true, such as when join, // which is unexpected for `buildApproxPercentile` and `mysql.TypeEnum` and `mysql.TypeSet` will return unexpected `ETInt` here, // so here `evalType` are forcibly set to `ETString`. + // For mysql.TypeBit, just same as other aggregate function. if argType == mysql.TypeEnum || argType == mysql.TypeSet || argType == mysql.TypeBit { evalType = types.ETString } From 8c133bad85e668ae430f947697741ab53bde8859 Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Fri, 29 Dec 2023 10:33:58 +0800 Subject: [PATCH 7/8] add test --- pkg/executor/test/executor/executor_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/executor/test/executor/executor_test.go b/pkg/executor/test/executor/executor_test.go index f5c5ab409c19c..ae0dc79888a51 100644 --- a/pkg/executor/test/executor/executor_test.go +++ b/pkg/executor/test/executor/executor_test.go @@ -2757,3 +2757,21 @@ func TestIssues49377(t *testing.T) { "limit 1" + ");").Sort().Check(testkit.Rows("1 Furina 1", "1 Furina 1", "1 Furina 1", "2 Klee 1", "2 Klee 1", "3 Eula 1", "3 Eula 1")) } + +func TestIssues40463(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + + tk.MustExec("use test;") + tk.MustExec("CREATE TABLE `4f380f26-9af6-4df8-959d-ad6296eff914` (`f7a9a4be-3728-449b-a5ea-df9b957eec67` enum('bkdv0','9rqy','lw','neud','ym','4nbv','9a7','bpkfo','xtfl','59','6vjj') NOT NULL DEFAULT 'neud', `43ca0135-1650-429b-8887-9eabcae2a234` set('8','5x47','xc','o31','lnz','gs5s','6yam','1','20ea','i','e') NOT NULL DEFAULT 'e', PRIMARY KEY (`f7a9a4be-3728-449b-a5ea-df9b957eec67`,`43ca0135-1650-429b-8887-9eabcae2a234`) /*T![clustered_index] CLUSTERED */) ENGINE=InnoDB DEFAULT CHARSET=ascii COLLATE=ascii_bin;") + tk.MustExec("INSERT INTO `4f380f26-9af6-4df8-959d-ad6296eff914` VALUES ('bkdv0','gs5s'),('lw','20ea'),('neud','8'),('ym','o31'),('4nbv','o31'),('xtfl','e');") + + tk.MustExec("CREATE TABLE `ba35a09f-76f4-40aa-9b48-13154a24bdd2` (`9b2a7138-14a3-4e8f-b29a-720392aad22c` set('zgn','if8yo','e','k7','bav','xj6','lkag','m5','as','ia','l3') DEFAULT 'zgn,if8yo,e,k7,ia,l3',`a60d6b5c-08bd-4a6d-b951-716162d004a5` set('6li6','05jlu','w','l','m','e9r','5q','d0ol','i6ajr','csf','d32') DEFAULT '6li6,05jlu,w,l,m,d0ol,i6ajr,csf,d32',`fb753d37-6252-4bd3-9bd1-0059640e7861` year(4) DEFAULT '2065', UNIQUE KEY `51816c39-27df-4bbe-a0e7-d6b6f54be2a2` (`fb753d37-6252-4bd3-9bd1-0059640e7861`), KEY `b0dfda0a-ffed-4c5b-9a72-4113bc1cbc8e` (`9b2a7138-14a3-4e8f-b29a-720392aad22c`,`fb753d37-6252-4bd3-9bd1-0059640e7861`)) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin /*T! SHARD_ROW_ID_BITS=5 */;") + tk.MustExec("insert into `ba35a09f-76f4-40aa-9b48-13154a24bdd2` values ('if8yo', '6li6,05jlu,w,l,m,d0ol,i6ajr,csf,d32', 2065);") + + tk.MustExec("CREATE TABLE `07ccc74e-14c3-4685-bb41-c78a069b1a6d` (`8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae` bigint(20) NOT NULL DEFAULT '-4604789462044748682',`30b19ecf-679f-4ca3-813f-d3c3b8f7da7e` date NOT NULL DEFAULT '5030-11-23',`1c52eaf2-1ebb-4486-9410-dfd00c7c835c` decimal(7,5) DEFAULT '-81.91307',`4b09dfdc-e688-41cb-9ffa-d03071a43077` float DEFAULT '1.7989023',PRIMARY KEY (`30b19ecf-679f-4ca3-813f-d3c3b8f7da7e`,`8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae`) /*T![clustered_index] CLUSTERED */,KEY `ae7a7637-ca52-443b-8a3f-69694f730cc4` (`8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae`),KEY `42640042-8a17-4145-9510-5bb419f83ed9` (`8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae`),KEY `839f4f5a-83f3-449b-a7dd-c7d2974d351a` (`30b19ecf-679f-4ca3-813f-d3c3b8f7da7e`),KEY `c474cde1-6fe4-45df-9067-b4e479f84149` (`8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae`),KEY `f834d0a9-709e-4ca8-925d-73f48322b70d` (`8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae`)) ENGINE=InnoDB DEFAULT CHARSET=gbk COLLATE=gbk_chinese_ci;") + tk.MustExec("set sql_mode=``;") + tk.MustExec("INSERT INTO `07ccc74e-14c3-4685-bb41-c78a069b1a6d` VALUES (616295989348159438,'0000-00-00',1.00000,1.7989023),(2215857492573998768,'1970-02-02',0.00000,1.7989023),(2215857492573998768,'1983-05-13',0.00000,1.7989023),(-2840083604831267906,'1984-01-30',1.00000,1.7989023),(599388718360890339,'1986-09-09',1.00000,1.7989023),(3506764933630033073,'1987-11-22',1.00000,1.7989023),(3506764933630033073,'2002-02-26',1.00000,1.7989023),(3506764933630033073,'2003-05-14',1.00000,1.7989023),(3506764933630033073,'2007-05-16',1.00000,1.7989023),(3506764933630033073,'2017-02-20',1.00000,1.7989023),(3506764933630033073,'2017-08-06',1.00000,1.7989023),(2215857492573998768,'2019-02-18',1.00000,1.7989023),(3506764933630033073,'2020-08-11',1.00000,1.7989023),(3506764933630033073,'2028-06-07',1.00000,1.7989023),(3506764933630033073,'2036-08-16',1.00000,1.7989023);") + + tk.MustQuery("select /*+ use_index_merge( `4f380f26-9af6-4df8-959d-ad6296eff914` ) */ /*+ stream_agg() */ approx_percentile( `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` , 77 ) as r0 , `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` as r1 from `4f380f26-9af6-4df8-959d-ad6296eff914` where not( IsNull( `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` ) ) and not( `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` in ( select `8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae` from `07ccc74e-14c3-4685-bb41-c78a069b1a6d` where `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` in ( select `a60d6b5c-08bd-4a6d-b951-716162d004a5` from `ba35a09f-76f4-40aa-9b48-13154a24bdd2` where not( `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` between 'bpkfo' and '59' ) and not( `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` in ( select `fb753d37-6252-4bd3-9bd1-0059640e7861` from `ba35a09f-76f4-40aa-9b48-13154a24bdd2` where IsNull( `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` ) or not( `4f380f26-9af6-4df8-959d-ad6296eff914`.`43ca0135-1650-429b-8887-9eabcae2a234` in ( select `8a93bdc5-2214-4f96-b5a7-1ba4c0d396ae` from `07ccc74e-14c3-4685-bb41-c78a069b1a6d` where IsNull( `4f380f26-9af6-4df8-959d-ad6296eff914`.`43ca0135-1650-429b-8887-9eabcae2a234` ) and not( `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67` between 'neud' and 'bpkfo' ) ) ) ) ) ) ) ) group by `4f380f26-9af6-4df8-959d-ad6296eff914`.`f7a9a4be-3728-449b-a5ea-df9b957eec67`;") +} From dab705f935f408645c99775955185bae27a57534 Mon Sep 17 00:00:00 2001 From: xzhangxian1008 Date: Fri, 29 Dec 2023 11:52:44 +0800 Subject: [PATCH 8/8] update --- pkg/executor/test/executor/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/executor/test/executor/BUILD.bazel b/pkg/executor/test/executor/BUILD.bazel index 5f088f9a45836..0eaf1b6493539 100644 --- a/pkg/executor/test/executor/BUILD.bazel +++ b/pkg/executor/test/executor/BUILD.bazel @@ -8,7 +8,7 @@ go_test( "main_test.go", ], flaky = True, - shard_count = 47, + shard_count = 48, deps = [ "//pkg/config", "//pkg/ddl",