types: always handle overflow error outside the types package

[lcwangchao]

What problem does this PR solve?

Issue Number: close #47517

All functions in types package should respect settings types.Context. However, for most functions, it will ignore overflowAsWarning settings and return this error directly. To make sure all methods follows the same behavior, we unified this behavior to always return overflow error and the overflow handle should be outside the types package

What is changed and how it works?

1. Remove HandleOverflow in types.ConvertJSONToInt and types.ProduceDecWithSpecifiedTp
2. Add HandleOverflow in builtinCastXXXAsDecimalSig.evalDecimal and builtinCastJSONAsIntSig.evalInt.
3. Remove useless FlagIgnoreOverflowError, FlagZeroDateAsWarning, FlagZeroInDateAsWarning and FlagInvalidDateAsWarning flags.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No need to test

  • I checked and no code files have been changed.

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

[codecov]

Codecov Report

Merging #47997 (e2099f9) into master (f135464) will increase coverage by 1.1829%.
The diff coverage is 85.7142%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #47997        +/-   ##
================================================
+ Coverage   71.5845%   72.7674%   +1.1829%     
================================================
  Files          1401       1424        +23     
  Lines        405930     412323      +6393     
================================================
+ Hits         290583     300037      +9454     
+ Misses        95533      93370      -2163     
+ Partials      19814      18916       -898     

Flag	Coverage Δ
integration	42.3521% <81.4285%> (?)
unit	71.5679% <75.7142%> (-0.0166%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	54.0503% <ø> (ø)
parser	∅ <ø> (∅)
br	48.6251% <ø> (-4.2606%)	⬇️

[lcwangchao]

/retest

[YangKeao]

LGTM

Do we agree that the error handling in types package should follow similar pattern? Which means the function in types package should do its best-effort, and return valid values and all errors, the caller of these functions should take the responsibility to handle these errors.

Also, should we do the same refractoring for HandleTruncate in the future?

The problem is that, some functions will return both TruncateError and OverflowError (in different cases), then we may need to provide a function to handle all possible errors 🤔, but I wonder whether it's too hard/complex to implement this function right considering all these situations. As MySQL itself is not clean, the attitude to warnings in different situations is different. For example, casting a json string to integer can have different warnings compared with casting a normal string to integer:

select cast('-1' as unsigned); -- has warning
select cast(cast('"-1"' as json) as unsigned); -- no warning

(Though, TiDB doesn't do well in warning compatibility now. Maybe it's not a big problem).

[lcwangchao]

@YangKeao

Do we agree that the error handling in types package should follow similar pattern? Which means the function in types package should do its best-effort, and return valid values and all errors, the caller of these functions should take the responsibility to handle these errors.

Yes, I agree with you. The best way is to remove all flags and force return the error. However, I'm not sure it is practice now because too many places rely on the error processing inside the types package. Maybe we should reconsider it after finishing decoupling types and stmtctx.

The problem is that, some functions will return both TruncateError and OverflowError (in different cases), then we may need to provide a function to handle all possible errors 🤔, but I wonder whether it's too hard/complex to implement this function right considering all these situations.

Maybe need serval functions to translate SQLMode and other information such as statement type to ErrorHandleContext. At this time, we can still have one function to handle these error and just make the ErrorHandleContext as the input parameter which has different settings in different scene.

[lcwangchao]

/retest

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: XuHuaiyu, YangKeao

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [XuHuaiyu,YangKeao]
• pkg/expression/OWNERS [XuHuaiyu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2023-10-26 08:37:56.413029675 +0000 UTC m=+2510274.000139817: ☑️ agreed by YangKeao.
• 2023-10-30 03:40:01.822676784 +0000 UTC m=+2837999.409786928: ☑️ agreed by XuHuaiyu.

[lcwangchao]

/retest

[lcwangchao]

/retest