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

An incorrect query result. #41734

Closed
sayJason opened this issue Feb 25, 2023 · 1 comment · Fixed by #41823
Closed

An incorrect query result. #41734

sayJason opened this issue Feb 25, 2023 · 1 comment · Fixed by #41823
Assignees
Labels
affects-4.0 This bug affects 4.0.x versions. affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 severity/major sig/planner SIG: Planner type/bug The issue is confirmed as a bug.

Comments

@sayJason
Copy link

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

Case 1:

CREATE TABLE t1(c0 tinyint(1));
INSERT INTO t1 VALUES (0);
SELECT c0 FROM t1 WHERE (NOT (CAST(IFNULL(t1.c0, HEX(((t1.c0)>>(t1.c0)))) AS DATE))); -- {}

Case 2:

CREATE TABLE t1(c0 tinyint(1) not null );
INSERT INTO t1 VALUES (0);
SELECT c0 FROM t1 WHERE (NOT (CAST(IFNULL(t1.c0, HEX(((t1.c0)>>(t1.c0)))) AS DATE))); -- {0}

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

Case 1 and case 2 should return the same result.

3. What did you see instead (Required)

Case 1 and case 2 return different results.

4. What is your TiDB version? (Required)

Release Version: v6.6.0
Edition: Community
Git Commit Hash: f4ca082
Git Branch: heads/refs/tags/v6.6.0
UTC Build Time: 2023-02-17 14:49:02
GoVersion: go1.19.5
Race Enabled: false
TiKV Min Version: 6.2.0-alpha
Check Table Before Drop: false
Store: tikv

@sayJason sayJason added the type/bug The issue is confirmed as a bug. label Feb 25, 2023
@ti-chi-bot ti-chi-bot added may-affects-4.0 This bug maybe affects 4.0.x versions. may-affects-5.0 This bug maybe affects 5.0.x versions. may-affects-5.1 This bug maybe affects 5.1.x versions. may-affects-5.2 This bug maybe affects 5.2.x versions. 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.0 may-affects-6.1 may-affects-6.2 may-affects-6.3 may-affects-6.4 may-affects-6.5 may-affects-6.6 labels Feb 27, 2023
@LittleFall LittleFall self-assigned this Mar 1, 2023
@LittleFall
Copy link
Contributor

LittleFall commented Mar 1, 2023

Analysis

Simple Reproduce

ifnull(nullable tinyint, string) produces a string return type although the value is not null. And then cast(string as date) produces null.

In the meanwhile, ifnull(notnullable tinyint, string) produces an int return type. And cast(0 as date) produces 0000-00-00.

CREATE TABLE t1(cnotnull tinyint not null, cnull tinyint null);
INSERT INTO t1 VALUES (0, 0);

mysql> select IFNULL(cnull, '1'), IFNULL(cnotnull, '1') from t1;
Field   1:  `IFNULL(cnull, '1')`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       VAR_STRING
Collation:  utf8mb4_bin (46)
Length:     16
Max_length: 1
Decimals:   31
Flags:      NOT_NULL BINARY

Field   2:  `IFNULL(cnotnull, '1')`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       TINY
Collation:  binary (63)
Length:     4
Max_length: 1
Decimals:   0
Flags:      NOT_NULL NO_DEFAULT_VALUE NUM


+--------------------+-----------------------+
| IFNULL(cnull, '1') | IFNULL(cnotnull, '1') |
+--------------------+-----------------------+
| 0                  |                     0 |
+--------------------+-----------------------+
1 row in set (0.01 sec)

select cast('0' as date), cast(0 as date);
+-------------------+-----------------+
| cast('0' as date) | cast(0 as date) |
+-------------------+-----------------+
| NULL              | 0000-00-00      |
+-------------------+-----------------+

Root Cause

  1. ifnull's result type should be the merge type of the two arguments.
  2. When tidb finds the first argument can't be null, it will directly eliminate the ifnull. Which makes the return type the first argument's.
  3. for mysql, it still keep the return type. (see below picture)
explain select IFNULL(cnull, now()), IFNULL(cnotnull, now()) from t1;
+-------------------------+---------+-----------+---------------+----------------------------------------------------------------------------------------------+
| id                      | estRows | task      | access object | operator info                                                                                |
+-------------------------+---------+-----------+---------------+----------------------------------------------------------------------------------------------+
| Projection_3            | 2.00    | root      |               | ifnull(cast(test.t1.cnull, var_string(20)), 2023-03-01 15:26:50)->Column#4, test.t1.cnotnull |
| └─TableReader_5         | 2.00    | root      |               | data:TableFullScan_4                                                                         |
|   └─TableFullScan_4     | 2.00    | cop[tikv] | table:t1      | keep order:false, stats:pseudo                                                               |
+-------------------------+---------+-----------+---------------+----------------------------------------------------------------------------------------------+
3 rows in set (0.00 sec)

image

Fix

When we find the first argument is not null, we should optimize it to a cast instead of eliminate it.

@LittleFall LittleFall removed may-affects-4.0 This bug maybe affects 4.0.x versions. may-affects-5.1 This bug maybe affects 5.1.x versions. may-affects-5.2 This bug maybe affects 5.2.x versions. 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-5.0 This bug maybe affects 5.0.x versions. may-affects-6.0 may-affects-6.1 may-affects-6.2 may-affects-6.3 may-affects-6.4 labels Mar 1, 2023
@LittleFall LittleFall added affects-4.0 This bug affects 4.0.x versions. affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 and removed may-affects-6.5 may-affects-6.6 labels Mar 1, 2023
ti-chi-bot pushed a commit that referenced this issue Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.0 This bug affects 4.0.x versions. affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 severity/major sig/planner SIG: Planner type/bug The issue is confirmed as a bug.
Projects
None yet
4 participants