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

planner: add warning when join hint has no arguments #15583

Merged
merged 7 commits into from
Mar 30, 2020

Conversation

francis0407
Copy link
Member

What problem does this PR solve?

Problem Summary:

This PRs adds warning when join hint has no arguments.
warning message:

[planner:1815]Hint HASH_JOIN() is inapplicable. Please specify the table names in the arguments.

What is changed and how it works?

How it Works:
check whether the hint.Tables is empty.

Related changes

  • Need to cherry-pick to the release branch 3.1

Check List

Tests

  • Unit test

Release note

adds warning message when join hint has no arguments.

@francis0407 francis0407 requested a review from a team as a code owner March 23, 2020 09:36
@ghost ghost requested review from eurekaka and lzmhhh123 and removed request for a team March 23, 2020 09:36
@codecov
Copy link

codecov bot commented Mar 23, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@6eda698). Click here to learn what that means.
The diff coverage is 93.9393%.

@@             Coverage Diff             @@
##             master     #15583   +/-   ##
===========================================
  Coverage          ?   80.4083%           
===========================================
  Files             ?        504           
  Lines             ?     134649           
  Branches          ?          0           
===========================================
  Hits              ?     108269           
  Misses            ?      17895           
  Partials          ?       8485

@@ -2195,23 +2205,45 @@ func (b *PlanBuilder) pushTableHints(hints []*ast.TableOptimizerHint, nodeType n
for _, hint := range hints {
switch hint.HintName.L {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about check hint.Tables before the switch. Then we can reduce much redundant code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I have merged these codes. PTAL

…nt_warning

# Conflicts:
#	planner/core/logical_plan_builder.go
#	planner/core/testdata/integration_suite_in.json
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

LGTM

@lzmhhh123 lzmhhh123 added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 30, 2020
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 status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 30, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

Your auto merge job has been accepted, waiting for 15678, 15773, 15552

@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

/run-all-tests

@sre-bot sre-bot merged commit 5067639 into pingcap:master Mar 30, 2020
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 30, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

cherry pick to release-3.1 in PR #15874

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 30, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

cherry pick to release-4.0 in PR #15875

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants