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: Add the check for expression evaluation in some executors (#16339) #16383

Merged
merged 7 commits into from
May 9, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented Apr 15, 2020

cherry-pick #16339 to release-4.0


What problem does this PR solve?

Issue Number: close #16337

Problem Summary:
Lack of the check when we use expression evaluation in some executors.

What is changed and how it works?

Add the check to the EvalExpr to choose whether it uses the VecExpr or Expr.

Related changes

Rename VecEval to EvalExpr.
Add the check to the EvalExpr to choose whether it uses the VecExpr or Expr.

Check List

Tests

  • Unit test
  • Integration test

Release note

  • expression: Add the check for expression evaluation in some executors

@sre-bot sre-bot requested a review from a team as a code owner April 15, 2020 02:52
@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 15, 2020

/run-all-tests

@qw4990
Copy link
Contributor

qw4990 commented Apr 15, 2020

/run-sqllogic-test-1

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

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 15, 2020
@Reminiscent
Copy link
Contributor

@qw4990 PTAL

Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

@qw4990 qw4990 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 23, 2020
@zz-jason zz-jason added the status/can-merge Indicates a PR has been approved by a committer. label Apr 23, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 23, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 23, 2020

@sre-bot merge failed.

@Reminiscent Reminiscent changed the title Add the check for expression evaluation in some executors (#16339) expression: Add the check for expression evaluation in some executors (#16339) Apr 23, 2020
@Reminiscent
Copy link
Contributor

Wait for #16156 to fix the fail of the unit test.

@Reminiscent
Copy link
Contributor

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 23, 2020

Sorry @Reminiscent, you don't have permission to trigger auto merge event on this branch.

@zz-jason
Copy link
Member

/run-all-tests

@zz-jason
Copy link
Member

/build

@zz-jason
Copy link
Member

/run-all-tests

1 similar comment
@Reminiscent
Copy link
Contributor

/run-all-tests

@Reminiscent
Copy link
Contributor

/run-integration-common-test

2 similar comments
@qw4990
Copy link
Contributor

qw4990 commented Apr 26, 2020

/run-integration-common-test

@Reminiscent
Copy link
Contributor

/run-integration-common-test

@Reminiscent
Copy link
Contributor

/run-all-tests

@ngaut ngaut added status/can-merge Indicates a PR has been approved by a committer. and removed status/can-merge Indicates a PR has been approved by a committer. labels Apr 30, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 30, 2020

Your auto merge job has been accepted, waiting for:

  • 16830

@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 30, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 30, 2020

@sre-bot merge failed.

@Reminiscent
Copy link
Contributor

@zz-jason PTLA. Please help merge this PR. Thanks!

@zhouqiang-cl
Copy link
Contributor

/rebuild

@Reminiscent
Copy link
Contributor

/rebuild

@Reminiscent
Copy link
Contributor

/run-all-tests

@Reminiscent
Copy link
Contributor

Reminiscent commented May 7, 2020

The failed test can be fixed by #16997

@Reminiscent
Copy link
Contributor

/run-integration-copr-test

@ngaut ngaut merged commit 6ce1067 into pingcap:release-4.0 May 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants