fix: preserve extended TIME function semantics

[VioletQwQ-0]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

Fixes #24784

What this PR does / why we need it:

Continues and completes the work from #25801 while preserving the original commits and authorship.

Changes:

• return complete extended TIME hours from HOUR;
• preserve extended hours and negative signs in TIME_FORMAT, and return NULL for an empty format;
• preserve MAKETIME fractional precision and negative/extended-hour behavior;
• use MySQL-compatible half-away-from-zero conversion for floating hour/minute arguments;
• keep VARCHAR/DECIMAL seconds exact through the microsecond rounding boundary instead of routing them through FLOAT64;
• bound exact-second parsing input/exponents to prevent unbounded big-integer allocation;
• cover integer widths, NULL/invalid inputs, endpoint clamping, exact rounding boundaries, and planner result scales.

Validation:

• go test -count=1 -run 'Test(TimeFormat|MakeTime|MakeTimeReturnScale)' ./pkg/sql/plan/function
• go test -race -count=10 -run 'Test(TimeFormat|MakeTime)' ./pkg/sql/plan/function
• go test -count=1 ./pkg/sql/plan/function
• go test -count=1 ./pkg/sql/plan
• go vet ./pkg/sql/plan/function
• git diff --check

Distributed SQL expectations were updated for the exact VARCHAR/DECIMAL boundary, .5 hour/minute rounding, and positive/negative empty formats; CI is the execution source for those BVT cases.

[mergify]

⚠️ The sha of the head commit of this PR conflicts with #25801. Mergify cannot evaluate rules on this PR. Once #25801 is merged or closed, Mergify will resume processing this PR. ⚠️

[VioletQwQ-0]

Fixed both remaining MAKETIME compatibility findings in 8f1ca3ac3.

Changes:

• binary seconds now use the same unsigned big-endian decoder as binary hours/minutes, so arbitrary leading-zero width extension is accepted while genuinely overflowing or out-of-range second values remain NULL;
• exact VARCHAR seconds now coerce empty and nonnumeric text to non-NULL zero, matching MySQL numeric coercion, while SQL NULL, negative/out-of-range numeric values, and the existing bounded-exponent guard retain their prior behavior.

Coverage:

• parser -> binder -> expression executor regressions for zero-extended binary seconds 1 and 59, true wider overflow, and empty/nonnumeric VARCHAR seconds;
• function-level vector coverage for empty/nonnumeric VARCHAR coercion;
• distributed SQL expectations for the reported forms.

Validation:

• go test -count=1 ./pkg/sql/plan/...
• go test -race -count=1 -run '^(TestMakeTimeExactStringSecondRounding|TestMakeTimeBinaryIntegerBoundaries|TestMakeTimeIntegerSecondRange)$' ./pkg/sql/plan/function
• go test -race -count=1 -run '^TestMakeTimeBinaryLiteralBindAndExecute$' ./pkg/sql/plan
• go vet ./pkg/sql/plan/function ./pkg/sql/plan
• go build ./pkg/sql/plan/function ./pkg/sql/plan
• git diff --check

I also reran the complete mo-self-review gate over the full PR diff against current upstream/main: all six lenses, both binder-to-executor closures, and Q1-Q3 passed with zero remaining blocker. The PR remains Ready for review.

[gouhongshen]

Codex automated review

One blocking DECIMAL precision regression remains. Earlier reported binary, empty-format, and VARCHAR coercion defects are fixed at this head. Focused Go tests could not complete because required CGo headers are unavailable.

P1 - Preserve exact DECIMAL hour/minute values through rounding (pkg/sql/plan/function/func_binary.go:7539)

When the seconds argument is DECIMAL, makeTimeCheck routes hour and minute DECIMAL arguments through FLOAT64. Values just below a half boundary can therefore become exactly .5 before this new math.Round call. For example, MAKETIME(12, CAST('59.49999999999999999999' AS DECIMAL(30,20)), CAST('0' AS DECIMAL(2,1))) converts the minute to float64 59.5, rounds it to 60, and returns NULL. MySQL 8.4.10 returns 12:59:00; the base implementation selected the same float overload but truncated it to 59, so this is a concrete regression. Preserve DECIMAL hour/minute precision until half-away-from-zero conversion and add boundary coverage.

[VioletQwQ-0]

Fixed the remaining DECIMAL hour/minute precision issue in 3e1d50ae9.

What changed:

• DECIMAL hour/minute arguments now stay on an exact DECIMAL128 overload instead of being converted to FLOAT64 before rounding.
• Exact DECIMAL rounding is half away from zero and clamps only after the exact integer result is known.
• The overload matrix covers DECIMAL hour/minute combined with float, VARCHAR, DECIMAL, and exact-second inputs without changing VARCHAR truncation semantics.

Coverage added:

• the reported 59.49999999999999999999 minute case;
• DECIMAL64 and DECIMAL128 values below/at the half boundary;
• negative half-hour, minute rounding to 60, and positive/negative DECIMAL(38,0) overflow;
• parser -> binder -> expression-executor probes for an 8K-digit exponent and a 4097-digit mantissa, including terminal executor/process cleanup.

I also rechecked every earlier change request at the new head: binary IsBin, empty TIME_FORMAT, zero-extended binary seconds, and nonnumeric VARCHAR-second coercion remain covered and passing.

Validation:

• go test ./pkg/sql/plan/function ./pkg/sql/plan -count=1
• focused -race tests in both packages, 3 runs each
• go vet and go build for both packages
• git diff --check
• full exact-head preflight manifest and mo-self-review: PASS, no pending evidence or open blocker

[XuPeng-SH]

Codex deep review — changes requested

Reviewed exact head 3e1d50ae90d748ed3d6e771c2e13d294e1536916. The earlier binary IsBin, zero-extended binary-second, nonnumeric VARCHAR-second, and DECIMAL128 boundary findings are fixed. Three reachable MySQL-compatibility blockers remain. Existing package tests and CI pass because these cases are not covered.

P1 — Keep DECIMAL256 hour/minute exact instead of narrowing to DECIMAL128(38,0) (pkg/sql/plan/function/list_builtIn.go:10815)

exactHour and exactMinute accept every Oid.IsDecimal(), including SQL DECIMAL(65,...) backed by DECIMAL256, but the checker unconditionally targets DECIMAL128. SetTargetScaleFromSource does not preserve a DECIMAL256 source scale for a DECIMAL128 target, so DECIMAL(65,30) is bound as DECIMAL128(38,0) before the new exact getter runs.

A planner-to-expression-executor regression test reproduces both failures:

• MAKETIME(CAST("12.499999999999999999999999999999" AS DECIMAL(65,30)),0,0) returns 13:00:00; MySQL 8.0.44 and 8.4.10 return 12:00:00.
• A 65-digit DECIMAL(65,0) hour errors while converting to DECIMAL128; both MySQL versions return the saturated 838:59:59.

Add DECIMAL256 overload/getter support or another bounded exact path, and cover below/at-half plus positive/negative overflow through binder and executor.

P1 — Do not reject safe exponent underflow or a zero mantissa (pkg/sql/plan/function/func_binary.go:7471)

The allocation guard returns NULL solely when the exponent magnitude exceeds 4096. This changes the value semantics of short, safely classifiable inputs: MAKETIME(12,34,"1e-5000") and MAKETIME(12,34,"0e5000") both become NULL here, while MySQL 8.0.44 and 8.4.10 return 12:34:00.000000 for both. The current extreme test covers only a nonzero huge positive exponent.

Keep the bounded parser, but classify exponent direction and a zero mantissa before rejecting: sufficiently negative exponents underflow to zero, and a zero mantissa remains zero for any exponent. Add direct and binder-to-executor regressions.

P1 — Empty TIME_FORMAT must remain NULL (test/distributed/cases/function/func_datetime_time_format.test:43)

The newly added BVT expectations codify an empty non-NULL string for both signs, and TimeFormat appends the empty buffer with isNull=false. Direct checks against official MySQL 8.0.44 and 8.4.10 both report TIME_FORMAT(CAST("15:30:45" AS TIME), "") IS NULL = 1 (same for negative TIME). This also matches the PR description, which says an empty format returns NULL. The earlier review claiming an empty string is contradicted by both official runtime versions.

Restore the NULL result for an empty format and update both unit/BVT expectations.

[gouhongshen]

Codex automated review

The latest DECIMAL128 precision fix is valid, but it introduces a blocking DECIMAL256 regression. Earlier review findings remain fixed. Focused tests could not run locally because cgo/libmo.dylib is absent; exact-head CI is green.

P1 - Preserve DECIMAL256 hour/minute inputs instead of narrowing to DECIMAL128 (pkg/sql/plan/function/list_builtIn.go:10816)

IsDecimal() includes DECIMAL256, but the new exact path unconditionally targets DECIMAL128 for hour/minute arguments. Thus a valid value such as MAKETIME(CAST('999999999999999999999999999999999999999' AS DECIMAL(39,0)),0,0) is implicitly narrowed to DECIMAL128(38,0); decimal256ToDecimal128 calls ParseDecimal128, which rejects the 39-digit value and aborts evaluation instead of letting MAKETIME clamp it to 838:59:59. Before this head, DECIMAL256 followed the FLOAT64 overload and reached the existing endpoint clamp. DECIMAL256 is SQL-reachable for precision above 38 (build_util.go:205-206), but the new tests cover only DECIMAL64/128. Add an exact DECIMAL256 getter/overloads, or otherwise preserve its range and scale without narrowing.

[VioletQwQ-0]

Addressed the latest change requests in 3513b2343.

@XuPeng-SH:

• DECIMAL256 hour/minute values now stay on DECIMAL256 overloads instead of narrowing through DECIMAL128. The planner/executor tests cover below-half, at-half, and positive/negative overflow, and the overload test covers all 14 mixed DECIMAL256 combinations.
• The exact-second parser now distinguishes value semantics from its allocation guard: large negative exponents underflow to zero, and a zero mantissa stays zero even with a large exponent or a 4097-digit zero representation. Large positive nonzero exponents and over-limit nonzero mantissas remain bounded/rejected.
• Empty TIME_FORMAT is NULL again for both positive and negative TIME. I also checked the official MySQL 8.4.6 source independently: Item_func_date_format::val_str takes the NULL path when the format length is zero.

@gouhongshen:

• The duplicate DECIMAL256 narrowing finding is fixed by the same concrete DECIMAL256 getter/overload path above; it no longer calls the DECIMAL256-to-DECIMAL128 cast.

Validation:

• go test ./pkg/sql/plan/function ./pkg/sql/plan -count=1
• focused planner/executor MAKETIME and TIME_FORMAT tests
• git diff --check
• full exact-head preflight/self-review manifest PASS on 3513b23431994225264db1ec1ec7c0d6983b679c

[XuPeng-SH]

Codex deep review — changes requested

Reviewed exact head 3513b23431994225264db1ec1ec7c0d6983b679c. The earlier DECIMAL256, exponent-underflow, zero-mantissa, and empty-TIME_FORMAT blockers are fixed. Full package tests, focused race tests, go vet, formatting, and current CI all pass. One reachable MySQL-compatibility blocker remains.

P1 — Apply the exact-second cap to significant digits, not raw mantissa width (pkg/sql/plan/function/func_binary.go:7478)

The allocation guard counts every mantissa digit before removing insignificant zeros. A VARCHAR second consisting of 4096 leading zeroes followed by 1 has 4097 digits and a nonzero mantissa, so this branch returns NULL even though its numeric value is exactly 1. Both official MySQL 8.0.44 and 8.4.10 return 12:34:01.000000 for MAKETIME(12,34,<that literal>). The new 4097-zero test misses this because the all-zero early return runs before the cap; analogous over-limit trailing-zero forms such as 1. followed by zeroes have the same problem.

Keep the allocation bound, but normalize/skip insignificant leading integer zeroes and trailing fractional zeroes (with the decimal exponent adjusted as needed), or otherwise cap significant digits rather than representation length. Add direct parser and binder-to-executor regressions for an over-limit zero-extended nonzero value.

[gouhongshen]

Codex automated review

One blocking compatibility defect remains in the bounded exact-second parser.

P1 - Cap significant digits rather than raw mantissa length (pkg/sql/plan/function/func_binary.go:7478)

The exact VARCHAR-second path counts every mantissa digit before parsing and returns NULL when that count exceeds 4096. Therefore a value such as MAKETIME(12, 34, '000...001') with 4096 leading zeroes is rejected even though its exact numeric value is 1 and is within the valid second range. The existing all-zero test does not catch this because it returns through the earlier zero-mantissa branch. Normalize insignificant leading/trailing zeroes or otherwise cap significant digits, and add direct plus binder-to-executor coverage.