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

max/min got wrong result #43805

Closed
AricSu opened this issue May 15, 2023 · 4 comments · Fixed by #43906
Closed

max/min got wrong result #43805

AricSu opened this issue May 15, 2023 · 4 comments · Fixed by #43906
Assignees
Labels
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 5.4.x versions. affects-6.1 affects-6.5 affects-7.1 report/community The community has encountered this bug. severity/critical sig/execution SIG execution type/bug The issue is confirmed as a bug.

Comments

@AricSu
Copy link
Contributor

AricSu commented May 15, 2023

Bug Report

Original question : https://asktug.com/t/topic/1005851

What's the reason or issue to get the wrong result? Thanks.
And if it's been fixed?

1. Minimal reproduce step (Required)

select
min(if(apply_time > 0 and stage_num > 1 and apply_to_now_days <= 30,loan,null)) as min,
max(if(apply_time > 0 and stage_num > 1 and apply_to_now_days <= 720,loan,null)) as max
from
(select
loan,stage_num,apply_time,
datediff(from_unixtime(unix_timestamp() + 18000), from_unixtime(apply_time/1000 + 18000)) as apply_to_now_days
from order where account_id = 210802010000721168 and id != 1) t1


CREATE TABLE orders (
id bigint(20) unsigned NOT NULL ,
account_id bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
product_id bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
product_id_org bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
loan bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
loan_org bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
float_rate int(11) NOT NULL DEFAULT ‘0’ ,
float_rate_org int(11) NOT NULL DEFAULT ‘0’ ,
period int(10) unsigned NOT NULL DEFAULT ‘0’ ,
period_org int(10) unsigned NOT NULL DEFAULT ‘0’ ,
pre_order bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
roll_times int(10) NOT NULL DEFAULT ‘0’ ,
min_repay_amount bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
op_uid bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
risk_ctl_status smallint(2) unsigned NOT NULL DEFAULT ‘0’ ,
risk_ctl_finish_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
risk_ctl_regular varchar(32) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ‘’ ,
reject_reason smallint(2) unsigned NOT NULL DEFAULT ‘0’ ,
is_temporary tinyint(1) unsigned NOT NULL DEFAULT ‘0’,
is_overdue tinyint(2) NOT NULL DEFAULT ‘0’ ,
is_dead_debt tinyint(1) unsigned NOT NULL DEFAULT ‘0’ ,
is_reloan tinyint(1) unsigned NOT NULL DEFAULT ‘0’ ,
is_up_hold_photo tinyint(1) NOT NULL DEFAULT ‘0’ ,
check_status tinyint(1) unsigned NOT NULL DEFAULT ‘1’ ,
apply_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
check_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
phone_verify_time bigint(20) unsigned NOT NULL DEFAULT ‘0’,
repay_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
loan_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
finish_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
penalty_utime bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
credit_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
random_value tinyint(3) unsigned NOT NULL DEFAULT ‘0’ ,
fixed_random int(10) NOT NULL DEFAULT ‘0’ ,
livingbest_reloanhand_similar varchar(30) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ‘’ ,
after_black_similar varchar(30) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ‘’ ,
random_mark int(10) NOT NULL DEFAULT ‘0’,
op_desc varchar(50) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ‘’ ,
from_platform tinyint(2) NOT NULL DEFAULT ‘0’ ,
stage_num int(20) unsigned NOT NULL DEFAULT ‘0’ ,
stage_info text COLLATE utf8mb4_unicode_ci DEFAULT NULL ,
third_paid_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
amount bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
overdue_run_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
risk_hyrule_status int(10) DEFAULT ‘0’ ,
order_invalid_type tinyint(2) unsigned NOT NULL DEFAULT ‘0’ ,
order_invalid_time bigint(20) unsigned NOT NULL DEFAULT ‘0’,
first_diff_tag int(11) unsigned NOT NULL DEFAULT ‘0’ ,
phone_verify_failed_time bigint(20) unsigned NOT NULL DEFAULT ‘0’,
phone_verify_failed_type tinyint(2) unsigned NOT NULL DEFAULT ,
confirm_enter_time bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
loan_flow_version tinyint(5) unsigned NOT NULL DEFAULT ‘0’ ,
confirm_disburse_time bigint(20) NOT NULL DEFAULT ‘0’ ,
offline_handle_time bigint(20) NOT NULL DEFAULT ‘0’ ,
is_jump_confirm smallint(2) NOT NULL DEFAULT ‘0’ ,
is_deleted tinyint(2) NOT NULL DEFAULT ‘0’ ,
orders_ctime bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
orders_utime bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
orders_ext_ctime bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
orders_ext_utime bigint(20) unsigned NOT NULL DEFAULT ‘0’ ,
PRIMARY KEY (id) /*T![clustered_index] CLUSTERED */,
KEY idx_orders_account_id (account_id),
KEY idx_orders_apply_time (apply_time),
KEY idx_orders_orders_ctime (orders_ctime),
KEY idx_orders_utime (orders_utime),
KEY idx_orders_ext_utime (orders_ext_utime)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci COMMENT=‘订单表’

image

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

±----------------------------±-----------------------------+
| min | max |
±----------------------------±-----------------------------+
| 20000 | 35100 |
±----------------------------±-----------------------------+

3. What did you see instead (Required)

±----------------------------±-----------------------------+
| min | max |
±----------------------------±-----------------------------+
| 13314398617609 | 13314398617609 |
±----------------------------±-----------------------------+

4. What is your TiDB version? (Required)

v5.4

@AricSu AricSu added the type/bug The issue is confirmed as a bug. label May 15, 2023
@ti-chi-bot ti-chi-bot bot added 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.1 may-affects-6.5 may-affects-7.1 labels May 16, 2023
@jebter jebter added the affects-5.4 This bug affects 5.4.x versions. label May 16, 2023
@ti-chi-bot ti-chi-bot bot removed the may-affects-5.4 This bug maybe affects 5.4.x versions. label May 16, 2023
@zanmato1984
Copy link
Contributor

zanmato1984 commented May 16, 2023

Somehow minimized the reproduction steps to following:

CREATE TABLE orders (
id bigint(20) unsigned NOT NULL ,
account_id bigint(20) unsigned NOT NULL DEFAULT '0' ,
loan bigint(20) unsigned NOT NULL DEFAULT '0' ,
stage_num int(20) unsigned NOT NULL DEFAULT '0' ,
apply_time bigint(20) unsigned NOT NULL DEFAULT '0' ,
PRIMARY KEY (id) /*T![clustered_index] CLUSTERED */,
KEY idx_orders_account_id (account_id),
KEY idx_orders_apply_time (apply_time)
);

insert into orders values (20, 210802010000721168, 20000 , 2 , 1682484268727), (22, 210802010000721168, 35100 , 4 , 1650885615002);

> select min(if(apply_to_now_days <= 30,loan,null)) as min, max(if(apply_to_now_days <= 720,loan,null)) as max from (select loan, datediff(from_unixtime(unix_timestamp() + 18000), from_unixtime(apply_time/1000 + 18000)) as apply_to_now_days from orders) t1;
+----------------+----------------+
| min            | max            |
+----------------+----------------+
| 85899345920009 | 85899345920009 |
+----------------+----------------+
1 row in set (0.01 sec)

@zanmato1984
Copy link
Contributor

zanmato1984 commented May 16, 2023

The plan for the wrong query is:

mysql> explain select min(if(apply_to_now_days <= 30,loan,null)) as min, max(if(apply_to_now_days <= 720,loan,null)) as max from (select loan, datediff(from_unixtime(unix_timestamp() + 18000), from_unixtime(apply_time/1000 + 18000)) as apply_to_now_days from orders) t1;
+----------------------------+---------+-----------+---------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| id                         | estRows | task      | access object | operator info                                                                                                                                                                                                                                                                                                                                                                                                                                   |
+----------------------------+---------+-----------+---------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| StreamAgg_9                | 1.00    | root      |               | funcs:min(Column#10)->Column#7, funcs:max(Column#11)->Column#8                                                                                                                                                                                                                                                                                                                                                                                  |
| └─Projection_19            | 2.00    | root      |               | if(le(datediff(2023-05-16 17:24:36, from_unixtime(plus(div(cast(test.orders.apply_time, decimal(20,0) UNSIGNED BINARY), 1000), 18000))), 30), cast(test.orders.loan, decimal(20,0) UNSIGNED BINARY), <nil>)->Column#10, if(le(datediff(2023-05-16 17:24:36, from_unixtime(plus(div(cast(test.orders.apply_time, decimal(20,0) UNSIGNED BINARY), 1000), 18000))), 720), cast(test.orders.loan, decimal(20,0) UNSIGNED BINARY), <nil>)->Column#11 |
|   └─TableReader_16         | 2.00    | root      |               | data:TableFullScan_15                                                                                                                                                                                                                                                                                                                                                                                                                           |
|     └─TableFullScan_15     | 2.00    | cop[tikv] | table:orders  | keep order:false, stats:pseudo                                                                                                                                                                                                                                                                                                                                                                                                                  |
+----------------------------+---------+-----------+---------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

meanwhile if we tune the query a bit, by inlining the apply_to_now_days column:

mysql> explain select min(if(datediff(from_unixtime(unix_timestamp() + 18000), from_unixtime(apply_time/1000 + 18000)) <= 30,loan,null)) as min, max(if(datediff(from_unixtime(unix_timestamp() + 18000), from_unixtime(apply_time/1000 + 18000)) <= 720,loan,null)) as max from orders;
+----------------------------+---------+-----------+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| id                         | estRows | task      | access object | operator info                                                                                                                                                                                                                                                                                                                                                       |
+----------------------------+---------+-----------+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| StreamAgg_8                | 1.00    | root      |               | funcs:min(Column#8)->Column#6, funcs:max(Column#9)->Column#7                                                                                                                                                                                                                                                                                                        |
| └─Projection_18            | 2.00    | root      |               | if(le(datediff(2023-05-16 17:26:13, from_unixtime(plus(div(cast(test.orders.apply_time, decimal(20,0) UNSIGNED BINARY), 1000), 18000))), 30), test.orders.loan, <nil>)->Column#8, if(le(datediff(2023-05-16 17:26:13, from_unixtime(plus(div(cast(test.orders.apply_time, decimal(20,0) UNSIGNED BINARY), 1000), 18000))), 720), test.orders.loan, <nil>)->Column#9 |
|   └─TableReader_15         | 2.00    | root      |               | data:TableFullScan_14                                                                                                                                                                                                                                                                                                                                               |
|     └─TableFullScan_14     | 2.00    | cop[tikv] | table:orders  | keep order:false, stats:pseudo                                                                                                                                                                                                                                                                                                                                      |
+----------------------------+---------+-----------+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

One notable thing is that there is an extra cast(... as decimal(20, 0)) applied to loan in the wrong plan - probably implying something wrong with this cast, e.g. casted to decimal but still being interpreted as an integer?

@wshwsh12
Copy link
Contributor

The problem occurs because the expression is restructured when trying to push the agg down(aggregationPushDownSolver optimizer) under the projection. This construct introduces cast decimal, and the return type of the expression is changed from int to decimal.
However, the corresponding agg function has not been updated, and the previous max4uint is still used, resulting in an error in the wrong result.

image

@wshwsh12 wshwsh12 added the affects-5.1 This bug affects 5.1.x versions. label May 17, 2023
@ti-chi-bot ti-chi-bot bot removed the may-affects-5.1 This bug maybe affects 5.1.x versions. label May 17, 2023
@wshwsh12 wshwsh12 added affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. may-affects-5.1 This bug maybe affects 5.1.x versions. labels May 17, 2023
@ti-chi-bot ti-chi-bot bot removed may-affects-5.2 This bug maybe affects 5.2.x versions. may-affects-5.3 This bug maybe affects 5.3.x versions. labels May 17, 2023
@wshwsh12 wshwsh12 added may-affects-5.2 This bug maybe affects 5.2.x versions. may-affects-5.3 This bug maybe affects 5.3.x versions. affects-6.1 affects-6.5 affects-7.1 labels May 17, 2023
@wshwsh12 wshwsh12 removed 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. labels May 17, 2023
@seiya-annie
Copy link

/found community

@ti-chi-bot ti-chi-bot bot added the report/community The community has encountered this bug. label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 5.4.x versions. affects-6.1 affects-6.5 affects-7.1 report/community The community has encountered this bug. severity/critical sig/execution SIG execution type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants