pingcap / parser

A MySQL Compatible SQL Parser
Apache License 2.0
1.41k stars 489 forks source link

mysql: modify TypeNewDecimal length in defaultLengthAndDecimalForCast #1202

Closed HuGanghui closed 3 years ago

HuGanghui commented 3 years ago

What problem does this PR solve?

Fix https://github.com/pingcap/tidb/issues/23495 in TiDB

What is changed and how it works?

modify TypeNewDecimal length in defaultLengthAndDecimalForCast from 11 to 10.

Check List

Tests

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

XuHuaiyu commented 3 years ago

/lgtm

XuHuaiyu commented 3 years ago

/rebuild

HuGanghui commented 3 years ago

and the integration failed: FAIL github.com/pingcap/tidb/planner/core 149.403s Is ok? And I am not sure why this happened, if need to be fixed please give some guidance.

ichn-hu commented 3 years ago

/run-all-tests

ichn-hu commented 3 years ago

and the integration failed: FAIL github.com/pingcap/tidb/planner/core 149.403s Is ok? And I am not sure why this happened, if need to be fixed please give some guidance.

you can checkout the circle CI's log by clicking details (or see it here)

your code breaks the TiDB's test, namely:

----------------------------------------------------------------------
FAIL: typeinfer_test.go:56: testInferTypeSuite.TestInferType

typeinfer_test.go:152:
    c.Assert(tp.Flen, Equals, tt.flen, comment)
... obtained int = 10
... expected int = 11
... for select CAST(c_int_d AS DECIMAL) from t

OOPS: 574 passed, 1 skipped, 1 FAILED
--- FAIL: TestT (25.35s)

you need to fix them accordingly on TiDB's side.

I have also commented on the original issue, https://github.com/pingcap/tidb/issues/23495#issuecomment-809980259 please take a look.

Let me know if you have any questions.

HuGanghui commented 3 years ago

@ichn-hu ok, thanks for you help, I will try to correct it.

kennytm commented 3 years ago

/lgtm