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

sql bind will not be hit if the sql is similar as select * from x limit 0, 1 #13871

Closed
SunRunAway opened this issue Dec 3, 2019 · 4 comments · Fixed by #13889
Closed

sql bind will not be hit if the sql is similar as select * from x limit 0, 1 #13871

SunRunAway opened this issue Dec 3, 2019 · 4 comments · Fixed by #13889
Labels
sig/planner SIG: Planner type/bug The issue is confirmed as a bug.

Comments

@SunRunAway
Copy link
Contributor

Bug Report

Please answer these questions before submitting your issue. Thanks!

  1. What did you do?
    If possible, provide a recipe for reproducing the error.
create table t(a int, b int, INDEX ia (a), INDEX ib (b));
insert into t value(1, 1);

create global binding for select a, b, sleep(3) from t where a = 1 limit 0, 1 using select a, b, sleep(3) from t use index (ib) where a = 1 limit 0, 1;

Then run,

select a, b, sleep(3) from t where a = 1 limit 0, 1;
  1. What did you expect to see?
    During the sleeping, show the execution plan in another client, but it still uses index ia
mysql> show processlist;
+------+------+-----------+------+---------+------+-------+-----------------------------------------------------+
| Id   | User | Host      | db   | Command | Time | State | Info                                                |
+------+------+-----------+------+---------+------+-------+-----------------------------------------------------+
|    1 | root | 127.0.0.1 | test | Query   |    0 | 2     | select a, b, sleep(3) from t where a = 1 limit 0, 1 |
|    2 | root | 127.0.0.1 | test | Query   |    0 | 2     | show processlist                                    |
+------+------+-----------+------+---------+------+-------+-----------------------------------------------------+
2 rows in set (0.00 sec)

mysql> explain for connection 1;
+--------------------------+-------+-----------+---------------------------------------------------------------+
| id                       | count | task      | operator info                                                 |
+--------------------------+-------+-----------+---------------------------------------------------------------+
| Projection_7             | 1.00  | root      | Column#1, Column#2, sleep(3)                                  |
| └─IndexLookUp_18         | 1.00  | root      | limit embedded(offset:0, count:1)                             |
|   ├─Limit_17             | 1.00  | cop[tikv] | offset:0, count:1                                             |
|   │ └─IndexScan_15       | 1.00  | cop[tikv] | table:t, index:a, range:[1,1], keep order:false, stats:pseudo |
|   └─TableScan_16         | 1.00  | cop[tikv] | table:t, keep order:false, stats:pseudo                       |
+--------------------------+-------+-----------+---------------------------------------------------------------+
5 rows in set (0.00 sec)
  1. What did you see instead?

  2. What version of TiDB are you using (tidb-server -V or run select tidb_version(); on TiDB)?

@SunRunAway SunRunAway added type/bug The issue is confirmed as a bug. sig/planner SIG: Planner labels Dec 3, 2019
@SunRunAway
Copy link
Contributor Author

SunRunAway commented Dec 4, 2019

Why the problem occurs:
parser.DigestHash do an extra and inexplicit normalize, see https://github.com/pingcap/parser/blob/55f02bc42e92e6e00115cd0ceb89d1a30552e95a/digester.go#L76

  1. when binding: using parser.DigestHash, normalized twice and calculate a hash.
  2. when run explain analyze select * from t limit 0, 1: using parser.DigestHash, normalized twice and calculate the hash.
  3. when run select * from t limit 0, 1: using parser.NormalizeDigest, normalized once and calculate the hash.

So the hash of case 3 can not be matched with case 1.

@SunRunAway
Copy link
Contributor Author

SunRunAway commented Dec 4, 2019

parser.Normalize is not idempotent, and parser.DigestHash do an inexplicit parser.Normalize.

package main

import "fmt"
import "github.com/pingcap/parser"

func main() {
	sql := "select a from t limit 100,10"
	normalized := parser.Normalize(sql)
	digest := parser.DigestHash(normalized)
	fmt.Println(normalized, digest)

	normalized, digest = parser.NormalizeDigest(sql)
	fmt.Println(normalized, digest)
}
select a from t limit ... ef9154a8cb0464d9937f913ae571a38dc948923bdcb3034153c8b2f28256c1ed
select a from t limit ... fcbb6647dc9d4dd307f6dacad43dd437a57e6c69d739d5aa1670513c4a51c148

@lysu
Copy link
Contributor

lysu commented Dec 4, 2019

parser.DigestHash do an extra and inexplicit normalize, see https://github.com/pingcap/parser/blob/55f02bc42e92e6e00115cd0ceb89d1a30552e95a/digester.go#L76

I don't think this normalize is extra, "digest"'s definition is fingerprint of two SQL, it's not just MD5 of two unmoralized SQL? does the root cause should be "why we need pass a normalized sql into digest" - -?

but for backward compatibility, it seems we need add a new method.

@SunRunAway
Copy link
Contributor Author

SunRunAway commented Dec 4, 2019

parser.DigestHash do an extra and inexplicit normalize, see https://github.com/pingcap/parser/blob/55f02bc42e92e6e00115cd0ceb89d1a30552e95a/digester.go#L76

I don't think this normalize is extra, "digest"'s definition is fingerprint of two SQL, it's not just MD5 of two unmoralized SQL? does the root cause should be "why we need pass a normalized sql into digest" - -?

but for backward compatibility, it seems we need add a new method.

Yes, agree with you.
DigestHash has a naming problem to let people use it improperly, maybe it's better to be named as NormalizeAndDigestHash similar as parser.NormalizeDigest.
Also there's a comment with NormalizeDigest, which lead us to assume that DigestHash is just doing digesting but not normalizing.

NormalizeDigest combines Normalize and DigestHash into one method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants