executor: remove Next function for HashAggExec #5987

lwhile · 2018-03-09T08:23:39Z

No description provided.

sre-bot · 2018-03-09T08:23:43Z

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

CLAassistant · 2018-03-09T08:23:46Z

All committers have signed the CLA.

zz-jason · 2018-03-09T08:30:23Z

executor/aggregate.go

-	// In this stage we consider all data from src as a single group.
-	if !e.executed {
-		for {
-			hasMore, err := e.innerNext(ctx)


can we remove innerNext ?

I think so, let me remove it.

zz-jason · 2018-03-09T08:34:59Z

@lwhile please fix ci.

zz-jason · 2018-03-09T08:39:12Z

to #5985

lwhile · 2018-03-09T08:49:45Z

@zz-jason Ok, I will fix it tonight or tomorrow. If RP has the time limit, please let me know.

lwhile · 2018-03-10T07:10:22Z

@zz-jason Can I remove all the Next test in executor/executor_test.go?

zz-jason · 2018-03-10T07:19:07Z

@lwhile No. We should rewrite them instead.

lwhile · 2018-03-10T07:23:35Z

@zz-jason I am confused about the output of CI and I have no debug environment.Please give me some Infomation for fix CI.

XuHuaiyu · 2018-03-11T02:14:14Z

@lwhile You can use make dev to check whether your code could pass the UTs.

XuHuaiyu · 2018-03-11T02:16:07Z

FAIL: admin_test.go:204: testSuite.TestScan

admin_test.go:287:
s.testIndex(c, ctx, db.L, tb, tb.Indices()[0])
admin_test.go:356:
c.Assert(err, IsNil)
... value *errors.Err = &errors.Err{message:"", cause:(*errors.Err)(0xc420fdb040), previous:(errors.Err)(0xc420fdb040), file:"/go/src/github.com/pingcap/tidb/util/admin/admin.go", line:199} ("can not get count, sql SELECT COUNT() FROM test_admin.t USE INDEX(c) result rows 0")

zz-jason · 2018-03-18T06:03:10Z

@lwhile any update ?

shenli · 2018-03-18T13:12:02Z

/run-all-tests

shenli · 2018-03-18T13:27:46Z

@lwhile A friendly ping. Please fix the CI.

lwhile · 2018-03-19T05:09:32Z

@shenli Sorry, I will try to fix the errors in the next few days :)

lwhile · 2018-03-20T10:35:14Z

@shenli Hi, I could pass all the test in my local environment by using the command make dev after I merge the latest branch master to my patch, but the online CI says

./executor/aggregate.go
Makefile:75: recipe for target 'check' failed
make: *** [check] Error 1
Exited with code 2

Could give me more tips?

XuHuaiyu · 2018-03-20T10:37:32Z

You can run go fmt ./... in executor directory.

tiancaiamao · 2018-03-20T10:40:04Z

What's your Go version? You need the latest Go to format the source files. @lwhile

lwhile · 2018-03-20T11:03:28Z

Can pass the CI check now. @XuHuaiyu

Thanks for your help. I am a reader of your blog, and your article is very well. @tiancaiamao

XuHuaiyu · 2018-03-21T11:41:28Z

LGTM
PTAL @zz-jason

zz-jason · 2018-03-21T11:45:51Z

executor/aggregate.go

@@ -143,37 +143,7 @@ func (e *HashAggExec) execute(ctx context.Context) (err error) {

 // Next implements the Executor Next interface.
 func (e *HashAggExec) Next(ctx context.Context) (Row, error) {


we can remore Next function totally now.

zz-jason · 2018-03-22T04:42:47Z

/run-all-tests

zz-jason

LGTM

lwhile · 2018-03-22T05:03:45Z

@zz-jason The Jenkins test didn't pass.Should I fix it?

zz-jason · 2018-03-22T05:12:33Z

@lwhile It's a known issue, not your fault, we are working on it

zz-jason · 2018-03-22T05:15:41Z

/run-common-test
/run-integration-common-test

zz-jason · 2018-03-22T05:52:40Z

/run-common-test
/run-integration-common-test

zz-jason · 2018-03-22T06:09:59Z

/run-integration-common-test

Remove HashAggExec Next method of executor/aggregate.go

d4ba984

zz-jason reviewed Mar 9, 2018

View reviewed changes

zz-jason changed the title ~~Remove HashAggExec Next method of executor/aggregate.go~~ executor: remove Next function for HashAggExec Mar 9, 2018

XuHuaiyu mentioned this pull request Mar 9, 2018

remove Next() function of all the Executors #5985

Closed

41 tasks

Remove HashAggExec innerNext method of executor/aggregate.go

5e71985

zz-jason added the contribution This PR is from a community contributor. label Mar 9, 2018

Merge branch 'master' into patch-3

60fcb18

Merge branch 'master' into patch-3

6365057

lwhile added 2 commits March 19, 2018 15:32

Merge branch 'master' into patch-3

d609b73

Merge branch 'master' into patch-3

8d0a515

run gofmt tool

9b9d5ab

lwhile force-pushed the patch-3 branch from 0d1f34f to 9b9d5ab Compare March 20, 2018 10:53

lwhile added 2 commits March 20, 2018 19:03

Merge branch 'master' into patch-3

0672337

Merge branch 'master' into patch-3

84f2d3c

XuHuaiyu added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 21, 2018

zz-jason reviewed Mar 21, 2018

View reviewed changes

lwhile added 2 commits March 22, 2018 12:18

Merge branch 'master' into patch-3

0572b00

Remove Next function totally

273e2f2

lwhile force-pushed the patch-3 branch from 09b93bb to 273e2f2 Compare March 22, 2018 04:23

zz-jason approved these changes Mar 22, 2018

View reviewed changes

zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 22, 2018

lwhile and others added 2 commits March 22, 2018 13:47

Merge branch 'master' into patch-3

7b422c6

Merge branch 'master' into patch-3

26f12ca

Merge branch 'master' into patch-3

05d5dfd

ngaut merged commit 861103b into pingcap:master Mar 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor: remove Next function for HashAggExec #5987

executor: remove Next function for HashAggExec #5987

lwhile commented Mar 9, 2018

sre-bot commented Mar 9, 2018

CLAassistant commented Mar 9, 2018 •

edited

Loading

zz-jason Mar 9, 2018

lwhile Mar 9, 2018

zz-jason commented Mar 9, 2018

zz-jason commented Mar 9, 2018

lwhile commented Mar 9, 2018 •

edited

Loading

lwhile commented Mar 10, 2018

zz-jason commented Mar 10, 2018

lwhile commented Mar 10, 2018 •

edited

Loading

XuHuaiyu commented Mar 11, 2018

XuHuaiyu commented Mar 11, 2018

zz-jason commented Mar 18, 2018

shenli commented Mar 18, 2018

shenli commented Mar 18, 2018

lwhile commented Mar 19, 2018

lwhile commented Mar 20, 2018 •

edited

Loading

XuHuaiyu commented Mar 20, 2018

tiancaiamao commented Mar 20, 2018

lwhile commented Mar 20, 2018 •

edited

Loading

XuHuaiyu commented Mar 21, 2018

zz-jason Mar 21, 2018 •

edited

Loading

lwhile Mar 22, 2018

zz-jason commented Mar 22, 2018

zz-jason left a comment

lwhile commented Mar 22, 2018

zz-jason commented Mar 22, 2018

zz-jason commented Mar 22, 2018

zz-jason commented Mar 22, 2018

zz-jason commented Mar 22, 2018

		@@ -143,37 +143,7 @@ func (e *HashAggExec) execute(ctx context.Context) (err error) {

		// Next implements the Executor Next interface.
		func (e *HashAggExec) Next(ctx context.Context) (Row, error) {

executor: remove Next function for HashAggExec #5987

executor: remove Next function for HashAggExec #5987

Conversation

lwhile commented Mar 9, 2018

sre-bot commented Mar 9, 2018

CLAassistant commented Mar 9, 2018 • edited Loading

zz-jason Mar 9, 2018

Choose a reason for hiding this comment

lwhile Mar 9, 2018

Choose a reason for hiding this comment

zz-jason commented Mar 9, 2018

zz-jason commented Mar 9, 2018

lwhile commented Mar 9, 2018 • edited Loading

lwhile commented Mar 10, 2018

zz-jason commented Mar 10, 2018

lwhile commented Mar 10, 2018 • edited Loading

XuHuaiyu commented Mar 11, 2018

XuHuaiyu commented Mar 11, 2018

zz-jason commented Mar 18, 2018

shenli commented Mar 18, 2018

shenli commented Mar 18, 2018

lwhile commented Mar 19, 2018

lwhile commented Mar 20, 2018 • edited Loading

XuHuaiyu commented Mar 20, 2018

tiancaiamao commented Mar 20, 2018

lwhile commented Mar 20, 2018 • edited Loading

XuHuaiyu commented Mar 21, 2018

zz-jason Mar 21, 2018 • edited Loading

Choose a reason for hiding this comment

lwhile Mar 22, 2018

Choose a reason for hiding this comment

zz-jason commented Mar 22, 2018

zz-jason left a comment

Choose a reason for hiding this comment

lwhile commented Mar 22, 2018

zz-jason commented Mar 22, 2018

zz-jason commented Mar 22, 2018

zz-jason commented Mar 22, 2018

zz-jason commented Mar 22, 2018

CLAassistant commented Mar 9, 2018 •

edited

Loading

lwhile commented Mar 9, 2018 •

edited

Loading

lwhile commented Mar 10, 2018 •

edited

Loading

lwhile commented Mar 20, 2018 •

edited

Loading

lwhile commented Mar 20, 2018 •

edited

Loading

zz-jason Mar 21, 2018 •

edited

Loading