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

Incompatible query results by CAST('-787360724' AS TIME) #45410

Closed
sayJason opened this issue Jul 18, 2023 · 5 comments · Fixed by #46620
Closed

Incompatible query results by CAST('-787360724' AS TIME) #45410

sayJason opened this issue Jul 18, 2023 · 5 comments · Fixed by #46620
Assignees
Labels
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 severity/major 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)

DROP TABLE IF EXISTS t1;
CREATE TABLE t1 (c1 TINYINT(1) UNSIGNED NOT NULL );
INSERT INTO t1 VALUES (0);
SELECT c1>=CAST('-787360724' AS TIME) FROM t1; -- actual: {0}, expected: {1}

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

SELECT returns {1}

3. What did you see instead (Required)

SELECT returns {0}

4. What is your TiDB version? (Required)

Release Version: v7.2.0
Edition: Community
Git Commit Hash: 9fd5f4a
Git Branch: heads/refs/tags/v7.2.0
UTC Build Time: 2023-06-27 15:04:42
GoVersion: go1.20.5
Race Enabled: false
Check Table Before Drop: false
Store: tikv

@sayJason sayJason added the type/bug The issue is confirmed as a bug. label Jul 18, 2023
@ti-chi-bot ti-chi-bot bot 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. 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 Jul 18, 2023
@aytrack aytrack added 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 and 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. 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 Jul 18, 2023
@hawkingrei
Copy link
Member

@sayJason Hello! I noticed that you have recently discovered many bugs in TiDB. Have you developed a fuzzing tool to find these issues?

@sayJason
Copy link
Author

@sayJason Hello! I noticed that you have recently discovered many bugs in TiDB. Have you developed a fuzzing tool to find these issues?

Yes. Our work is still under research and unpublished. Once it is released, I'll be very pleased to share all details.

@hawkingrei
Copy link
Member

@sayJason Hello! I noticed that you have recently discovered many bugs in TiDB. Have you developed a fuzzing tool to find these issues?

Yes. Our work is still under research and unpublished. Once it is released, I'll be very pleased to share all details.

We are looking forward to your paper. If it gets published, please let us know as soon as possible. If you have any questions, feel free to ask us.

@sayJason
Copy link
Author

@sayJason Hello! I noticed that you have recently discovered many bugs in TiDB. Have you developed a fuzzing tool to find these issues?

Yes. Our work is still under research and unpublished. Once it is released, I'll be very pleased to share all details.

We are looking forward to your paper. If it gets published, please let us know as soon as possible. If you have any questions, feel free to ask us.

Thanks for your attention.

@wshwsh12
Copy link
Contributor

wshwsh12 commented Sep 5, 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 ConvertIntToUint(sc *stmtctx.StatementContext, val int64, upperBound uint64, tp byte) (uint64, error) {
	if sc.ShouldClipToZero() && val < 0 {
		return 0, overflow(val, tp)
	}

	if uint64(val) > upperBound {
		return upperBound, overflow(val, tp)
	}

	return uint64(val), nil
}

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 ConvertIntToUint doesn't care about its return value. When converting time -838:59:59 to uint32, it returns (maxUInt32, overflow).

Fix

We can use ConvertDecimalToUint.

@jayl1e jayl1e mentioned this issue Sep 21, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 severity/major 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.

4 participants