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 #41736

Closed
sayJason opened this issue Feb 25, 2023 · 5 comments · Fixed by #41791 or #53590
Closed

An incorrect query result #41736

sayJason opened this issue Feb 25, 2023 · 5 comments · Fixed by #41791 or #53590
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 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. severity/critical sig/execution SIG execution 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)

CREATE TABLE t0(c0 tinyint(1) unsigned not null );
INSERT INTO t0 VALUES (1);
SELECT * FROM t0 WHERE CASE 0 WHEN (t0.c0)>(-1.194192591E9) THEN null ELSE 1 END; -- actual: {}, expected: {1}

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

The query result should be {1}.

3. What did you see instead (Required)

The query result is an empty set.

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
@ChenPeng2013 ChenPeng2013 added sig/planner SIG: Planner severity/critical 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 labels Feb 27, 2023
@ti-chi-bot ti-chi-bot added the may-affects-4.0 This bug maybe affects 4.0.x versions. label Feb 27, 2023
@chrysan
Copy link
Contributor

chrysan commented Feb 27, 2023

It's due to(t0.c0)>(-1.194192591E9) is evaluated to 0.
Simplified reproduce sql:

mysql> SELECT (t0.c0)>(-1.194192591E9) from t0;
+--------------------------+
| (t0.c0)>(-1.194192591E9) |
+--------------------------+
|                        0 |
+--------------------------+

7PVVBzNbRw

@jebter jebter added sig/execution SIG execution and removed sig/planner SIG: Planner labels Feb 28, 2023
@LittleFall LittleFall self-assigned this Feb 28, 2023
@LittleFall
Copy link
Contributor

LittleFall commented Feb 28, 2023

reproduce for all int type

drop table t0;

CREATE TABLE t0(
    ti tinyint unsigned not null,
    si smallint unsigned not null,
    mi mediumint unsigned not null,
    ii int unsigned not null,
    bi bigint unsigned not null
);
INSERT INTO t0 VALUES (1, 1, 1, 1, 1);

select ti, ti>-99.999 from t0; # 1
select ti, ti>-100.0 from t0; # 0

select ti>-99,    si>-9999,    mi>-9999999,    ii>-999999999,    bi>-9999999999999999999 from t0;    # 1 1 1 1 1
select ti>-99.0,  si>-9999.0,  mi>-9999999.0,  ii>-999999999.0,  bi>-9999999999999999999.0 from t0;  # 1 1 1 1 1
select ti>-100.0, si>-10000.0, mi>-10000000.0, ii>-1000000000.0, bi>-10000000000000000000.0 from t0; # 0 0 0 0 0
select ti>-100,   si>-10000,   mi>-10000000,   ii>-1000000000,   bi>-10000000000000000000 from t0;   # 1 1 1 1 0

@LittleFall LittleFall added affects-4.0 This bug affects 4.0.x versions. and removed may-affects-4.0 This bug maybe affects 4.0.x versions. labels Feb 28, 2023
@LittleFall
Copy link
Contributor

LittleFall commented Mar 1, 2023

Analysis

Callstack:

func (c *compareFunctionClass) refineArgs(ctx sessionctx.Context, args []Expression) []Expression {
                arg1, isExceptional = RefineComparedConstant(ctx, *arg0Type, arg1, c.op)
                ...
		isExceptional = isExceptional && mysql.HasNotNullFlag(arg0Type.GetFlag())
		if isExceptional && arg1.GetType().EvalType() == types.ETInt {
			// Judge it is inf or -inf
			// For int:
			//			inf:  01111111 & 1 == 1
			//		   -inf:  10000000 & 1 == 0
			// For uint:
			//			inf:  11111111 & 1 == 1
			//		   -inf:  00000000 & 1 == 0
			if arg1.Value.GetInt64()&1 == 1 {
				isPositiveInfinite = true
			} else {
				isNegativeInfinite = true
			}
		}

//...	If the op == LT,LE,GT,GE and it gets an Overflow when converting, return inf/-inf.
func RefineComparedConstant(ctx sessionctx.Context, targetFieldType types.FieldType, con *Constant, op opcode.Op) (_ *Constant, isExceptional bool) {
        intDatum, err = dt.ConvertTo(sc, &targetFieldType)
	if err != nil {
		if terror.ErrorEqual(err, types.ErrOverflow) {
			return &Constant{
				Value:        intDatum,
				RetType:      &targetFieldType,
				DeferredExpr: con.DeferredExpr,
				ParamMarker:  con.ParamMarker,
			}, true
		}
		return con, false
	}

func (d *Datum) ConvertTo(sc *stmtctx.StatementContext, target *FieldType) (Datum, error) {
func (d *Datum) convertToUint(sc *stmtctx.StatementContext, target *FieldType) (Datum, error) {
func ConvertDecimalToUint(sc *stmtctx.StatementContext, d *MyDecimal, upperBound uint64, tp byte) (uint64, error) {
func convertDecimalStrToUint(sc *stmtctx.StatementContext, str string, upperBound uint64, tp byte) (uint64, error) {
        ......
	if sc.ShouldClipToZero() && intStr[0] == '-' {
		return 0, overflow(str, tp)
	}
        ......
	upperStr := strconv.FormatUint(upperBound, 10)
	if len(intStr) > len(upperStr) ||
		(len(intStr) == len(upperStr) && intStr > upperStr) {
		return upperBound, overflow(str, tp)
	}
	val, err := strconv.ParseUint(intStr, 10, 64)
	if err != nil {
		return val, overflow(str, tp)
	}

func (sc *StatementContext) ShouldClipToZero() bool {
	return sc.InInsertStmt || sc.InLoadDataStmt || sc.InUpdateStmt || sc.InCreateOrAlterStmt || sc.IsDDLJobInQueue
}

Root Cause

  1. compareFunctionClass will convert the constant (e.g., -100) to the column type (e.g. unsigned tinyint), when it gets an overflow error from dt.convertTo call, it thinks the returned value has already been set to inf with the correct direction (e.g. 0 or 255, means NegativeInfinite or PositiveInfinite).
  2. But convertDecimalStrToUint doesn't care about its return value. When converting -99 to uint8, it returns (0, overflow). When converting -100 to uint8, it returns (255, overflow), because the length of "-100" is greater than the length of "255".

Fix

When convert to uint and the first character is '-', just return (0, overflow).

@guo-shaoge
Copy link
Collaborator

This issue still exsits. We expect result is 1 instead of empty.
a7eca747-580d-463d-8337-fcf6fcbcb73e

@guo-shaoge guo-shaoge reopened this May 21, 2024
@guo-shaoge
Copy link
Collaborator

call stacks:

(c *compareFunctionClass) getFunction() ->
(c *compareFunctionClass) refineArgs
RefineComparedConstant(): 
    1. this function returns wrong converted value(convert double type of -1.194192591E9 as a very big uint64).
    2. Then `refineArgs` treat this as `isPositiveInfinite`, which will return zero and one as refined args.

@ti-chi-bot ti-chi-bot added affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. labels May 24, 2024
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 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. severity/critical sig/execution SIG execution type/bug The issue is confirmed as a bug.
Projects
None yet
7 participants