executor: fix point get -1 return max.uInt64 value #10113
Conversation
|
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #10113 +/- ##
================================================
+ Coverage 77.3166% 77.3358% +0.0192%
================================================
Files 412 412
Lines 86623 86520 -103
================================================
- Hits 66974 66911 -63
+ Misses 14501 14477 -24
+ Partials 5148 5132 -16 |
|
/run-all-tests |
2 similar comments
|
/run-all-tests |
|
/run-all-tests |
| @@ -44,6 +44,7 @@ func (b *executorBuilder) buildPointGet(p *plannercore.PointGetPlan) Executor { | |||
| idxVals: p.IndexValues, | |||
| handle: p.Handle, | |||
| startTS: startTS, | |||
| done: p.UnsignedHandle && p.Handle < 0, | |||
There was a problem hiding this comment.
this make PointGetExecutor's Next return no result, we cannot use tabledual, this just like tabedual, and no need add more flag- -
There was a problem hiding this comment.
Other findings are:
- if condition is like
col = -1, we cannot use point get now, becausegetNameValuePairsdoes not consider minus function;
mysql> create table t1(a bigint primary key, b int);
Query OK, 0 rows affected (0.01 sec)
mysql> explain select * from t1 where a = -1;
+-------------------+-------+------+---------------------------------------------------------+
| id | count | task | operator info |
+-------------------+-------+------+---------------------------------------------------------+
| TableReader_6 | 1.00 | root | data:TableScan_5 |
| └─TableScan_5 | 1.00 | cop | table:t1, range:[-1,-1], keep order:false, stats:pseudo |
+-------------------+-------+------+---------------------------------------------------------+
- if handle column is unsigned type, we cannot use point get when filter value exceeds upper bound of int64:
mysql> create table t(a bigint unsigned primary key, b int);
Query OK, 0 rows affected (0.02 sec)
mysql> explain select * from t where a = 9223372036854775807;
+-------------+-------+------+-------------------------------------+
| id | count | task | operator info |
+-------------+-------+------+-------------------------------------+
| Point_Get_1 | 1.00 | root | table:t, handle:9223372036854775807 |
+-------------+-------+------+-------------------------------------+
1 row in set (0.00 sec)
mysql> explain select * from t where a = 9223372036854775808;
+-------------------+-------+------+------------------------------------------------------------------------------------------+
| id | count | task | operator info |
+-------------------+-------+------+------------------------------------------------------------------------------------------+
| TableReader_6 | 1.00 | root | data:TableScan_5 |
| └─TableScan_5 | 1.00 | cop | table:t, range:[9223372036854775808,9223372036854775808], keep order:false, stats:pseudo |
+-------------------+-------+------+------------------------------------------------------------------------------------------+
2 rows in set (0.00 sec)
so I am wondering about the current behavior of the following cases when prepare plan cache is enabled:
- handle column is unsigned, if the first execute is
pk = 1, and the second execute is likepk = 18446744073709551615, since we are using int64 as handle type inPointGet, would the second execute cause overflow and wrong result? - same as the first case, but handle column is int64, would the second execute report error?
| tk.MustExec("set @p2=1") | ||
| tk.MustExec(`prepare stmt7 from "select a from t where a = ?"`) | ||
| tk.MustQuery("execute stmt7 using @p1").Check(testkit.Rows()) | ||
| tk.MustQuery("execute stmt7 using @p2").Check(testkit.Rows("1")) |
There was a problem hiding this comment.
This test case cannot test PointGet plan actually? the plan generated using @p1 is table_scan + selection, not point get, so @p2 actually reuses this table_scan + selection plan?
There was a problem hiding this comment.
@eurekaka oh...... this logic is magic...
This test case cannot test PointGet plan actually? the plan generated using @p1 is table_scan + selection, not point get, so @p2 actually reuses this table_scan + selection plan?
In my test this can use point-get in two executes, because prepare use ? instead of -1 or 1, so getNameValuePair can handle ParramMarkExpr, then prepare success a point-get and execute as point-get.
mysql> show create table t1;
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table |
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------------+
| t1 | CREATE TABLE `t1` (
`a` bigint(20) NOT NULL,
`b` int(11) DEFAULT NULL,
PRIMARY KEY (`a`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin |
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)
mysql> explain select * from t1 where a = -1; +-------------------+-------+------+---------------------------------------------------------+
| id | count | task | operator info |
+-------------------+-------+------+---------------------------------------------------------+
| TableReader_6 | 1.00 | root | data:TableScan_5 |
| └─TableScan_5 | 1.00 | cop | table:t1, range:[-1,-1], keep order:false, stats:pseudo |
+-------------------+-------+------+---------------------------------------------------------+
2 rows in set (0.01 sec)
mysql> prepare st from "explain select * from t1 where a = ?";
Query OK, 0 rows affected (0.00 sec)
mysql> set @x=-1;
Query OK, 0 rows affected (0.00 sec)
mysql> execute st using @x;
+-------------+-------+------+---------------------+
| id | count | task | operator info |
+-------------+-------+------+---------------------+
| Point_Get_1 | 1.00 | root | table:t1, handle:-1 |
+-------------+-------+------+---------------------+
1 row in set (0.00 sec)
same as the first case, but handle column is int64, would the second execute report error?
so for this case:
same as the first case, but handle column is int64, would the second execute report error?
maybe luck is just fine
handle column is unsigned, if the first execute is pk = 1, and the second execute is like pk = 18446744073709551615, since we are using int64 as handle type in PointGet, would the second execute cause overflow and wrong result?
will ok for both use point-get plan, but there are another question, we keep param as string datum and try to cast it to int64, so will got a overflow question, it seems not easy to fix so it take another issue in here #10111.
🤣 it's very stranger -1 + int64 cannot use plan cache, and prepare and normal query use different plan..
There was a problem hiding this comment.
I think the root cause of these problems is that we are using int64 to store the handle in PointGet, while it can be unsigned.
|
Can we add a bool flag called |
|
Please resolve the conflicts, @lysu . |
|
@XuHuaiyu we have UnsignedHandle to mark it in pointGetPlann :D |
|
@lysu Should this commit be cherry-picked to release-2.1? |
|
It seems that, not for sure, we failed to cherry-pick this commit to release-2.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @lysu PTAL. |
What problem does this PR solve?
fixes #10056.
What is changed and how it works?
this question only occurred in prepare + point-select, in normal query optimize will collapse condition a = -100 to none if a is unsigned and got a TableDual in this case, but point-get some different code path, and plan cache also need be take care(we can not cache a TableDual and want it be a real table in later input).
so here, we keep unsigned info in PointGetPlan and generate a "done=true" PointGetExecutor if give a negative parameter.
Check List
Tests
Code changes
Side effects
Related changes
relate question #10111 but not easy to fix that and plan to fix them when we solve the string-cast question in plan-cache
This change is