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

executor: init builder startTS in buildTableDual #6830

Merged
merged 2 commits into from
Jun 13, 2018

Conversation

coocood
Copy link
Member

@coocood coocood commented Jun 13, 2018

Fixes #6828

What have you changed? (mandatory)

When we build an index join executor with outer table is table dual, we never call b.getStartTS during build time.
But later when we execute the query, we use the builder to build inner table which needs startTS, but by then, the transaction is set to invalid, then we panic.

This PR init the startTS in executorBuilder.buildTableDual to fix the panic issue.

What are the type of the changes (mandatory)?

  • Bug fix (non-breaking change which fixes an issue)

How has this PR been tested (mandatory)?

  • unit-test

Fixes panic when table dual is index join outer table.
@coocood coocood added the type/bugfix This PR fixes a bug. label Jun 13, 2018
@shenli
Copy link
Member

shenli commented Jun 13, 2018

Is there a related issue?

@coocood
Copy link
Member Author

coocood commented Jun 13, 2018

/run-all-tests

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

zz-jason
zz-jason previously approved these changes Jun 13, 2018
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@shenli shenli dismissed zz-jason’s stale review June 13, 2018 10:26

CI failed.

@coocood coocood changed the title executor: init startTS in newExecuorBuilder executor: init startTS in buildTableDual Jun 13, 2018
@coocood coocood changed the title executor: init startTS in buildTableDual executor: init builder startTS in buildTableDual Jun 13, 2018
@coocood
Copy link
Member Author

coocood commented Jun 13, 2018

/run-all-tests

Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

LGTM

@coocood coocood merged commit e2c8c4e into pingcap:master Jun 13, 2018
@coocood coocood deleted the builder-start-ts branch June 13, 2018 12:04
@shenli
Copy link
Member

shenli commented Jun 13, 2018

@coocood We need to cherry-pick this to the release-2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error Code: 1105. runtime error: invalid memory address or nil pointer dereference
6 participants