From d0f22716437f9c5ab8d6c766f5e8daaae16a5c1d Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Sun, 17 Mar 2024 19:19:46 +0800 Subject: [PATCH 1/9] fix unstable test --- .../core/casetest/correlated/BUILD.bazel | 16 +++++ .../casetest/correlated/correlated_test.go | 64 +++++++++++++++++++ .../core/casetest/correlated/main_test.go | 38 +++++++++++ planner/core/logical_plan_builder.go | 23 +++++-- 4 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 pkg/planner/core/casetest/correlated/BUILD.bazel create mode 100644 pkg/planner/core/casetest/correlated/correlated_test.go create mode 100644 pkg/planner/core/casetest/correlated/main_test.go diff --git a/pkg/planner/core/casetest/correlated/BUILD.bazel b/pkg/planner/core/casetest/correlated/BUILD.bazel new file mode 100644 index 0000000000000..bd738c8dbab48 --- /dev/null +++ b/pkg/planner/core/casetest/correlated/BUILD.bazel @@ -0,0 +1,16 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "correlated_test", + timeout = "short", + srcs = [ + "correlated_test.go", + "main_test.go", + ], + flaky = True, + deps = [ + "//pkg/testkit", + "//pkg/testkit/testsetup", + "@org_uber_go_goleak//:goleak", + ], +) diff --git a/pkg/planner/core/casetest/correlated/correlated_test.go b/pkg/planner/core/casetest/correlated/correlated_test.go new file mode 100644 index 0000000000000..2f4785caae3ae --- /dev/null +++ b/pkg/planner/core/casetest/correlated/correlated_test.go @@ -0,0 +1,64 @@ +// Copyright 2023 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package correlated + +import ( + "testing" + + "github.com/pingcap/tidb/pkg/testkit" +) + +func TestCorrelatedSubquery(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec(`CREATE TABLE tlc07c2a51 ( + col_1 date DEFAULT NULL, + col_2 json NOT NULL, + col_3 varbinary(345) DEFAULT 'vE5ARCSlc%iI$Q', + col_4 json NOT NULL, + col_5 varchar(247) COLLATE utf8_general_ci NOT NULL, + col_6 bit(21) NOT NULL DEFAULT b'110000110101111111000', + col_7 bigint(20) NOT NULL DEFAULT '8151770874925830095', + PRIMARY KEY (col_7,col_5) /*T![clustered_index] CLUSTERED */ +) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_general_ci +PARTITION BY HASH (col_7) PARTITIONS 6;`) + tk.MustExec(`CREATE TABLE tc4cf4a6b ( + col_1 date DEFAULT NULL, + col_2 json NOT NULL, + col_3 varbinary(345) DEFAULT 'vE5ARCSlc%iI$Q', + col_4 json NOT NULL, + col_5 varchar(247) COLLATE utf8_general_ci NOT NULL, + col_6 bit(21) NOT NULL DEFAULT b'110000110101111111000', + col_7 bigint(20) NOT NULL DEFAULT '8151770874925830095', + PRIMARY KEY (col_7,col_5) /*T![clustered_index] CLUSTERED */ +) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_general_ci +PARTITION BY HASH (col_7) PARTITIONS 6;`) + tk.MustExec("INSERT INTO `tlc07c2a51` VALUES('2025-02-16','[\"xyJVLmVKsRn2ReGAfFkByZOztvVD8Xk5LGeLZo1WxYdG8LrA4WqO67szs9dvpLwK\", \"XQlzkZj81WXYJzMeVJkM1J2BVXNLyVlGc5g2TXUnh92o2NQp9vkAwsl8szXzNtmv\", \"CgcdqXY0HtrPc06XPeVPA6EcbqKnxVMS6TxkZObriQzKtlKv3HOEavEoyBdIFNac\"]',x'46245164543872483543','[\"yzegaiBdfb7svUI0JPXPzpoT8bkqQFexLr33ptDUtHWG3YLmrAvPU6hDP6h8YMOW\", \"gO9YIayc2kdwvnZ9362i2U9vQlM78COelLXxE9dGmvhBexkhvbEXXFEOyTATET6n\", \"TB3coCHSmOLXvzXt2KS9rRkotZ9fZk2JKwBRHoVR45EH8hW5gbrWbzBEwsTKKO41\"]','J%GY9U+&1#peaZ&Lg',x'04fd7c',1425801567592606408),('2025-02-16','[\"QEcM0XVHagvAoGbWAPkeAN26zG5WYnn2VPrVnDkAu71BGXQgkcTh8YOHPoAl6Plt\", \"Ba2NxxuOAXB40XzAowXIQlm0V2Pm2v65mY6j3w2tPxOeKwWopSgoUMmcNpYjlTlR\", \"HXry0elnp7ACh5tcenWOnh9sUeEjlTxquZLTfVfs5DuYzThBEqNtot7Fx3moiogI\"]',x'46245164543872483543','[\"E0yDwpJcR9qD4yDpZHw6DubUo2iG33MS3n8LKHCpbfMrCdOGMlzCCX8UYayiEi2c\", \"hL4tNhJwACwWuCvMRPkGRuA33cxrTZ8sNE4Fhh8LPpp5cboNPfm0EmOZBYzhwHbZ\", \"EfUYPQcEiHnlMN1LQa7vNMaGnEZ4k7epGKOxIbPsBQWtWe2fnOr7ZsZUSsp14Rqg\"]','FhP6pCikhV@_3*fH_',x'1e3ee7',2351169536288823809),('2001-04-19','[\"sWbccKqdpkgbdT2V94wPXU2pRT9UfngWMh0XClnCsvF2e3f3bnxJBlVrNXsT0VAh\", \"d4DpAYeD9lIl8ElznHJZX4iMisZFJlvguO0epZN02s8eMqsMJa8jUtOXkZeUlwYL\"]',x'7e665e6d76346d433724505e63516938','[\"NP5pVB7KUOaEa6NwD6oCIs8jCUE8d6RiExRkJwxt52rkuvGqxYV8wqdLV6mGVH2Z\", \"jLuDxB3x7Wr7enKMnW62OFcHJenidyCoPVmomerdTmmLycuqsWlb4SE2bx41gwjj\", \"tcrn9AuYmnwrvdHSdrD8KvJLOSW9gKGxzEukgAZlWK8cGVjjYTZO6YJcNcdmzCay\"]','Y^$',x'0b11a2',3652259273996203228),('2033-02-27','[\"osJeD8MsHuzSH0xdSQablscHNffpStRRrgdnhyDxrDq8eul7XjG36GpNZfgGnFci\", \"5Z8yHsJcLTWnP2yqK67te0WIAyMxFmPCW7EswNfNPAhXRxR7wOkYShUsqHxQLjbg\"]',x'54','[\"1PwgCYrA4KVwdCdV7E483ZpaPEjMtTv5MNrHQDkKlAEfS8FFY44G5olC3WPHhxZk\"]','%CZLL',x'12bce2',3717145498561167222),('2012-06-02','[\"V54Y14kWulMThN1zS9fW1kN5O74Q7WEh4I5aBSLTzTfXm3wRyLfb0Iq4psea6HZf\"]',x'614b594855416c513164','[\"Tw5f9qrnS5OleyctGGsB5eNFeM4TsCx7fOIt1y4y3v1TFldmvTbMmZccPP6gG2nf\", \"mNCgtgW12foWs3dQiUcCVnbRjkZyiiqXfGf3X3xItp4l40psarAXQgYl31gwSWwX\", \"SQxPqFHQHhilYzCCAjvpOVTXXhsRg0nBzKqdsvBD4xYsbNEmfE73ErKVYaMoD0qB\", \"bXPrB8c8va8Q5fPNReZUs8IBJKF0LDBzD2Tuwi2P4J8HRGElg7ftrbajtuKBSlpW\"]','+ix8V+ft8~uVf',x'069f5c',6189906848193069044),('2008-08-10','[\"V0vn67z0oCJdhvnr0riW4gITlMHpv5YanbNNzVXkWLTkEUDoGUY55FN2ii7C5r4h\", \"bp0N4YFYOzkDB2rDSzKNmJpfGmwykzpwb7vRykXPwEpXWui8rNWRTPynBsNnLpMg\"]',x'5e5236575a3974592441','[\"v8GdeCQYeGF7Om3SrunPEOd20VElZdDilpeMSKrUFxgS3TjbeakTqrocJTFz6BNZ\", \"lGmK6aGmdSh0oPg0cs9QWuvq2uKEuLZb6EruoenXEFuS5EHUyPYNXT2tsI5FSPJz\", \"cLSbqVhAVVez4Vksx84SLdjRTPZGrPBoKgvpn3y80ro5DEkg8BRubhJETzRaSKVK\"]','b#',x'0cdcc0',6546519514658297081),('2009-01-22','[\"IbPY5E7yenD30IaHAUL3qC59dtPYxwERoqfWp6rqpL7hsOaBbUm2vek5NZ0itZHQ\"]',x'2166256c662d6c30334e38454748','[\"8FF6cqAisQx1L6xtScBjYNXVf9Lql9bHaGNpXuVt7AWQaGAGXW2xiPAyzapGHiGG\"]','X~9~N9jHba#GV8tB4Vi',x'164e11',6607813116980509751),('2002-11-04','[\"Tf9RAVEBF9PHTSosZh5lWsc7LPrS9B24SG9yCqruuInClfYuUNtXY0MwZmrLCX8k\", \"e4TWSCiURA4V0agGj4id8Q6L0wLkCEbb0mtpMKTjtzeq527RvS1fCJvLtUZMTUXZ\", \"7mLipNkFUVvdEbfcPfsCu1b5bD4kILowxXtzXXwm81NwItdQwsdocpmzTszP1cAP\"]',x'7a2576','[\"tnoSerc2aAakQ3P9QDsg66lGTWkZ9Gh9M4vop9gJdMpqRJwemyWQJpFOTrOSZipj\", \"D1YFyRHunNdMKh17fVZoGMWylXkSBwrbReamEw7wXQBaQDq2Wa3OjWKylEZ29zM7\", \"T6jtYPW9FgcAfRWfcP8b9pNyxykO6GGIHgsCcmWUCmla8vOVhjhWrUJadQhFPitY\", \"ut7hb4CC5bH9KQmC0Zo8OtWrFPAfFkvcopQeLAcYkselVIN5au3DjFu6Q1Wh8Qjs\", \"tlNxKzCNxsjxgTpsRAyEXSbTvWdAFNZdbP5KRm0eD6sI7mZIegsZtFrXHtUIjJKz\"]','+m(=E-7BD_PTO4xAlN',x'1d22e5',7517561678520304673),('2025-02-16','[\"ih1dfdPoKD9NQqkguZJtRDLhiBz7YX0EUbxWaKoOj6Jcdfu6Z0rVvXOdYsefYoih\"]',x'46245164543872483543','[\"sN2f5pJoSizXgV7B2PgaNdtQFGdJnnmEFR1xslNzkGeoYI79JvYvQ2JYCBqvRGAm\", \"ZeY4KZAr9Ofkre3hoKRSbQJPf5ewPIkndMCZ0dRUqnwPRUjZBN5aaiXI3vefD8fY\"]','4FY*GRDPu#x',x'032849',7726971008804453429),('1994-09-29','[\"yuVOcYYUdna4b3mQqmuBfR3C03Nf7LLshm2zeWUSUgV5pXfpCDr5eKZh2KZwvBk7\"]',x'2a682a674964595f757240684837575e65','[\"UekvSaOwHY7q4cY7WKDQsIMl6JFI1NFMWXjAVYO2xazXcBommPNg23Xzdy9tLVca\", \"1R9FT97KQeeXAhjwnt7lydI74Yhwhs73w4JVUHPWeCA6VTKDhSxNgE29NncuJljX\", \"1J6C6axIsgZTarapkIQEc4cJjCRPaMGnPqmyQz4Fpp8yS0oK8yt91y0ryxWZnxcz\", \"dsKflwi46pGNv13HIkXKr34QqzfQ5xfdgEARdOceitD6vPgCp63D2AzQVVdhXLF2\"]','-hTu_Tv6rT@lv',x'18bae9',8059165531753386662);") + tk.MustExec("INSERT INTO `tc4cf4a6b` VALUES('1997-09-05','[\"eHmlQuuQLz6Cr6RJbYMupozlc9H4C4y9iALlRW3K3idQXqAZqmHzzv1AvN0Z6kLC\"]',x'6321553645395e4631652b','[\"KbyfwnLn5f5XskAkAopGODRErHa2g9M0tKWf1hiGfn3ermF9A3wqpGUVgBrP2Iux\", \"vmlfVpV4shEqeSjelAyQYEXolbiGyBfD1KTYnneiKPNbk2YmsIkysXQJBdjNA14L\"]','QyzHu~fb&u^V!',x'000000',-9223372036854775808),('1997-09-05','[\"hfiExxjCGXpidyDlzCBUwDksJH0yB7IKMrx95R1N2ZvoompbxobSORfJWu4NdZES\", \"WLBPyTfd7GxOKIlccTI2kwXyb4UFyBwvt4X3NbADJkpkefpZP1VB6sjO2y73vJwh\", \"V73KjNUyfJHLQ2oFaOcK721AA8QWwaPz7VTsQ5aevwG7lewlW4Y1evLpVMz2LIbn\"]',x'742873664b4a256b57613823346128643830','[\"KbyfwnLn5f5XskAkAopGODRErHa2g9M0tKWf1hiGfn3ermF9A3wqpGUVgBrP2Iux\", \"vmlfVpV4shEqeSjelAyQYEXolbiGyBfD1KTYnneiKPNbk2YmsIkysXQJBdjNA14L\"]','XSnyU0E07X2',x'000000',-9223372036854775808);") + tk.MustExec("analyze table tlc07c2a51;") + tk.MustExec("analyze table tc4cf4a6b;") + for i := 0; i < 10; i++ { + tk.MustQuery(`SELECT 1 +FROM tlc07c2a51 +WHERE NOT (tlc07c2a51.col_1>= + (SELECT GROUP_CONCAT(tc4cf4a6b.col_7 + ORDER BY tc4cf4a6b.col_7 SEPARATOR ',') AS r0 + FROM (tlc07c2a51) + JOIN tc4cf4a6b + WHERE ISNULL(tc4cf4a6b.col_3) + HAVING tlc07c2a51.col_6>1951988)) ;`).Check(testkit.Rows()) + } +} diff --git a/pkg/planner/core/casetest/correlated/main_test.go b/pkg/planner/core/casetest/correlated/main_test.go new file mode 100644 index 0000000000000..beea165ecd094 --- /dev/null +++ b/pkg/planner/core/casetest/correlated/main_test.go @@ -0,0 +1,38 @@ +// Copyright 2023 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package correlated + +import ( + "flag" + "testing" + + "github.com/pingcap/tidb/pkg/testkit/testsetup" + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + testsetup.SetupForCommonTest() + flag.Parse() + opts := []goleak.Option{ + goleak.IgnoreTopFunction("github.com/golang/glog.(*fileSink).flushDaemon"), + goleak.IgnoreTopFunction("github.com/bazelbuild/rules_go/go/tools/bzltestutil.RegisterTimeoutHandler.func1"), + goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), + goleak.IgnoreTopFunction("go.etcd.io/etcd/client/pkg/v3/logutil.(*MergeLogger).outputLoop"), + goleak.IgnoreTopFunction("gopkg.in/natefinch/lumberjack%2ev2.(*Logger).millRun"), + goleak.IgnoreTopFunction("github.com/tikv/client-go/v2/txnkv/transaction.keepAlive"), + goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"), + } + goleak.VerifyTestMain(m, opts...) +} diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index b7491c94c53dc..1a2f3aa977d41 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -2186,7 +2186,7 @@ func (a *havingWindowAndOrderbyExprResolver) Enter(n ast.Node) (node ast.Node, s return n, false } -func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameExpr, p LogicalPlan) (int, error) { +func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameExpr, p LogicalPlan, resolveFieldsFirst bool) (int, error) { idx, err := expression.FindFieldName(p.OutputNames(), v.Name) if err != nil { return -1, err @@ -2199,7 +2199,7 @@ func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameEx // retrieve the `t2.a` from the underlying join. switch x := p.(type) { case *LogicalLimit, *LogicalSelection, *LogicalTopN, *LogicalSort, *LogicalMaxOneRow: - return a.resolveFromPlan(v, p.Children()[0]) + return a.resolveFromPlan(v, p.Children()[0], resolveFieldsFirst) case *LogicalJoin: if len(x.fullNames) != 0 { idx, err = expression.FindFieldName(x.fullNames, v.Name) @@ -2226,6 +2226,19 @@ func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameEx return i, nil } } + // From https://github.com/pingcap/tidb/issues/51107 + // You should make the column in the having clause as the correlated column + // which is not relation with select's fields and GroupBy's fields. + // For SQLs like: + // SELECT * FROM `t1` WHERE NOT (`t1`.`col_1`>= ( + // SELECT `t2`.`col_7` + // FROM (`t1`) + // JOIN `t2` + // WHERE ISNULL(`t2`.`col_3`) HAVING `t1`.`col_6`>1951988) + // ) ; + if resolveFieldsFirst && a.curClause == havingClause && a.gbyItems == nil { + return -1, nil + } sf := &ast.SelectField{ Expr: &ast.ColumnNameExpr{Name: newColName}, Auxiliary: true, @@ -2301,11 +2314,11 @@ func (a *havingWindowAndOrderbyExprResolver) Leave(n ast.Node) (node ast.Node, o } if index == -1 { if a.curClause == orderByClause { - index, a.err = a.resolveFromPlan(v, a.p) + index, a.err = a.resolveFromPlan(v, a.p, resolveFieldsFirst) } else if a.curClause == havingClause && v.Name.Table.L != "" { // For SQLs like: // select a from t b having b.a; - index, a.err = a.resolveFromPlan(v, a.p) + index, a.err = a.resolveFromPlan(v, a.p, resolveFieldsFirst) if a.err != nil { return node, false } @@ -2325,7 +2338,7 @@ func (a *havingWindowAndOrderbyExprResolver) Leave(n ast.Node) (node ast.Node, o // We should ignore the err when resolving from schema. Because we could resolve successfully // when considering select fields. var err error - index, err = a.resolveFromPlan(v, a.p) + index, err = a.resolveFromPlan(v, a.p, resolveFieldsFirst) _ = err if index == -1 && a.curClause != fieldList && a.curClause != windowOrderByClause && a.curClause != partitionByClause { From 0c7724acfb28e03eae96af5125f16684b79816fc Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Thu, 21 Mar 2024 08:42:14 +0800 Subject: [PATCH 2/9] update Signed-off-by: Weizhen Wang --- planner/core/logical_plan_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 1a2f3aa977d41..2e12af1e61644 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -2236,7 +2236,7 @@ func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameEx // JOIN `t2` // WHERE ISNULL(`t2`.`col_3`) HAVING `t1`.`col_6`>1951988) // ) ; - if resolveFieldsFirst && a.curClause == havingClause && a.gbyItems == nil { + if resolveFieldsFirst && a.curClause == havingClause { return -1, nil } sf := &ast.SelectField{ From f66b4085a7f762412d658d047d696eacf0238504 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Thu, 21 Mar 2024 08:46:02 +0800 Subject: [PATCH 3/9] update Signed-off-by: Weizhen Wang --- .../core/casetest/correlated/correlated_test.go | 12 ++++++++++++ planner/core/logical_plan_builder.go | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/planner/core/casetest/correlated/correlated_test.go b/pkg/planner/core/casetest/correlated/correlated_test.go index 2f4785caae3ae..9fd52506cbc03 100644 --- a/pkg/planner/core/casetest/correlated/correlated_test.go +++ b/pkg/planner/core/casetest/correlated/correlated_test.go @@ -61,4 +61,16 @@ WHERE NOT (tlc07c2a51.col_1>= WHERE ISNULL(tc4cf4a6b.col_3) HAVING tlc07c2a51.col_6>1951988)) ;`).Check(testkit.Rows()) } + for i := 0; i < 10; i++ { + tk.MustQuery(`SELECT 1 +FROM tlc07c2a51 +WHERE NOT (tlc07c2a51.col_1>= + (SELECT GROUP_CONCAT(tc4cf4a6b.col_7 + ORDER BY tc4cf4a6b.col_7 SEPARATOR ',') AS r0 + FROM (tlc07c2a51) + JOIN tc4cf4a6b + WHERE ISNULL(tc4cf4a6b.col_3) + group by tlc07c2a51.col_6 + HAVING tlc07c2a51.col_6>1951988)) ;`).Check(testkit.Rows()) + } } diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 2e12af1e61644..1a2f3aa977d41 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -2236,7 +2236,7 @@ func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameEx // JOIN `t2` // WHERE ISNULL(`t2`.`col_3`) HAVING `t1`.`col_6`>1951988) // ) ; - if resolveFieldsFirst && a.curClause == havingClause { + if resolveFieldsFirst && a.curClause == havingClause && a.gbyItems == nil { return -1, nil } sf := &ast.SelectField{ From 35f5c50a94bde4bb35bf9dc64a07955a1c131a73 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Thu, 21 Mar 2024 09:00:12 +0800 Subject: [PATCH 4/9] update Signed-off-by: Weizhen Wang --- planner/core/logical_plan_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 1a2f3aa977d41..2e12af1e61644 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -2236,7 +2236,7 @@ func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameEx // JOIN `t2` // WHERE ISNULL(`t2`.`col_3`) HAVING `t1`.`col_6`>1951988) // ) ; - if resolveFieldsFirst && a.curClause == havingClause && a.gbyItems == nil { + if resolveFieldsFirst && a.curClause == havingClause { return -1, nil } sf := &ast.SelectField{ From c986fdc39b69d37ba85f698d9e59eb955b48f46f Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Thu, 21 Mar 2024 09:07:12 +0800 Subject: [PATCH 5/9] update Signed-off-by: Weizhen Wang --- pkg/planner/core/casetest/correlated/correlated_test.go | 6 +++--- planner/core/logical_plan_builder.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/planner/core/casetest/correlated/correlated_test.go b/pkg/planner/core/casetest/correlated/correlated_test.go index 9fd52506cbc03..461e095f9dc8b 100644 --- a/pkg/planner/core/casetest/correlated/correlated_test.go +++ b/pkg/planner/core/casetest/correlated/correlated_test.go @@ -65,12 +65,12 @@ WHERE NOT (tlc07c2a51.col_1>= tk.MustQuery(`SELECT 1 FROM tlc07c2a51 WHERE NOT (tlc07c2a51.col_1>= - (SELECT GROUP_CONCAT(tc4cf4a6b.col_7 + any (SELECT GROUP_CONCAT(tc4cf4a6b.col_7 ORDER BY tc4cf4a6b.col_7 SEPARATOR ',') AS r0 - FROM (tlc07c2a51) + FROM tlc07c2a51 JOIN tc4cf4a6b WHERE ISNULL(tc4cf4a6b.col_3) group by tlc07c2a51.col_6 - HAVING tlc07c2a51.col_6>1951988)) ;`).Check(testkit.Rows()) + HAVING tlc07c2a51.col_6>0)) ;`).Check(testkit.Rows()) } } diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 2e12af1e61644..1a2f3aa977d41 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -2236,7 +2236,7 @@ func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameEx // JOIN `t2` // WHERE ISNULL(`t2`.`col_3`) HAVING `t1`.`col_6`>1951988) // ) ; - if resolveFieldsFirst && a.curClause == havingClause { + if resolveFieldsFirst && a.curClause == havingClause && a.gbyItems == nil { return -1, nil } sf := &ast.SelectField{ From e916deae64c7f759e2199be288d1da30d916b560 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Thu, 21 Mar 2024 11:58:43 +0800 Subject: [PATCH 6/9] update Signed-off-by: Weizhen Wang --- .../core/casetest/correlated/correlated_test.go | 10 +++------- planner/core/logical_plan_builder.go | 4 +++- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/planner/core/casetest/correlated/correlated_test.go b/pkg/planner/core/casetest/correlated/correlated_test.go index 461e095f9dc8b..8c70500a67035 100644 --- a/pkg/planner/core/casetest/correlated/correlated_test.go +++ b/pkg/planner/core/casetest/correlated/correlated_test.go @@ -50,8 +50,7 @@ PARTITION BY HASH (col_7) PARTITIONS 6;`) tk.MustExec("INSERT INTO `tc4cf4a6b` VALUES('1997-09-05','[\"eHmlQuuQLz6Cr6RJbYMupozlc9H4C4y9iALlRW3K3idQXqAZqmHzzv1AvN0Z6kLC\"]',x'6321553645395e4631652b','[\"KbyfwnLn5f5XskAkAopGODRErHa2g9M0tKWf1hiGfn3ermF9A3wqpGUVgBrP2Iux\", \"vmlfVpV4shEqeSjelAyQYEXolbiGyBfD1KTYnneiKPNbk2YmsIkysXQJBdjNA14L\"]','QyzHu~fb&u^V!',x'000000',-9223372036854775808),('1997-09-05','[\"hfiExxjCGXpidyDlzCBUwDksJH0yB7IKMrx95R1N2ZvoompbxobSORfJWu4NdZES\", \"WLBPyTfd7GxOKIlccTI2kwXyb4UFyBwvt4X3NbADJkpkefpZP1VB6sjO2y73vJwh\", \"V73KjNUyfJHLQ2oFaOcK721AA8QWwaPz7VTsQ5aevwG7lewlW4Y1evLpVMz2LIbn\"]',x'742873664b4a256b57613823346128643830','[\"KbyfwnLn5f5XskAkAopGODRErHa2g9M0tKWf1hiGfn3ermF9A3wqpGUVgBrP2Iux\", \"vmlfVpV4shEqeSjelAyQYEXolbiGyBfD1KTYnneiKPNbk2YmsIkysXQJBdjNA14L\"]','XSnyU0E07X2',x'000000',-9223372036854775808);") tk.MustExec("analyze table tlc07c2a51;") tk.MustExec("analyze table tc4cf4a6b;") - for i := 0; i < 10; i++ { - tk.MustQuery(`SELECT 1 + tk.MustQuery(`SELECT 1 FROM tlc07c2a51 WHERE NOT (tlc07c2a51.col_1>= (SELECT GROUP_CONCAT(tc4cf4a6b.col_7 @@ -60,9 +59,7 @@ WHERE NOT (tlc07c2a51.col_1>= JOIN tc4cf4a6b WHERE ISNULL(tc4cf4a6b.col_3) HAVING tlc07c2a51.col_6>1951988)) ;`).Check(testkit.Rows()) - } - for i := 0; i < 10; i++ { - tk.MustQuery(`SELECT 1 + tk.MustQuery(`SELECT 1 FROM tlc07c2a51 WHERE NOT (tlc07c2a51.col_1>= any (SELECT GROUP_CONCAT(tc4cf4a6b.col_7 @@ -71,6 +68,5 @@ WHERE NOT (tlc07c2a51.col_1>= JOIN tc4cf4a6b WHERE ISNULL(tc4cf4a6b.col_3) group by tlc07c2a51.col_6 - HAVING tlc07c2a51.col_6>0)) ;`).Check(testkit.Rows()) - } + HAVING tlc07c2a51.col_6>0)) ;`).Check(testkit.Rows("1", "1", "1", "1", "1", "1", "1", "1", "1", "1")) } diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 1a2f3aa977d41..63c8efc66fc74 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -2236,7 +2236,9 @@ func (a *havingWindowAndOrderbyExprResolver) resolveFromPlan(v *ast.ColumnNameEx // JOIN `t2` // WHERE ISNULL(`t2`.`col_3`) HAVING `t1`.`col_6`>1951988) // ) ; - if resolveFieldsFirst && a.curClause == havingClause && a.gbyItems == nil { + // + // if resolveFieldsFirst is false, the groupby is not nil. + if resolveFieldsFirst && a.curClause == havingClause { return -1, nil } sf := &ast.SelectField{ From 079f9d3725776ad23a1fdd34aac4be301b501fb9 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 8 May 2024 14:21:22 +0800 Subject: [PATCH 7/9] *: upgrade Signed-off-by: Weizhen Wang --- {pkg/planner => planner}/core/casetest/correlated/BUILD.bazel | 4 ++-- .../core/casetest/correlated/correlated_test.go | 2 +- .../planner => planner}/core/casetest/correlated/main_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename {pkg/planner => planner}/core/casetest/correlated/BUILD.bazel (81%) rename {pkg/planner => planner}/core/casetest/correlated/correlated_test.go (99%) rename {pkg/planner => planner}/core/casetest/correlated/main_test.go (96%) diff --git a/pkg/planner/core/casetest/correlated/BUILD.bazel b/planner/core/casetest/correlated/BUILD.bazel similarity index 81% rename from pkg/planner/core/casetest/correlated/BUILD.bazel rename to planner/core/casetest/correlated/BUILD.bazel index bd738c8dbab48..a293ee1ef2697 100644 --- a/pkg/planner/core/casetest/correlated/BUILD.bazel +++ b/planner/core/casetest/correlated/BUILD.bazel @@ -9,8 +9,8 @@ go_test( ], flaky = True, deps = [ - "//pkg/testkit", - "//pkg/testkit/testsetup", + "//testkit", + "//testkit/testsetup", "@org_uber_go_goleak//:goleak", ], ) diff --git a/pkg/planner/core/casetest/correlated/correlated_test.go b/planner/core/casetest/correlated/correlated_test.go similarity index 99% rename from pkg/planner/core/casetest/correlated/correlated_test.go rename to planner/core/casetest/correlated/correlated_test.go index 8c70500a67035..6f3fe70438853 100644 --- a/pkg/planner/core/casetest/correlated/correlated_test.go +++ b/planner/core/casetest/correlated/correlated_test.go @@ -17,7 +17,7 @@ package correlated import ( "testing" - "github.com/pingcap/tidb/pkg/testkit" + "github.com/pingcap/tidb/testkit" ) func TestCorrelatedSubquery(t *testing.T) { diff --git a/pkg/planner/core/casetest/correlated/main_test.go b/planner/core/casetest/correlated/main_test.go similarity index 96% rename from pkg/planner/core/casetest/correlated/main_test.go rename to planner/core/casetest/correlated/main_test.go index beea165ecd094..c047722bc81df 100644 --- a/pkg/planner/core/casetest/correlated/main_test.go +++ b/planner/core/casetest/correlated/main_test.go @@ -18,7 +18,7 @@ import ( "flag" "testing" - "github.com/pingcap/tidb/pkg/testkit/testsetup" + "github.com/pingcap/tidb/testkit/testsetup" "go.uber.org/goleak" ) From 0b792e26de798d533f1e598480b7572507666486 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 8 May 2024 15:04:49 +0800 Subject: [PATCH 8/9] *: upgrade Signed-off-by: Weizhen Wang --- planner/core/casetest/correlated/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/casetest/correlated/main_test.go b/planner/core/casetest/correlated/main_test.go index c047722bc81df..a0123c5f5a28c 100644 --- a/planner/core/casetest/correlated/main_test.go +++ b/planner/core/casetest/correlated/main_test.go @@ -26,7 +26,7 @@ func TestMain(m *testing.M) { testsetup.SetupForCommonTest() flag.Parse() opts := []goleak.Option{ - goleak.IgnoreTopFunction("github.com/golang/glog.(*fileSink).flushDaemon"), + goleak.IgnoreTopFunction(" github.com/golang/glog.(*loggingT).flushDaemon"), goleak.IgnoreTopFunction("github.com/bazelbuild/rules_go/go/tools/bzltestutil.RegisterTimeoutHandler.func1"), goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), goleak.IgnoreTopFunction("go.etcd.io/etcd/client/pkg/v3/logutil.(*MergeLogger).outputLoop"), From 6be4f37b2b1aec14b9664518e5d00511b10923a1 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 8 May 2024 15:25:15 +0800 Subject: [PATCH 9/9] *: upgrade Signed-off-by: Weizhen Wang --- planner/core/casetest/correlated/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/casetest/correlated/main_test.go b/planner/core/casetest/correlated/main_test.go index a0123c5f5a28c..63e637f6da22d 100644 --- a/planner/core/casetest/correlated/main_test.go +++ b/planner/core/casetest/correlated/main_test.go @@ -26,7 +26,7 @@ func TestMain(m *testing.M) { testsetup.SetupForCommonTest() flag.Parse() opts := []goleak.Option{ - goleak.IgnoreTopFunction(" github.com/golang/glog.(*loggingT).flushDaemon"), + goleak.IgnoreTopFunction("github.com/golang/glog.(*loggingT).flushDaemon"), goleak.IgnoreTopFunction("github.com/bazelbuild/rules_go/go/tools/bzltestutil.RegisterTimeoutHandler.func1"), goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), goleak.IgnoreTopFunction("go.etcd.io/etcd/client/pkg/v3/logutil.(*MergeLogger).outputLoop"),