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

TiDB incorrectly converts update statement to point get #47445

Closed
Rustin170506 opened this issue Oct 8, 2023 · 8 comments · Fixed by #47454
Closed

TiDB incorrectly converts update statement to point get #47445

Rustin170506 opened this issue Oct 8, 2023 · 8 comments · Fixed by #47454
Labels

Comments

@Rustin170506
Copy link
Member

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

CREATE TABLE golang1 (
`fcbpdt` CHAR (8) COLLATE utf8_general_ci NOT NULL,
`fcbpsq` VARCHAR (20) COLLATE utf8_general_ci NOT NULL,
`procst` char (4) COLLATE utf8_general_ci DEFAULT NULL,
`cipstx` VARCHAR (105) COLLATE utf8_general_ci DEFAULT NULL,
`cipsst` CHAR (4) COLLATE utf8_general_ci DEFAULT NULL,
`dyngtg` VARCHAR(4) COLLATE utf8_general_ci DEFAULT NULL,
`blncdt` VARCHAR (8) COLLATE utf8_general_ci DEFAULT NULL, PRIMARY KEY ( fcbpdt, fcbpsq ));
insert into golang1 values('20230925','12023092502158016','abc','','','','');
create table golang2 (
`sysgrp` varchar(20) NOT NULL,
`procst` varchar(8) NOT NULL,
`levlid` int(11) NOT NULL,PRIMARY key (procst));
insert into golang2 VALUES('COMMON','ACSC',90);
insert into golang2 VALUES('COMMON','abc',8);
insert into golang2 VALUES('COMMON','CH02',6);
UPDATE golang1 a
SET procst =(
CASE WHEN
( SELECT levlid FROM golang2 b WHERE b.sysgrp = 'COMMON' AND b.procst = 'ACSC' )
>
( SELECT levlid FROM golang2 c WHERE c.sysgrp = 'COMMON' AND c.procst = a.procst )
THEN 'ACSC' ELSE a.procst END
) ,
cipstx = 'CI010000',
cipsst = 'ACSC',
dyngtg = 'EAYT',
blncdt= '20230925'
WHERE
fcbpdt = '20230925'
AND fcbpsq = '12023092502158016';

2. What did you expect to see? (Required)

Update success.

3. What did you see instead (Required)

Panic: ERROR 1105 (HY000): runtime error: index out of range [0] with length 0

4. What is your TiDB version? (Required)

mysql> select tidb_version();
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| tidb_version()                                                                                                                                                                                                                                                                               |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Release Version: v6.5.1
Edition: Community
Git Commit Hash: 4084b077d615f9dc0a41cf2e30bc6e1a02332df2
Git Branch: heads/refs/tags/v6.5.1
UTC Build Time: 2023-03-07 16:04:14
GoVersion: go1.19.5
Race Enabled: false
TiKV Min Version: 6.2.0-alpha
Check Table Before Drop: false
Store: tikv |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)
@Rustin170506 Rustin170506 added the type/bug The issue is confirmed as a bug. label Oct 8, 2023
@Rustin170506
Copy link
Member Author

It works on v6.3.0:

mysql> explain UPDATE golang1 a
    -> SET procst =(
    -> CASE WHEN
    -> ( SELECT levlid FROM golang2 b WHERE b.sysgrp = 'COMMON' AND b.procst = 'ACSC' )
    -> >
    -> ( SELECT levlid FROM golang2 c WHERE c.sysgrp = 'COMMON' AND c.procst = a.procst )
    -> THEN 'ACSC' ELSE a.procst END
    -> ) ,
    -> cipstx = 'CI010000',
    -> cipsst = 'ACSC',
    -> dyngtg = 'EAYT',
    -> blncdt= '20230925'
    -> WHERE
    -> fcbpdt = '20230925'
    -> AND fcbpsq = '12023092502158016';
+------------------------------------+---------+-----------+----------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------+
| id                                 | estRows | task      | access object                                | operator info                                                                                                                                                |
+------------------------------------+---------+-----------+----------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Update_17                          | N/A     | root      |                                              | N/A                                                                                                                                                          |
| └─IndexJoin_23                     | 1.25    | root      |                                              | left outer join, inner:IndexLookUp_22, outer key:test.golang1.procst, inner key:test.golang2.procst, equal cond:eq(test.golang1.procst, test.golang2.procst) |
|   ├─Point_Get_36(Build)            | 1.00    | root      | table:golang1, index:PRIMARY(fcbpdt, fcbpsq) |                                                                                                                                                              |
|   └─IndexLookUp_22(Probe)          | 1.00    | root      |                                              |                                                                                                                                                              |
|     ├─IndexRangeScan_19(Build)     | 1.00    | cop[tikv] | table:c, index:PRIMARY(procst)               | range: decided by [eq(test.golang2.procst, test.golang1.procst)], keep order:false, stats:pseudo                                                             |
|     └─Selection_21(Probe)          | 1.00    | cop[tikv] |                                              | eq(test.golang2.sysgrp, "COMMON")                                                                                                                            |
|       └─TableRowIDScan_20          | 1.00    | cop[tikv] | table:c                                      | keep order:false, stats:pseudo                                                                                                                               |
+------------------------------------+---------+-----------+----------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------+
7 rows in set (0.01 sec)

But on v6.5.1:

mysql> explain UPDATE golang1 a
    -> SET procst =(
    -> CASE WHEN
    -> ( SELECT levlid FROM golang2 b WHERE b.sysgrp = 'COMMON' AND b.procst = 'ACSC' )
    -> >
    -> ( SELECT levlid FROM golang2 c WHERE c.sysgrp = 'COMMON' AND c.procst = a.procst )
    -> THEN 'ACSC' ELSE a.procst END
    -> ) ,
    -> cipstx = 'CI010000',
    -> cipsst = 'ACSC',
    -> dyngtg = 'EAYT',
    -> blncdt= '20230925'
    -> WHERE
    -> fcbpdt = '20230925'
    -> AND fcbpsq = '12023092502158016';
+-------------------+---------+------+--------------------------------------------------------+---------------+
| id                | estRows | task | access object                                          | operator info |
+-------------------+---------+------+--------------------------------------------------------+---------------+
| Update_20         | N/A     | root |                                                        | N/A           |
| └─Point_Get_1     | 1.00    | root | table:golang1, clustered index:PRIMARY(fcbpdt, fcbpsq) |               |
+-------------------+---------+------+--------------------------------------------------------+---------------+
2 rows in set (0.00 sec)

@Rustin170506
Copy link
Member Author

Rustin170506 commented Oct 8, 2023

The main difference is the v6.5.1 TiDB converted the update statement to a point-get operation.

I first checked which commit introduced this bug, and I found after this commit c6e1982 the plan changed.

Then I dug into the code, and I found that we have different results when we call this code on different versions.

tblName, tblAlias := getSingleTableNameAndAlias(selStmt.From)

  1. We should always return nil, because we can not convert this SQL to a simple point get.
  2. For v6.3.0, we also incorrectly tried to convert it but because we didn't use the clustered index, so it just returned nil.
    We used the wrong schema to resolve indexes.
    newAssign.Expr, err = expr.ResolveIndices(plan.Schema())
image
  1. For v6.5.1, we incorrectly converted it to a point get.
    We also used the wrong schema to resolve indexes.
    newAssign.Expr, err = expr.ResolveIndices(plan.Schema())
image

So both v6.3.0 and v6.5.1 all have bugs. We should return from here:

tblName, tblAlias := getSingleTableNameAndAlias(selStmt.From)

Then we don't need to try to convert to a point get.

@ti-chi-bot ti-chi-bot bot added may-affects-5.3 This bug maybe affects 5.3.x versions. may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Oct 8, 2023
@Rustin170506 Rustin170506 removed may-affects-5.3 This bug maybe affects 5.3.x versions. may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Oct 8, 2023
@JasonWu0506
Copy link

/type regression

@jackysp jackysp added found/gs found by gs affects-5.4 This bug affects 5.4.x versions. labels Jan 23, 2024
@jackysp
Copy link
Member

jackysp commented Jan 23, 2024

It happened in v5.4.1, I think it may also affect release-6.1.

@Rustin170506
Copy link
Member Author

I tested it on v6.1. It worked on v6.1.

mysql> UPDATE golang1 a
    -> SET procst =(
    -> CASE WHEN
    -> ( SELECT levlid FROM golang2 b WHERE b.sysgrp = 'COMMON' AND b.procst = 'ACSC' )
    -> >
    -> ( SELECT levlid FROM golang2 c WHERE c.sysgrp = 'COMMON' AND c.procst = a.procst )
    -> THEN 'ACSC' ELSE a.procst END
    -> ) ,
    -> cipstx = 'CI010000',
    -> cipsst = 'ACSC',
    -> dyngtg = 'EAYT',
    -> blncdt= '20230925'
    -> WHERE
    -> fcbpdt = '20230925'
    -> AND fcbpsq = '12023092502158016';
Query OK, 1 row affected (0.01 sec)
Rows matched: 1  Changed: 1  Warnings: 0

I think the bug was only triggered after we enabled the index of the cluster. To fully resolve this issue, we need to cherry-pick it. Because in version 6.1, I believe other reasons have obscured this problem.

@kennedy8312
Copy link

Regression Analysis
PR caused this regression: #38447

@Rustin170506
Copy link
Member Author

Regression Analysis PR caused this regression: #38447

I don't believe that is the root cause. The problem has existed for a long time. This only triggered the bug but did not introduce it.

@kennedy8312
Copy link

Regression Analysis PR caused this regression: #38447

I don't believe that is the root cause. The problem has existed for a long time. This only triggered the bug but did not introduce it.

I agree with you for the long existing root cause and the configuration change exposed the issue. However, the change broke an existing function per user point of view, which meant a functional regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants